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

WasabiSynchronizer: Fix exception being handled. #8821

Conversation

kiminuo
Copy link
Collaborator

@kiminuo kiminuo commented Jul 30, 2022

Yahia's comment shows that we do not handle proper exception and thus when stopping WW we might show an exception stacktrace when it's not needed.

This PR should correct that slightly.

@kiminuo kiminuo marked this pull request as ready for review July 30, 2022 21:12
@kiminuo kiminuo requested a review from molnard July 31, 2022 17:12
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

LGTM except that if we ever get TorConnectionException as well - now we are not catching it.

@kiminuo
Copy link
Collaborator Author

kiminuo commented Aug 1, 2022

I don't understand.

TorConnectionException is always wrapped when thrown from TorHttpPool. So in "user code" we should never catch Tor*Exceptions but rather HttpRequestExceptions.

Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

If that is the case #8821 (comment) then this is OK.

@molnard molnard merged commit f5955f5 into zkSNACKs:master Aug 1, 2022
@molnard
Copy link
Collaborator

molnard commented Aug 1, 2022

@kiminuo kiminuo deleted the feature/2022-07-30-WasabiSynchronizer-exception branch August 1, 2022 09:31
@kiminuo
Copy link
Collaborator Author

kiminuo commented Aug 1, 2022

Yes and that's because this is inside TorHttpPool (logically).

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

Successfully merging this pull request may close these issues.

None yet

2 participants