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

Return a numeric error code for the exceptions that will be thrown #1541

Merged
merged 1 commit into from Jan 31, 2020

Conversation

mrBliss
Copy link
Contributor

@mrBliss mrBliss commented Jan 30, 2020

Fixes #1201.

The node will use this ExitFailures in the toplevelExceptionHandler (in
cardano-node) to report to the wallet why it failed. Based on the
ExitFailure code, the wallet can give a more useful error message and
perform (or instruct the user to perform) the right action.

@mrBliss mrBliss added the consensus issues related to ouroboros-consensus label Jan 30, 2020
@mrBliss mrBliss requested a review from edsko January 30, 2020 15:50
-- | There is a problem with the network connection, the user should
-- investigate.
--
-- TODO We're not yet returning this.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@coot @karknu Which exception would the network layer throw when there is a network connection error?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could have internal "impossible" errors, but none expected due to a bad network connection. Those would not be fatal for the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thinking is that when the node is started without a working internet connection, it should be reported to the wallet so that it can give some useful instructions to the user instead of some cryptic (to most non-developers) error message. Would the lack of a working internet connection result in a MuxError or would it just time out and retry continuously?

How do you suggest we report such issues to the wallet?

Maybe it's best to remove that case from this PR for now and let you guys come up with a better way to report network connectivity issues?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we leave the error code and decide later on how to actually detect this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. This means we can merge as is.

Fixes #1201.

The node will use this `ExitFailure`s in the `toplevelExceptionHandler` (in
`cardano-node`) to report to the wallet why it failed. Based on the
`ExitFailure` code, the wallet can give a more useful error message and
perform (or instruct the user to perform) the right action.
@mrBliss
Copy link
Contributor Author

mrBliss commented Jan 31, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jan 31, 2020
1541: Return a numeric error code for the exceptions that will be thrown r=mrBliss a=mrBliss

Fixes #1201.

The node will use this `ExitFailure`s in the `toplevelExceptionHandler` (in
`cardano-node`) to report to the wallet why it failed. Based on the
`ExitFailure` code, the wallet can give a more useful error message and
perform (or instruct the user to perform) the right action.

Co-authored-by: Thomas Winant <thomas@well-typed.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 31, 2020

@iohk-bors iohk-bors bot merged commit d4737d8 into master Jan 31, 2020
@iohk-bors iohk-bors bot deleted the mrBliss/exit-failure branch January 31, 2020 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inform Daedalus about fatal exceptions and what to do
4 participants