Avoid calling OnBlockEnd on panic
#31828
Open
+46
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This avoid problem where the tracer receives `OnBlockEnd(nil)` and thus cannot know that an error happen, in which case it could flush data out. Technically, shouldn't be a big problem as since there is a panic, the block will not be committed to geth storage and will be replayed correctly. The other possibility would be to still call the tracer but passing the recovered error to `OnBlockEnd`, but this would require transforming the recovered `any` type into an `error`.
|
Right I see it. This is true for all of the other deferred hooks, e.g. OnTxEnd. If any of their lower level hooks panics they will still be called. #31226 will fix this but I am unsure of that PR because of the performance penalty. On the other hand, this PR will catch any panic not only the ones arising from the tracer. One could make the case that if anything panics in the blockchain we should probably stop reporting to the tracer altogether but even then I would like to make a more comprehensive fix than just for one of the hooks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This avoid problem where the tracer receives
OnBlockEnd(nil)and thus cannot know that an error happen, in which case it could flush data out.Technically, shouldn't be a big problem as since there is a panic, the block will not be committed to geth storage and will be replayed correctly.
The other possibility would be to still call the tracer but passing the recovered error to
OnBlockEnd, but this would require transforming the recoveredanytype into anerror.Relates to #31226, unsure if it would handle the case here.