From 623b5fce5da07b7ccfc28bdd3167ed3f9c2518a0 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 25 Jul 2024 15:42:08 -0400 Subject: [PATCH 1/4] fix(NODE-6276): preserve top level error code MongoWriteConcernError --- src/bulk/common.ts | 4 ++-- src/error.ts | 37 ++++++++++++++++++++++------------ test/unit/error.test.ts | 32 ++++++++++++++++++++++++++++- test/unit/mongo_client.test.ts | 2 +- 4 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index ad0451e9ac1..bfee9ae8c25 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -616,8 +616,8 @@ function handleMongoWriteConcernError( callback( new MongoBulkWriteError( { - message: err.result?.writeConcernError.errmsg, - code: err.result?.writeConcernError.result + message: err?.result.writeConcernError.errmsg, + code: err?.result.writeConcernError.code }, new BulkWriteResult(bulkResult, isOrdered) ) diff --git a/src/error.ts b/src/error.ts index 938ce715ffe..f1d77ab2e18 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1158,6 +1158,22 @@ export class MongoServerSelectionError extends MongoSystemError { } } +/** + * The type of the result property of MongoWriteConcernError + * @public + */ +export interface WriteConcernErrorResult { + writeConcernError: { + code: number; + errmsg: string; + codeName?: string; + errInfo?: Document; + }; + ok: 0 | 1; + code?: number; + [x: string | number]: unknown; +} + /** * An error thrown when the server reports a writeConcernError * @public @@ -1165,7 +1181,7 @@ export class MongoServerSelectionError extends MongoSystemError { */ export class MongoWriteConcernError extends MongoServerError { /** The result document */ - result: Document; + result: WriteConcernErrorResult; /** * **Do not use this constructor!** @@ -1178,18 +1194,11 @@ export class MongoWriteConcernError extends MongoServerError { * * @public **/ - constructor(result: { - writeConcernError: { - code: number; - errmsg: string; - codeName?: string; - errInfo?: Document; - }; - errorLabels?: string[]; - }) { - super({ ...result, ...result.writeConcernError }); - this.errInfo = result.writeConcernError.errInfo; + constructor(result: WriteConcernErrorResult) { + super(result); + this.errInfo = result.writeConcernError?.errInfo; this.result = result; + this.code = result.code ? result.code : undefined; } override get name(): string { @@ -1237,7 +1246,9 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): } if (error instanceof MongoWriteConcernError) { - return RETRYABLE_WRITE_ERROR_CODES.has(error.result?.code ?? error.code ?? 0); + return RETRYABLE_WRITE_ERROR_CODES.has( + error.result.writeConcernError?.code ?? error?.code ?? 0 + ); } if (error instanceof MongoError && typeof error.code === 'number') { diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index c57ee71da6f..47aa6c35050 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -30,7 +30,8 @@ import { setDifference, type TopologyDescription, type TopologyOptions, - WaitQueueTimeoutError as MongoWaitQueueTimeoutError + WaitQueueTimeoutError as MongoWaitQueueTimeoutError, + WriteConcernErrorResult } from '../mongodb'; import { ReplSetFixture } from '../tools/common'; import { cleanup } from '../tools/mongodb-mock/index'; @@ -740,4 +741,33 @@ describe('MongoErrors', () => { }); }); }); + + describe('MongoWriteConcernError constructor', function () { + context('when no top-level code is provided and writeConcernError.code exists', function () { + it('error.code remains undefined', function () { + const res: WriteConcernErrorResult = { + writeConcernError: { + code: 81, // nested code + errmsg: 'fake msg' + }, + ok: 1 + }; + expect(new MongoWriteConcernError(res).code).to.equal(undefined); + }); + }); + context('when top-level code is provided and writeConcernError.code exists', function () { + it('error.code equals the top-level code', function () { + const topLevelCode = 10; + const res: WriteConcernErrorResult = { + writeConcernError: { + code: 81, // nested code + errmsg: 'fake msg' + }, + ok: 1, + code: topLevelCode + }; + expect(new MongoWriteConcernError(res).code).to.equal(topLevelCode); + }); + }); + }); }); diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index 79663a26034..c98bb5fea5e 100644 --- a/test/unit/mongo_client.test.ts +++ b/test/unit/mongo_client.test.ts @@ -737,7 +737,7 @@ describe('MongoClient', function () { expect(error).to.have.property('code', 'EBADNAME'); }); - it('srvServiceName should not error if it is greater than 15 characters as long as the DNS query limit is not surpassed', async () => { + it.only('srvServiceName should not error if it is greater than 15 characters as long as the DNS query limit is not surpassed', async () => { const options = parseOptions('mongodb+srv://localhost.a.com', { srvServiceName: 'a'.repeat(16) }); From 355f05d41adc55319265072123c9d7fe3baf3da5 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 25 Jul 2024 17:30:24 -0400 Subject: [PATCH 2/4] smaller diff small small diff snaller diff temp tests passing --- src/bulk/common.ts | 4 ++-- src/error.ts | 15 +++++++-------- src/index.ts | 3 ++- test/unit/error.test.ts | 13 ++++++------- test/unit/mongo_client.test.ts | 2 +- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index bfee9ae8c25..1a79dd6b1e2 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -616,8 +616,8 @@ function handleMongoWriteConcernError( callback( new MongoBulkWriteError( { - message: err?.result.writeConcernError.errmsg, - code: err?.result.writeConcernError.code + message: err.result.writeConcernError.errmsg, + code: err.result.writeConcernError.code }, new BulkWriteResult(bulkResult, isOrdered) ) diff --git a/src/error.ts b/src/error.ts index f1d77ab2e18..81a09a3f4f7 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1169,8 +1169,9 @@ export interface WriteConcernErrorResult { codeName?: string; errInfo?: Document; }; - ok: 0 | 1; + ok: number; code?: number; + errorLabels?: string[]; [x: string | number]: unknown; } @@ -1181,7 +1182,7 @@ export interface WriteConcernErrorResult { */ export class MongoWriteConcernError extends MongoServerError { /** The result document */ - result: WriteConcernErrorResult; + result: Document; /** * **Do not use this constructor!** @@ -1195,10 +1196,10 @@ export class MongoWriteConcernError extends MongoServerError { * @public **/ constructor(result: WriteConcernErrorResult) { - super(result); - this.errInfo = result.writeConcernError?.errInfo; + super({ ...result, ...result.writeConcernError }); + this.errInfo = result.writeConcernError.errInfo; this.result = result; - this.code = result.code ? result.code : undefined; + this.code = result.code ?? result.writeConcernError.code ?? undefined; } override get name(): string { @@ -1246,9 +1247,7 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): } if (error instanceof MongoWriteConcernError) { - return RETRYABLE_WRITE_ERROR_CODES.has( - error.result.writeConcernError?.code ?? error?.code ?? 0 - ); + return RETRYABLE_WRITE_ERROR_CODES.has(error.result?.code ?? error.code ?? 0); } if (error instanceof MongoError && typeof error.code === 'number') { diff --git a/src/index.ts b/src/index.ts index 8bf6c686174..0ba8f82c01b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -73,7 +73,8 @@ export { MongoTopologyClosedError, MongoTransactionError, MongoUnexpectedServerResponseError, - MongoWriteConcernError + MongoWriteConcernError, + WriteConcernErrorResult } from './error'; export { AbstractCursor, diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index 47aa6c35050..6bab40d0318 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -30,8 +30,7 @@ import { setDifference, type TopologyDescription, type TopologyOptions, - WaitQueueTimeoutError as MongoWaitQueueTimeoutError, - WriteConcernErrorResult + WaitQueueTimeoutError as MongoWaitQueueTimeoutError } from '../mongodb'; import { ReplSetFixture } from '../tools/common'; import { cleanup } from '../tools/mongodb-mock/index'; @@ -743,22 +742,22 @@ describe('MongoErrors', () => { }); describe('MongoWriteConcernError constructor', function () { - context('when no top-level code is provided and writeConcernError.code exists', function () { - it('error.code remains undefined', function () { - const res: WriteConcernErrorResult = { + context('when no top-level code is provided', function () { + it('error.code is set to writeConcernError.code', function () { + const res = { writeConcernError: { code: 81, // nested code errmsg: 'fake msg' }, ok: 1 }; - expect(new MongoWriteConcernError(res).code).to.equal(undefined); + expect(new MongoWriteConcernError(res).code).to.equal(81); }); }); context('when top-level code is provided and writeConcernError.code exists', function () { it('error.code equals the top-level code', function () { const topLevelCode = 10; - const res: WriteConcernErrorResult = { + const res = { writeConcernError: { code: 81, // nested code errmsg: 'fake msg' diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index c98bb5fea5e..79663a26034 100644 --- a/test/unit/mongo_client.test.ts +++ b/test/unit/mongo_client.test.ts @@ -737,7 +737,7 @@ describe('MongoClient', function () { expect(error).to.have.property('code', 'EBADNAME'); }); - it.only('srvServiceName should not error if it is greater than 15 characters as long as the DNS query limit is not surpassed', async () => { + it('srvServiceName should not error if it is greater than 15 characters as long as the DNS query limit is not surpassed', async () => { const options = parseOptions('mongodb+srv://localhost.a.com', { srvServiceName: 'a'.repeat(16) }); From 26e5640430da7a7dbbcac4f61ef822c88466ec49 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 25 Jul 2024 18:13:59 -0400 Subject: [PATCH 3/4] fix unit test --- test/unit/index.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 6509568c018..8dc43c9c748 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -108,6 +108,7 @@ const EXPECTED_EXPORTS = [ 'MongoTransactionError', 'MongoUnexpectedServerResponseError', 'MongoWriteConcernError', + 'WriteConcernErrorResult', 'ObjectId', 'OrderedBulkOperation', 'ProfilingLevel', From 10a66298d17d083faf4dfb6c382fbcd0a45e6804 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 26 Jul 2024 13:00:38 -0400 Subject: [PATCH 4/4] streamline code --- src/error.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/error.ts b/src/error.ts index 81a09a3f4f7..8ef6c82f554 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1196,10 +1196,9 @@ export class MongoWriteConcernError extends MongoServerError { * @public **/ constructor(result: WriteConcernErrorResult) { - super({ ...result, ...result.writeConcernError }); + super({ ...result.writeConcernError, ...result }); this.errInfo = result.writeConcernError.errInfo; this.result = result; - this.code = result.code ?? result.writeConcernError.code ?? undefined; } override get name(): string { @@ -1247,7 +1246,7 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): } if (error instanceof MongoWriteConcernError) { - return RETRYABLE_WRITE_ERROR_CODES.has(error.result?.code ?? error.code ?? 0); + return RETRYABLE_WRITE_ERROR_CODES.has(error.result.writeConcernError.code ?? error?.code ?? 0); } if (error instanceof MongoError && typeof error.code === 'number') {