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

Chain DB: catch and rewrap iterator exceptions #797

Merged
merged 3 commits into from
Jul 26, 2019

Conversation

mrBliss
Copy link
Contributor

@mrBliss mrBliss commented Jul 19, 2019

Properly catch exceptions encountered by ImmutableDB iterators (used by ChainDB iterators and readers) and rewrap them in ChainDbFailure.

mrBliss added 3 commits July 19, 2019 12:46
The ImmutableDB and the VolatileDB use an `ErrorHandling` record to throw and
catch exceptions (~= a dictionary for `MonadCatch` that can also be
instantiated using `ExceptT`, etc.). In `withDB`, which catches
ImmutableDB/VolatileDB exceptions, we were just using `MonadCatch` instead of
`ErrorHandling.catchError`, so that we were not actually catching the
errors (nor rethrowing them wrapped in `ChainDbFailure`).

Fix this by storing the `ErrorHandling` that was passed to open the respective
DB and use it in `withDB` to properly catch the exceptions.
We use the `ImmDB.withDB` wrapper for functions on the ImmutableDB so that
errors are properly caught and rethrown.

This was not being used for functions using ImmutableDB iterators, so that
errors thrown by these were not being handled properly.

Fix this by exposing wrapped version of these functions. This means these
functions now take an `ImmDB` as extra argument.
@mrBliss mrBliss added bug Something isn't working consensus issues related to ouroboros-consensus labels Jul 19, 2019
@mrBliss mrBliss requested review from dcoutts and nc6 July 19, 2019 10:51
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

LGTM.

@mrBliss mrBliss merged commit f3fdb0c into master Jul 26, 2019
@iohk-bors iohk-bors bot deleted the mrBliss/chaindb-error-handling branch July 26, 2019 10:05
mrBliss added a commit that referenced this pull request Jul 26, 2019
PR #797 was recently merged, which no longer compiled after merging with
master because of PR #810.

CI did not signal this because there were no merge conflicts and it never
rebuilt PR #797 against the updated master.

To avoid this in the future, use `bors r+`, which first tries to build the
merged PR (as opposed to the unmerged PR branch) before actually pushing to
master.
@mrBliss mrBliss mentioned this pull request Jul 26, 2019
iohk-bors bot added a commit that referenced this pull request Jul 26, 2019
831: Fix broken master r=nc6 a=mrBliss

PR #797 was recently merged, which no longer compiled after merging with
master because of PR #810.

CI did not signal this because there were no merge conflicts and it never
rebuilt PR #797 against the updated master.

To avoid this in the future, use `bors r+`, which first tries to build the
merged PR (as opposed to the unmerged PR branch) before actually pushing to
master.

Co-authored-by: Thomas Winant <[email protected]>
coot pushed a commit that referenced this pull request May 16, 2022
…ndling

Chain DB: catch and rewrap iterator exceptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants