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

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

Comments

@stamblerre
Copy link
Contributor

commented Aug 6, 2019

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 label Aug 6, 2019

@gopherbot gopherbot added this to the Unreleased milestone Aug 6, 2019

@gopherbot gopherbot added the gopls label Aug 6, 2019

@jan-xyz

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2019

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 8, 2019

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

This comment has been minimized.

Copy link

commented Aug 22, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.