From 78394e7025fd36b033ba694b34b40d030f95d977 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 8 Nov 2023 16:35:49 +0100 Subject: [PATCH 1/2] fix(NODE-4863): do not use RetryableWriteError for non-server errors --- src/cmap/errors.ts | 2 +- src/error.ts | 6 ++- .../non-server-retryable_writes.test.ts | 49 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 test/integration/retryable-writes/non-server-retryable_writes.test.ts diff --git a/src/cmap/errors.ts b/src/cmap/errors.ts index af550508e8..9d51aa02c8 100644 --- a/src/cmap/errors.ts +++ b/src/cmap/errors.ts @@ -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 { diff --git a/src/error.ts b/src/error.ts index 11203a9a9a..1969b8e2f8 100644 --- a/src/error.ts +++ b/src/error.ts @@ -92,6 +92,7 @@ export const MongoErrorLabel = Object.freeze({ ResumableChangeStreamError: 'ResumableChangeStreamError', HandshakeError: 'HandshakeError', ResetPool: 'ResetPool', + PoolRequstedRetry: 'PoolRequstedRetry', InterruptInUseConnections: 'InterruptInUseConnections', NoWritesPerformed: 'NoWritesPerformed' } as const); @@ -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 */ diff --git a/test/integration/retryable-writes/non-server-retryable_writes.test.ts b/test/integration/retryable-writes/non-server-retryable_writes.test.ts new file mode 100644 index 0000000000..7ef748f560 --- /dev/null +++ b/test/integration/retryable-writes/non-server-retryable_writes.test.ts @@ -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']); + } + ); +}); From 9a7ee7693c8b7214eb5e9db35712c1e33d3d49da Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 8 Nov 2023 17:47:01 +0100 Subject: [PATCH 2/2] fix: check for both labels --- src/error.ts | 2 +- src/sessions.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/error.ts b/src/error.ts index 1969b8e2f8..f53b15b0d7 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1163,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. diff --git a/src/sessions.ts b/src/sessions.ts index b4b59f232c..eafbc3c156 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -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, @@ -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) ) { @@ -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