-
Notifications
You must be signed in to change notification settings - Fork 69
core/txpool: remove use of errors.Join function #27523 #1914
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
base: dev-upgrade
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
051aee9 to
e3ec789
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the use of Go's errors.Join function from the transaction pool's Close method, replacing it with a custom error aggregation approach using fmt.Errorf. The change was made to address issue ethereum#27523, likely for compatibility with older Go versions that don't support errors.Join (introduced in Go 1.20).
Key changes:
- Replaced
errorsimport withfmtimport - Modified error collection to only append non-nil errors instead of all errors
- Replaced
errors.Join(errs...)with a manual check and custom error formatting
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| return errors.Join(errs...) | ||
| if len(errs) > 0 { | ||
| return fmt.Errorf("subpool close errors: %v", errs) |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message "subpool close errors" is misleading because the errs slice can also contain an error from the quit channel (line 92-94), which is not a subpool close error. Consider using a more accurate message like "transaction pool close errors" or separately handling and reporting errors from different sources for better diagnostics.
| return fmt.Errorf("subpool close errors: %v", errs) | |
| return fmt.Errorf("transaction pool close errors: %v", errs) |
| if len(errs) > 0 { | ||
| return fmt.Errorf("subpool close errors: %v", errs) |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatting of multiple errors using fmt.Errorf with %v will produce a slice representation like "[error1 error2]" which is not as user-friendly as the errors.Join function which formats errors on separate lines. Consider using a loop with strings.Builder to format each error on its own line, or use a semicolon-separated format for better readability.
Proposed changes
Ref: ethereum#27523
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that