-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: retryable streaming server errors to be logged as debug #2019
Conversation
9bd1215
to
3d62ebe
Compare
internal/rpc/rpc.go
Outdated
@@ -240,7 +240,9 @@ func RetryStreamingServerStream[Req, Resp any]( | |||
} else { | |||
// Stream terminated; check if this was caused by an error | |||
err = stream.Err() | |||
logLevel = log.Warn | |||
if !useDebugErrorLevel(err) { |
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.
For readability, could we refactor this to logLevel = logLevelForError(err)
? That way, you don't need to mentally parse the !
and the variable scope/flow.
@@ -268,3 +268,12 @@ func RetryStreamingServerStream[Req, Resp any]( | |||
|
|||
} | |||
} | |||
|
|||
// useDebugErrorLevel indicates whether the specified error should be reported as a debug |
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.
nit: would you mind making a quick followup PR to fix this comment? Thanks! :)
// useDebugErrorLevel indicates whether the specified error should be reported as a debug | ||
// level log. | ||
func logLevelForError(err error) log.Level { | ||
if err != nil && strings.Contains(err.Error(), "connect: connection refused") { |
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.
It's better to use errors.Is
for this check, like: https://go.dev/play/p/M0X4W1wcSQn
Sorry I missed this earlier!
applying feedback from: #2019
fixes #2002
unexpected warnings caused by retryable errors - retryable errors will continue to be reported as debug.