-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Wrap RPC errors #12638
Wrap RPC errors #12638
Conversation
I see you updated files related to |
return s | ||
} | ||
|
||
func (s *subErrorWrapper) close() { |
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.
do we need to close the unSub chan?
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.
No, we should not close unSub
, as it might cause panic on the callers side if it calls Unsubscribe. Example. There is no guarantee that we won't try writing into unsub
, even if done
is closed.
Also we are not leaking resources. As soon as subErrorWrapper is closed, we won't have any additional reference to the unsub
, so when GC collects subErrorWrapper
it will also collect unsub
.
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.
@dhaidashenko looks good to me. Just one thing to confirm:
If I want to filter out all RPC related errors I need to exclude messages prefixed with RPCClient returned error...
correct?
I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:
|
00138d5
Quality Gate passedIssues Measures |
This reverts commit bcf7653.
Ticket