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

WIP Investigation into replacing anomaly with eyre for error handling #493

Closed
wants to merge 11 commits into from

Conversation

adizere
Copy link
Member

@adizere adizere commented Jan 2, 2021

Closes: #68

The PR in its current form is an experiment with replacing anomaly with eyre.
Only the following components have transitioned to eyre:

  • ICS02 (completely)
  • relayer-cli (the part about keys commands)
  • relayer (only some modules related to key manipulation)

If we're satisfied with eyre, we'll continue porting all of ibc-rs in a couple more separate PRs.

Discussion here: #68 (comment)


For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@codecov-io
Copy link

codecov-io commented Jan 2, 2021

Codecov Report

Merging #493 (f165f26) into master (b1b37f5) will increase coverage by 15.6%.
The diff coverage is 54.3%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #493      +/-   ##
=========================================
+ Coverage    13.6%   29.3%   +15.6%     
=========================================
  Files          69     160      +91     
  Lines        3752   12891    +9139     
  Branches     1374    4989    +3615     
=========================================
+ Hits          513    3785    +3272     
- Misses       2618    8418    +5800     
- Partials      621     688      +67     
Impacted Files Coverage Δ
...application/ics20_fungible_token_transfer/error.rs 0.0% <ø> (ø)
...pplication/ics20_fungible_token_transfer/events.rs 0.0% <ø> (ø)
...ion/ics20_fungible_token_transfer/msgs/transfer.rs 0.0% <0.0%> (ø)
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/msgs.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 6.8% <0.0%> (-26.6%) ⬇️
modules/src/ics03_connection/events.rs 0.0% <0.0%> (ø)
modules/src/ics04_channel/error.rs 13.0% <0.0%> (-12.0%) ⬇️
... and 287 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 453c1da...0db9943. Read the comment docs.

@adizere adizere changed the title Investigation into replacing anomaly with eyre for error handling WIP Investigation into replacing anomaly with eyre for error handling Jan 5, 2021
The test "incorrect versions" was not exercising an failing path due
to mismatching versions, but was failing because of error in consensus
state verification (ConsensusStateVerificationFailure). This test
is no longer useful since all versions are set to the default
and therefore always match, so there is no failing path to test.
@adizere
Copy link
Member Author

adizere commented Jan 7, 2021

Retiring this PR, we'll leave the work for later.

@adizere adizere closed this Jan 7, 2021
@romac romac deleted the adi/68_errors branch April 8, 2021 20:41
@adizere adizere mentioned this pull request May 17, 2021
5 tasks
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 this pull request may close these issues.

Error wrapping and conversion between modules
2 participants