Skip to content

Commit

Permalink
fix(NODE-4863): do not use RetryableWriteError for non-server errors (#…
Browse files Browse the repository at this point in the history
…3914)

Co-authored-by: Durran Jordan <durran@gmail.com>
  • Loading branch information
alenakhineika and durran authored Nov 9, 2023
1 parent 54adc9f commit 08c9fb4
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/cmap/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class PoolClearedError extends MongoNetworkError {
super(errorMessage, pool.serverError ? { cause: pool.serverError } : undefined);
this.address = pool.address;

this.addErrorLabel(MongoErrorLabel.RetryableWriteError);
this.addErrorLabel(MongoErrorLabel.PoolRequstedRetry);
}

override get name(): string {
Expand Down
8 changes: 6 additions & 2 deletions src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export const MongoErrorLabel = Object.freeze({
ResumableChangeStreamError: 'ResumableChangeStreamError',
HandshakeError: 'HandshakeError',
ResetPool: 'ResetPool',
PoolRequstedRetry: 'PoolRequstedRetry',
InterruptInUseConnections: 'InterruptInUseConnections',
NoWritesPerformed: 'NoWritesPerformed'
} as const);
Expand Down Expand Up @@ -1162,7 +1163,7 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number):

if (error instanceof MongoError) {
if (
(maxWireVersion >= 9 || error.hasErrorLabel(MongoErrorLabel.RetryableWriteError)) &&
(maxWireVersion >= 9 || isRetryableWriteError(error)) &&
!error.hasErrorLabel(MongoErrorLabel.HandshakeError)
) {
// If we already have the error label no need to add it again. 4.4+ servers add the label.
Expand Down Expand Up @@ -1194,7 +1195,10 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number):
}

export function isRetryableWriteError(error: MongoError): boolean {
return error.hasErrorLabel(MongoErrorLabel.RetryableWriteError);
return (
error.hasErrorLabel(MongoErrorLabel.RetryableWriteError) ||
error.hasErrorLabel(MongoErrorLabel.PoolRequstedRetry)
);
}

/** Determines whether an error is something the driver should attempt to retry */
Expand Down
5 changes: 3 additions & 2 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { PINNED, UNPINNED } from './constants';
import type { AbstractCursor } from './cursor/abstract_cursor';
import {
type AnyError,
isRetryableWriteError,
MongoAPIError,
MongoCompatibilityError,
MONGODB_ERROR_CODES,
Expand Down Expand Up @@ -731,7 +732,7 @@ function endTransaction(
session.transaction.transition(TxnState.TRANSACTION_COMMITTED);
if (error instanceof MongoError) {
if (
error.hasErrorLabel(MongoErrorLabel.RetryableWriteError) ||
isRetryableWriteError(error) ||
error instanceof MongoWriteConcernError ||
isMaxTimeMSExpiredError(error)
) {
Expand Down Expand Up @@ -767,7 +768,7 @@ function endTransaction(
session.unpin();
}

if (error instanceof MongoError && error.hasErrorLabel(MongoErrorLabel.RetryableWriteError)) {
if (error instanceof MongoError && isRetryableWriteError(error)) {
// SPEC-1185: apply majority write concern when retrying commitTransaction
if (command.commitTransaction) {
// per txns spec, must unpin session in this case
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { expect } from 'chai';
import * as sinon from 'sinon';

import {
type Collection,
type MongoClient,
MongoWriteConcernError,
PoolClearedError,
Server
} from '../../mongodb';

describe('Non Server Retryable Writes', function () {
let client: MongoClient;
let collection: Collection<{ _id: 1 }>;

beforeEach(async function () {
client = this.configuration.newClient({ monitorCommands: true, retryWrites: true });
await client
.db()
.collection('retryReturnsOriginal')
.drop()
.catch(() => null);
collection = client.db().collection('retryReturnsOriginal');
});

afterEach(async function () {
sinon.restore();
await client.close();
});

it(
'returns the original error with a PoolRequstedRetry label after encountering a WriteConcernError',
{ requires: { topology: 'replicaset', mongodb: '>=4.2.9' } },
async () => {
const serverCommandStub = sinon.stub(Server.prototype, 'command');
serverCommandStub.onCall(0).yieldsRight(new PoolClearedError('error'));
serverCommandStub
.onCall(1)
.yieldsRight(
new MongoWriteConcernError({ errorLabels: ['NoWritesPerformed'], errorCode: 10107 }, {})
);

const insertResult = await collection.insertOne({ _id: 1 }).catch(error => error);
sinon.restore();

expect(insertResult.errorLabels).to.be.deep.equal(['PoolRequstedRetry']);
}
);
});

0 comments on commit 08c9fb4

Please sign in to comment.