Throw AuthenticationException if failed to conn due to auth errors #315
Conversation
| @@ -81,7 +82,7 @@ public static TLSSocketChannel create( ByteChannel channel, Logger logger, SSLEn | |||
| } | |||
| catch ( SSLHandshakeException e ) | |||
| { | |||
| throw new ClientException( "Failed to establish secured connection with the server: " + e.getMessage(), e ); | |||
| throw new UnauthorizedException( "Failed to establish secured connection with the server: " + e.getMessage(), e ); | |||
There was a problem hiding this comment.
I'm not sure that we should reuse UnauthorizedException for TLS handshake failure as well as for actual login failure. The Python driver uses ServiceUnavailable here but that doesn't seem quite right either.
Perhaps we should have something separate such as SecurityLayerUnavailable?
There was a problem hiding this comment.
I would prefer to have fewer public errors. So how about change both of them to SecurityException/AuthorizationException?
There was a problem hiding this comment.
+1 for AuthorizationException
There was a problem hiding this comment.
I think the name is fine as-is for login failure but why would fewer errors be useful here? Login failure could trigger an application code path that requires the user to enter new login details whereas an SSL handshake failure should not.
There was a problem hiding this comment.
@zhenlineo changes look good, name of UnauthorizedException is probably the only thing that could be improved.
|
Will merge this one latter after the big routing specification change, as it should be easier to rebase this PR |
This includes the error that the client provide a wrong credentials and the server provides a wrong certificate.
…rity Changed from UnauthorizedException to AuthenticationException for wrong username/password Use SecurityException for failed to tls handshake
zhenlineo
force-pushed
the
1.1-auth-error
branch
from
ab35aab to
19b45fd
Compare
lutovich
changed the title
This includes the error that the client provide a wrong credentials and
the server provides a wrong certificate.