worryg0d commented on code in PR #1936:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1936#discussion_r2965524105
##########
frame.go:
##########
@@ -759,10 +759,26 @@ func (f *framer) parseErrorFrame() (frame, error) {
return nil, err
}
return res, nil
- case ErrCodeInvalid, ErrCodeBootstrapping, ErrCodeConfig,
ErrCodeCredentials, ErrCodeOverloaded,
- ErrCodeProtocol, ErrCodeServer, ErrCodeSyntax, ErrCodeTruncate,
ErrCodeUnauthorized:
- // TODO(zariel): we should have some distinct types for these
errors
- return errD, nil
+ case ErrCodeOverloaded:
+ return &RequestErrOverloaded{errorFrame: errD}, nil
+ case ErrCodeBootstrapping:
+ return &RequestErrBootstrapping{errorFrame: errD}, nil
+ case ErrCodeInvalid:
+ return &RequestErrInvalid{errorFrame: errD}, nil
+ case ErrCodeConfig:
+ return &RequestErrConfig{errorFrame: errD}, nil
+ case ErrCodeCredentials:
+ return &RequestErrCredentials{errorFrame: errD}, nil
+ case ErrCodeProtocol:
+ return &RequestErrProtocol{errorFrame: errD}, nil
+ case ErrCodeServer:
+ return &RequestErrServer{errorFrame: errD}, nil
+ case ErrCodeSyntax:
+ return &RequestErrSyntax{errorFrame: errD}, nil
+ case ErrCodeTruncate:
+ return &RequestErrTruncate{errorFrame: errD}, nil
+ case ErrCodeUnauthorized:
+ return &RequestErrUnauthorized{errorFrame: errD}, nil
Review Comment:
I spent some time reworking the protocol negotiation mechanism and ended up
with this:
```go
// Checks if the error is protocol related and should be retried during
startup.
// It returns the frame that caused the error and whether the error should
be retried.
func (s *startupCoordinator) checkProtocolRelatedError(err error) bool {
var protocolErr *protocolError
if errors.As(err, &protocolErr) {
if _, supportedFrame := protocolErr.frame.(*supportedFrame);
supportedFrame {
// We can receive a supportedFrame wrapped in
protocolError from Conn.recv if the host responds to a 0 stream id.
// If we receive a supportedFrame then we know that the
host is not compatible with the protocol version, but it is reachable, so we
can retry
return true
} else if requestErr, ok := protocolErr.frame.(RequestError);
ok {
// Unwrapping underlying error frame to check if it is
protocol related
err = requestErr
}
}
switch err.(type) {
case *RequestErrServer, *RequestErrProtocol:
// If we receive an errorFrame with codes ErrCodeProtocol or
ErrCodeServer,
// then we should try to downgrade a protocol version, so do
not skip the host
return true
default:
// In any other case we should not retry as it means the host
is not reachable or some other error happened
return false
}
}
```
But I would agree that it should be handled outside of this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
