Skip to content

Conversation

@chirag-parmar
Copy link
Contributor

@chirag-parmar chirag-parmar commented Aug 29, 2025

Description:

This PR rectifies all the improper error handling in verified proxy. For this, it could be seen as a refactoring PR. It additionally also brings in new error definitions that will prove to be useful in the future.

Notes:

  1. Error Definitions: https://github.com/status-im/nimbus-eth1/pull/3607/files#diff-e4cd66b69edca5086fbc7116e2235d0f00e217818de266a9e555d4dfee97e227R37
  2. Most of the caught CatchableErrors are now removed by translating them to engine errors
  3. CancelledErrors are propagated properly (reraised wherever CatchableError is caught)

meta: #3101

@chirag-parmar chirag-parmar marked this pull request as ready for review October 31, 2025 06:10
@chirag-parmar chirag-parmar requested review from kdeme and tersec October 31, 2025 06:10
Copy link
Contributor

@kdeme kdeme left a comment

Choose a reason for hiding this comment

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

This PR really changes the current usage of Result into the direction of exceptions to deal with error handling in NVP.

While it does look logically correct (I didn't check everything), and exception handling has been improved a lot with usages of raises and the addition of async raise, we have been mostly going in the other direction in our projects. That is, towards the usage of Result.

For reasoning behind this see also https://status-im.github.io/nim-style-guide/errors.html

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.

2 participants