-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
chore: reorder block production log statements #6601
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #6601 +/- ##
=========================================
Coverage 61.49% 61.49%
=========================================
Files 556 556
Lines 58895 58903 +8
Branches 1856 1857 +1
=========================================
+ Hits 36216 36222 +6
- Misses 22638 22640 +2
Partials 41 41 |
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.
This seems to be a regression introduced in #6241, we should make sure to log the error somewhere.
Also noticed that the thrown error might be confusing for users that do not have builder enabled as it states both block failed to produce which is incorrect in that case
throw Error("Builder and engine both failed to produce the block"); |
Maybe should differentiate errors based on isEnabled booleans for a cleaner UX
Performance Report✔️ no performance regression detected Full benchmark results
|
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.
Ahh makes sense to log the reasons individually as well
@nflaig I addressed your comment |
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.
LGTM
🎉 This PR is included in v1.18.0 🎉 |
Motivation
Make sure logs are not discarded
Description
Logs can be discarded by an exception thrown beforehand. Reorder logic so that it doesn't happen.