Example of a possible fix for indicating server errors #374
Conversation
| // IsServerErr returns true if the error was produced by the server. | ||
| func (e *Error) IsServerErr() bool { | ||
| return e.isServerErr | ||
| } |
There was a problem hiding this comment.
I'd prefer not to export this method - we've already got connect.IsServerErr.
There was a problem hiding this comment.
In that case the method isn't really needed because it's the same as inspecting the field
There was a problem hiding this comment.
Removed IsServerErr, can be reintroduced if we ever need it
error.go
Outdated
| } | ||
|
|
||
| // NewError annotates any Go error with a status code. | ||
| func NewError(c Code, underlying error) *Error { | ||
| return &Error{code: c, err: underlying} | ||
| } | ||
|
|
||
| // IsServerErr returns true if the error was produced by the server. |
There was a problem hiding this comment.
Let's rethink this name a little; IsServerErr suggests to me that we're categorizing errors into 4xx and 5xx. Perhaps IsFromWire?
Also, can we give some examples in the GoDoc? I think the idea is approachable, but a little subtle.
There was a problem hiding this comment.
I do like IsFromWire
There was a problem hiding this comment.
Although talking API design we probably want it to be IsWireErr because connect.IsFromWire doesn't give enough context
There was a problem hiding this comment.
field changed to wireErr bool and function changed to connect.IsWireErr(bool) for simplicity but also context around the fact that an error is still being checked
protocol_connect.go
Outdated
| @@ -478,6 +479,7 @@ func (cc *connectStreamingClientConn) Receive(msg any) error { | |||
| // For users to realize that the stream has ended, Receive must return an | |||
| // error. | |||
| serverErr.meta = cc.responseHeader.Clone() | |||
| serverErr.isServerErr = true | |||
There was a problem hiding this comment.
I think we can avoid both this change and the one above by handling this closer to the wire - let's add this code to connectWireError.asError instead.
Add `connect.IsWireErr`, so clients can distinguish synthesized from server-sent errors. Fixes #222.
Add
connect.IsWireErr, so clients can distinguish synthesized from server-sent errors.Fixes #222.