Skip to content

Improve error handling - #1841

Merged
Mrtenz merged 4 commits into
mainfrom
feat/error-handling
Oct 20, 2023
Merged

Improve error handling#1841
Mrtenz merged 4 commits into
mainfrom
feat/error-handling

Conversation

@Mrtenz

@MrtenzMrtenz commented Oct 11, 2023

Copy link
Copy Markdown
Member

This improves error handling by:

  • Using @metamask/rpc-errors instead of eth-rpc-errors throughout the entire stack.
  • Introducing a new SnapError, which will not cause the Snap to crash when thrown.
  • Unwrapping errors where possible.
  • Also bumps a ton of MetaMask dependencies, see Bump MetaMask core packages #1818

@socket-security

socket-securityBot commented Oct 19, 2023

Copy link
Copy Markdown

@socket-security

socket-securityBot commented Oct 19, 2023

Copy link
Copy Markdown

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: eth-block-tracker@8.1.0

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

    @codecov

    codecovBot commented Oct 19, 2023

    Copy link
    Copy Markdown

    Codecov Report

    Attention: 21 lines in your changes are missing coverage. Please review.

    Comparison is base (495bdd4) 96.06% compared to head (c9a9288) 96.11%.

    Additional details and impacted files
    @@ Coverage Diff @@## main #1841 +/- ##
    ==========================================
    + Coverage 96.06% 96.11% +0.05% 
    ==========================================
    Files 243 243 Lines 5664 5771 +107 Branches 895 925 +30 ==========================================
    + Hits 5441 5547 +106 - Misses 223 224 +1 
    FilesCoverage Δ
    ...ages/snaps-controllers/src/snaps/SnapController.ts97.55% <100.00%> (+0.01%)⬆️
    .../snaps-controllers/src/snaps/endowments/keyring.ts96.96% <100.00%> (ø)
    ...ps-controllers/src/snaps/endowments/name-lookup.ts100.00% <100.00%> (ø)
    ...ages/snaps-controllers/src/snaps/endowments/rpc.ts100.00% <100.00%> (ø)
    ...ollers/src/snaps/endowments/transaction-insight.ts100.00% <100.00%> (ø)
    ...cution-environments/src/common/endowments/index.ts100.00% <100.00%> (ø)
    ...ion-environments/src/common/endowments/interval.ts100.00% <100.00%> (ø)
    ...tion-environments/src/common/endowments/timeout.ts100.00% <100.00%> (ø)
    ...-execution-environments/src/common/globalEvents.ts100.00% <100.00%> (ø)
    ...s/snaps-execution-environments/src/common/utils.ts100.00% <ø> (+6.25%)⬆️
    ... and 27 more

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    Comment threadpackages/snaps-controllers/package.json Outdated
    FrederikBoldingand others added 2 commits October 20, 2023 15:10
    Rebased and updated version of
    #1728
    - Updates and uses `@metamask/rpc-errors` everywhere
    - Updates and uses `@metamask/json-rpc-engine` everywhere
    - Updates the following packages to latest:
    `@metamask/permission-controller`, `@metamask/approval-controller`,
    `@metamask/providers`, `@metamask/eth-json-rpc-middleware`
    Huge thank you to @legobeat for all of the help landing this ❤️
    Blocked by #1823
    ---------
    Co-authored-by: legobt <6wbvkn0j@anonaddy.me>
    Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
    This implements the `SnapError` class, and wrapping/unwrapping of errors
    thrown from a Snap. `SnapError`s will not cause the Snap to crash, and
    can be used to respond to JSON-RPC requests for example.
    Closes#1824.
    @Mrtenz
    Mrtenzforce-pushed the feat/error-handling branch from 3f374b4 to 0f40614CompareOctober 20, 2023 13:14
    @Mrtenz

    Copy link
    Copy Markdown
    MemberAuthor

    @SocketSecurity ignore eth-block-tracker@8.1.0

    This is ours.

    @Mrtenz
    Mrtenz marked this pull request as ready for review October 20, 2023 13:27
    @Mrtenz
    Mrtenz requested a review from a team as a code ownerOctober 20, 2023 13:27
    Comment threadpackages/snaps-controllers/package.json Outdated

    @FrederikBoldingFrederikBolding left a comment

    Copy link
    Copy Markdown
    Member

    Choose a reason for hiding this comment

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

    LGTM

    Holding off on approving until someone else has approved.

    @hmalik88hmalik88 left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    LGTM. Great work, guys 🚀

    @Mrtenz
    Mrtenz merged commit f2e652b into mainOct 20, 2023
    @Mrtenz
    Mrtenz deleted the feat/error-handling branch October 20, 2023 14:54
    Sign up for freeto 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.

    3 participants

    @Mrtenz@FrederikBolding@hmalik88