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

Silence discovery error logs if the request is cancelled #1846

Closed
yondonfu opened this issue Apr 19, 2021 · 1 comment · Fixed by #1837
Closed

Silence discovery error logs if the request is cancelled #1846

yondonfu opened this issue Apr 19, 2021 · 1 comment · Fixed by #1837
Assignees

Comments

@yondonfu
Copy link
Member

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

During discovery, a B will call GetOrchestrator() on all available Os. Once B receives a sufficient number of responses, it will cancel the remaining requests using a context. The cancellation of these requests results in a ton of error messages indicating that the GetOrchestrator() call failed due to a cancellation. As a result, the logs of B can become flooded with these error logs even though the logs don't indicate any issues with the Os because it was the B that cancelled the request.

Describe the solution you'd like
A clear and concise description of what you want to happen.

Silence discovery error logs if the GetOrchestrator() call was cancelled.

This draft PR outlines one potential approach to silencing those discovery error logs by using the new errors Is() API. But, the code is untested and it is not clear if the Is() API is going to work as expected - I'm not sure if the function that returns the error would need to support error wrapping/unwrapping. An alternative would be to check if the error string contains a context cancellation message. Being able to use the errors Is() API could be cleaner though as I think this type of error checking is what it is meant for.

@kyriediculous
Copy link
Contributor

Had to wrap the errors instead of concatenating the messages as the API docs of errors.Is() suggests. Works as expected, tested it on rinkeby and only get insufficient sender reserve errors now.

#1837

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants