-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[cmd/opampsupervisor] Fix early exits not shutting down client/server in bootstrapping #31944
[cmd/opampsupervisor] Fix early exits not shutting down client/server in bootstrapping #31944
Conversation
9f85771
to
1d9167f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple wording nits, feel free to ignore if they're not helpful. Looks good!
2666090
to
523bf0d
Compare
523bf0d
to
8f2b374
Compare
8f2b374
to
2f1c0d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting a fix for this, and sorry for the long delay. Could you explain the flow of what is returned to me? If we execute return errors.New("...")
, is err
set to the return value in the deferred functions?
Yes, that's exactly right. In the defer statements, Here's an example: https://go.dev/play/p/hT6HZSK62NH |
Great, thank you. That's straightforward, just hadn't seen that pattern before. |
Description:
Link to tracking Issue: Closes #31943
Testing:
I tested manually adding errors into the code. There isn't a good, consistent way to add e.g. the timeout error as-is.