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

service/dap: add panic and throw text to stopped event #2559

Merged
merged 4 commits into from
Jul 13, 2021

Conversation

suzmue
Copy link
Contributor

@suzmue suzmue commented Jun 30, 2021

We can add more information to the stopped events on errors using
the Text field in the stopped event. We already use this to display
the runtime errors. Adding this information to the stopped reason will
also help to show the user additional info when a stopped event is not
associated with a particular goroutine.

@suzmue
Copy link
Contributor Author

suzmue commented Jun 30, 2021

Example for deadlock:

Screen Shot 2021-06-30 at 3 31 33 PM

We can add more information to the stopped events on errors using
the `Text` field in the stopped event. We already use this to display
the runtime errors. Adding this information to the stopped reason will
also help to show the user additional info when a stopped event is not
associated with a particular goroutine.
Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@derekparker derekparker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking this from merge until #2563 is pulled in and this is rebased.

@derekparker derekparker added this to the v1.7.0 milestone Jul 6, 2021
@suzmue
Copy link
Contributor Author

suzmue commented Jul 7, 2021

Blocking this from merge until #2563 is pulled in and this is rebased.

This branch has been rebased.

body.Description, err = s.throwReason(goroutineID)
if err != nil {
body.Description = fmt.Sprintf("Error getting throw reason: %s", err.Error())
// This is not currently working for Go 1.16 or 1.17: https://github.com/golang/go/issues/46425.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue was closed 7 days ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed comment about 1.17

Copy link
Member

@derekparker derekparker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekparker derekparker merged commit f86ed67 into go-delve:master Jul 13, 2021
@suzmue suzmue deleted the stoppedReason branch July 13, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants