Skip to content

Commit

Permalink
fix(NODE-3688): remove wire version check
Browse files Browse the repository at this point in the history
  • Loading branch information
durran committed Apr 20, 2022
1 parent 52d9cf1 commit bb3e5ba
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 22 deletions.
5 changes: 1 addition & 4 deletions src/cmap/connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,7 @@ function performInitialHandshake(
}
provider.auth(authContext, err => {
if (err) {
if (
err instanceof MongoError &&
needsRetryableWriteLabel(err, response.maxWireVersion)
) {
if (err instanceof MongoError && needsRetryableWriteLabel(err)) {
err.addErrorLabel(MongoErrorLabel.RetryableWriteError);
}
return callback(err);
Expand Down
12 changes: 4 additions & 8 deletions src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,18 +743,14 @@ const RETRYABLE_WRITE_ERROR_CODES = new Set<number>([
MONGODB_ERROR_CODES.ExceededTimeLimit
]);

export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): boolean {
// pre-4.4 server, then the driver adds an error label for every valid case
// execute operation will only inspect the label, code/message logic is handled here
export function needsRetryableWriteLabel(error: Error): boolean {
// Network errors are always retryable.
if (error instanceof MongoNetworkError) {
return true;
}

if (
maxWireVersion >= 9 ||
(error instanceof MongoError && error.hasErrorLabel(MongoErrorLabel.RetryableWriteError))
) {
// If we already have the error label no need to add it again. 4.4+ servers add the label.
// We don't need to apply the label if it's already there.
if (error instanceof MongoError && error.hasErrorLabel(MongoErrorLabel.RetryableWriteError)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/sdam/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ function makeOperationHandler(
} else {
if (
(isRetryableWritesEnabled(server.s.topology) || isTransactionCommand(cmd)) &&
needsRetryableWriteLabel(error, maxWireVersion(server)) &&
needsRetryableWriteLabel(error) &&
!inActiveTransaction(session, cmd)
) {
error.addErrorLabel(MongoErrorLabel.RetryableWriteError);
Expand Down
5 changes: 1 addition & 4 deletions test/spec/retryable-writes/unified/handshakeError.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,7 @@
"saslContinue",
"ping"
],
"errorCode": 91,
"errorLabels": [
"RetryableWriteError"
]
"errorCode": 91
}
}
}
Expand Down
1 change: 0 additions & 1 deletion test/spec/retryable-writes/unified/handshakeError.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ tests:
data:
failCommands: [saslContinue, ping]
errorCode: 91 # ShutdownInProgress
errorLabels: ["RetryableWriteError"]
- name: runCommand
object: *database0
arguments:
Expand Down
8 changes: 4 additions & 4 deletions test/unit/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ describe('MongoErrors', () => {
maxWireVersion: BELOW_4_4
},
{
description: 'a MongoWriteConcernError with a retryable code above server 4.4',
result: false,
description: 'a MongoWriteConcernError with a retryable code but no label above 4.4',
result: true,
error: new MongoWriteConcernError({}, { code: 262 }),
maxWireVersion: ABOVE_4_4
},
Expand Down Expand Up @@ -485,9 +485,9 @@ describe('MongoErrors', () => {
maxWireVersion: ABOVE_4_4
}
];
for (const { description, result, error, maxWireVersion } of tests) {
for (const { description, result, error } of tests) {
it(`${description} ${result ? 'needs' : 'does not need'} a retryable write label`, () => {
expect(needsRetryableWriteLabel(error, maxWireVersion)).to.be.equal(result);
expect(needsRetryableWriteLabel(error)).to.be.equal(result);
});
}
});
Expand Down

0 comments on commit bb3e5ba

Please sign in to comment.