-
Notifications
You must be signed in to change notification settings - Fork 84
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
core: addressed some panic instances #2783
Conversation
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2783 +/- ##
==========================================
- Coverage 53.29% 53.26% -0.03%
==========================================
Files 199 199
Lines 27604 27602 -2
==========================================
- Hits 14711 14703 -8
- Misses 11073 11078 +5
- Partials 1820 1821 +1 ☔ View full report in Codecov by Sentry. |
// TODO(corver): This can panic due to json unmarshalling unexpected data. | ||
// For now, it is a good way to catch compatibility issues. But we should | ||
// recover panics and return an error before launching mainnet. | ||
func ParSignedDataFromProto(typ DutyType, data *pbv1.ParSignedData) (odata ParSignedData, oerr error) { |
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.
where is odata
returned?
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.
nowhere, but I think I cannot omit one while having names on the others..
or maybe I could just use _
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.
Yup, _
will do the trick.
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!
@@ -30,7 +30,8 @@ func StartDutyTrace(ctx context.Context, duty Duty, spanName string, opts ...tra | |||
ctx, outerSpan = tracer.Start(tracer.RootedCtx(ctx, traceID), fmt.Sprintf("core/duty.%s", strings.Title(duty.Type.String()))) | |||
ctx, innerSpan = tracer.Start(ctx, spanName, opts...) | |||
|
|||
outerSpan.SetAttributes(attribute.Int64("slot", safeInt64(duty.Slot))) | |||
slotStr := strconv.FormatUint(duty.Slot, 10) | |||
outerSpan.SetAttributes(attribute.String("slot", slotStr)) |
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.
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.
I don't think it should cause any ripple behaviors
Removed some unnecessary panic() calls and added recover() where appropriate. category: refactor ticket: none
Removed some unnecessary panic() calls and added recover() where appropriate.
category: refactor
ticket: none