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

Fix connected status when not. #1400

Merged
merged 2 commits into from May 8, 2019

Conversation

Projects
None yet
2 participants
@molnard
Copy link
Contributor

commented May 2, 2019

When I am on RegTest and my local Backend is not running, my Wasabi still indicating that my Backend is connected. It is better to set the connected state only if we got the first synchronization packet. Despite of the solution is trivial for some reason it was not fixed until now. This fact leads me to think there is hidden complexity behind this.

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 3, 2019

We set notconnected based on the exception. But now you want to set notconnected, no matter the exception?

@nopara73 nopara73 self-requested a review May 3, 2019

@nopara73
Copy link
Collaborator

left a comment

Did you look at the code above your change?

@molnard

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

Yes, GetSynchronizeAsync() can throw different exceptions with different meaning. What is strange for me in this code that an exception is thrown and we set Tor running and Backend connected which is clearly not true in my case. I can add one more catch clause but why do we indicate OK on any other exception which is not caught above?

One more thing to consider. Different OP systems can throw different Socket error messages in the same conditions.

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 3, 2019

One more thing to consider. Different OP systems can throw different Socket error messages in the same conditions.

I think we handle them properly and encapsulate them.

I can add one more catch clause but why do we indicate OK on any other exception which is not caught above?

If it's a connection issue then we should encapsulate it to a ConnectionException. Normal Exception should not be thrown at all. That means there's a bug in there somewhere.

@molnard molnard marked this pull request as ready for review May 6, 2019

@nopara73 nopara73 merged commit f7994a1 into zkSNACKs:master May 8, 2019

4 checks passed

CodeFactor No issues found.
Details
Wasabi.Linux #20190506.11 succeeded
Details
Wasabi.Osx #20190506.11 succeeded
Details
Wasabi.Windows #20190506.11 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.