Add natsProtoErr that can be used with errors.Is #1082
Conversation
Signed-off-by: Waldemar Quevedo <wally@nats.io>
| @@ -2377,8 +2390,7 @@ func (nc *Conn) sendConnect() error { | |||
| // in doReconnect()). | |||
| nc.processAuthError(authErr) | |||
| } | |||
|
|
|||
| return errors.New("nats: " + proto) | |||
| return &natsProtoErr{proto} | |||
There was a problem hiding this comment.
Sure, but have you tried adding a return authErr in the "if" statement above? Does that work? Point is that we may not need to do any conversions for the known errors handled in checkAuthError. Your changes is still valid I guess for all other cases?
There was a problem hiding this comment.
And by "does it work", I mean the user code could then do if err == nats.ErrAuthorization without even have to use the errors.Is().
There was a problem hiding this comment.
It does work but then I saw that we also tested for uppercase literal error in other tests like: https://github.com/nats-io/nats.go/blob/main/test/cluster_test.go#L192-L194
and recommendation from Go team is to start using errors.Is anyway instead of checking directly to check for wrapped errors as well.
The server can send both
-ERR 'Authorization Violation'or-ERR 'authorization violation'depending on some conditions so in the client side we usually try to normalize the behavior by changing to lowercase when checking for an error.One exception was during the initial connect where the client instead returned the raw error protocol message.
Since this behavior has been in the wild for many years, instead of returning the raw error or the proper error type like
ErrAuthorizationto keep things backwards compatible just in case, this adds a newnatsProtoErrthat can be used witherrors.Isto check whether the raw protocol message is equivalent to other errors.Fixes #1081
Signed-off-by: Waldemar Quevedo wally@nats.io