GODRIVER-3361 Improve connection error message. #2027
Conversation
API Change ReportNo changes found! |
qingyang-hu
force-pushed
the
godriver3361
branch
4 times, most recently
from
2678a3c to
e6fe071
Compare
qingyang-hu
force-pushed
the
godriver3361
branch
from
e6fe071 to
076f0df
Compare
x/mongo/driver/topology/errors.go
Outdated
| } | ||
| if e.Wrapped != nil { | ||
| return fmt.Sprintf("connection(%s) %s", e.ConnectionID, e.Wrapped.Error()) | ||
| if errors.Is(e.Wrapped, io.EOF) { | ||
| messages = append(messages, "socket was unexpectedly closed") |
There was a problem hiding this comment.
| messages = append(messages, "socket was unexpectedly closed") | |
| messages = append(messages, "connection closed unexpectedly by the other side") |
The ticket suggests "Wrap[ping] io.EOF with an additional error message, like 'connection closed unexpectedly by the other side'". The idea being that we let the user know nothing driver-side is responsible. When the driver closes a connection I would expect something like this: use of closed network connection. Here is a gist with both examples: https://gist.github.com/prestonvasquez/bbab8370950e9c98fa422b6b1ae1069a
| @@ -28,21 +32,28 @@ type ConnectionError struct { | |||
|
|
|||
| // Error implements the error interface. | |||
| func (e ConnectionError) Error() string { | |||
There was a problem hiding this comment.
Suggest adding a compile check for ConnectionError, I just noticed we don't do this:
var _ error = ConnectionError{}| messages = append(messages, "client timed out waiting for server response") | ||
| } else if err, ok := e.Wrapped.(net.Error); ok && err.Timeout() { | ||
| messages = append(messages, "client timed out waiting for server response") | ||
| } |
There was a problem hiding this comment.
the transformNetworkError function will wrap the error with context.DeadlineExceeded. Should we include that case here for safety?
GODRIVER-3361
Summary
Improve connection error message.
Background & Motivation