Drivers should retry operations if connection handshake fails #885
Conversation
| @@ -542,7 +546,8 @@ static boolean shouldAttemptToRetryWrite(final RetryState retryState, final Thro | |||
| Throwable failure = attemptFailure instanceof ResourceSupplierInternalException ? attemptFailure.getCause() : attemptFailure; | |||
| boolean decision = false; | |||
| MongoException exceptionRetryableRegardlessOfCommand = null; | |||
| if (failure instanceof MongoConnectionPoolClearedException) { | |||
| if (failure instanceof MongoConnectionPoolClearedException | |||
| || (failure instanceof MongoSecurityException && failure.getCause() != null && isRetryableException(failure.getCause()))) { | |||
There was a problem hiding this comment.
In the test scenario, is the cause of the MongoSecurityException some sort of socket exception? If so, I have a feeling we are mis-categorizing a socket exception as a MongoSecurityException. I don't think that was the intention. In DefaultAuthenticator#wrapException, for example, only a user-non-found code is wrapped (which seems too restrictive, frankly). But in SaslAuthenticator#wrapException, essentially all exceptions are wrapped.
I see three possibilities:
- Leave the MongoSecurityException code as is, and leave this check as is
- Re-consider MongoSecurityException creation as part of this PR, and simplify this check
- Re-consider MongoSecurityException creation as part of a new Jira issue, and simplify this check as part of that work.
I prefer 2 or 3. What do you think?
There was a problem hiding this comment.
In the test scenario, is the cause of the MongoSecurityException some sort of socket exception?
Not necessary a socket exception, but socket exceptions are also among causes. Examples of the causes in tests are "com.mongodb.MongoSocketReadException: Prematurely reached end of stream", "com.mongodb.MongoNodeIsRecoveringException: Command failed with error 91 (ShutdownInProgress)".
we are mis-categorizing a socket exception as a MongoSecurityException. I don't think that was the intention
Currently the driver wraps everything related to the authentication activity in MongoSecurityException:
Exception, ending the list at this point is enough :)MongoCommandException, which may be unrelated to securitySaslException, which is anIOExceptionUnknownHostException, which is anIOExceptionGSSException(checked)LoginException(checked)
It is not clear to me whether MongoSecurityException was intended to be used as a wrapper for anything, including IO, that goes wrong during the authentication process. The exception is documented as "thrown when there is an error reported by the underlying client authentication mechanism". If we change what exceptions are wrapped with MongoSecurityException, we will not only have to come up with an approach of discriminating the aforementioned exceptions as security-related and not, but also need to decide what unchecked exception should be used to wrap checked exceptions mentioned above.
Given the above, I think that options 1 and 3 are the only viable ones. And since you'd like us to refactor exception handling related to authentication, I will create a ticket for this.
There was a problem hiding this comment.
JAVA-4354