-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Preventing panics of RecoverableError casts #2267
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4054,7 +4054,7 @@ type RecoverableError struct { | |
|
||
// NewRecoverableError is used to wrap an error and mark it as recoverable or | ||
// not. | ||
func NewRecoverableError(e error, recoverable bool) error { | ||
func NewRecoverableError(e error, recoverable bool) *RecoverableError { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, I changed this return value from So let's keep returning error here to avoid that issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the awesome explanation. What a nasty side effect. |
||
if e == nil { | ||
return nil | ||
} | ||
|
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.
Instead of omitting the type assertion, let's just check if error is nil and return early.
Otherwise we re-introduce a bug where
reply.Error
gets set even when the original errore
was nil!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 was going to do a type assertion before I thought I could just change the return type.
Will have shortly.