Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: log error frames when logging an error #33499

Closed
stamblerre opened this issue Aug 6, 2019 · 5 comments
Closed

x/tools/gopls: log error frames when logging an error #33499

stamblerre opened this issue Aug 6, 2019 · 5 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@stamblerre
Copy link
Contributor

Now that we are using x/errors in gopls, we can also log error frames when logging errors.
@jan-xyz is interested in adding this behavior.

@stamblerre stamblerre added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 6, 2019
@gopherbot gopherbot added this to the Unreleased milestone Aug 6, 2019
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Aug 6, 2019
@jan-xyz
Copy link

jan-xyz commented Aug 8, 2019

According to the blog post https://blog.golang.org/experiment the design draft for printing frames is abandoned and they are on a look out for a better solution. I am not sure if the current implementation will stay. Do you have any more insights on this @stamblerre ?

My idea was to print the errors with log.Errorf("%+v", err)

@stamblerre
Copy link
Contributor Author

Ah, I missed Russ's talk at GopherCon. I think I'm fine with using the features supported by x/errors as long as they are around, just to try them out.

I think we could make these changes quite simple and easy to revert because @ianthehat's telemetry work introduced a new way of logging errors. Supporting this behavior might be as simple as adding the %+v in a single place. Take a look at https://godoc.org/golang.org/x/tools/internal/lsp/telemetry/log and let me know what you think.

@jan-xyz
Copy link

jan-xyz commented Aug 8, 2019

Just printing errors as %+v probably doesn't hurt in general. I suppose it's enough to change the format here:
https://github.com/golang/tools/blob/128824a23e7c038a341252700f1001ad29ef1988/internal/lsp/telemetry/log/entry.go#L46

Thanks for the hint :) I'll try it out locally. Any hint on how to reliably cause an error?

@stamblerre
Copy link
Contributor Author

Something like messing with the name of the package, adding an import that doesn't exist, completing something that's not declared - these would all likely cause errors to be logged.

@gopherbot
Copy link

Change https://golang.org/cl/189344 mentions this issue: internal/lsp: add additional information when logging errors

@golang golang locked and limited conversation to collaborators Aug 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants