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

Catch panic in trace transaction / trace call #2000

Merged
merged 5 commits into from
Dec 24, 2024

Conversation

jewei1997
Copy link
Contributor

@jewei1997 jewei1997 commented Dec 23, 2024

Describe your changes and provide context

Small change to catch the panic at the top level of TraceTransaction and TraceCall. This way, if one of these panics, it will return more gracefully fail and print out a panic error message instead of "method handler crashed".

Testing performed to validate your change

existing unit tests

Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.53%. Comparing base (fce45fd) to head (04cfe8d).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2000      +/-   ##
==========================================
+ Coverage   61.46%   61.53%   +0.07%     
==========================================
  Files         263      263              
  Lines       24480    24480              
==========================================
+ Hits        15046    15064      +18     
+ Misses       8311     8295      -16     
+ Partials     1123     1121       -2     

see 3 files with indirect coverage changes

@jewei1997 jewei1997 enabled auto-merge (squash) December 24, 2024 14:49
@jewei1997 jewei1997 merged commit 70c6339 into main Dec 24, 2024
47 checks passed
@jewei1997 jewei1997 deleted the catch-panic-outer-TraceTransaction branch December 24, 2024 14:55
jewei1997 added a commit that referenced this pull request Dec 24, 2024
* checkpoint

* fix

* fix

* update geth dep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants