From 6712fbed43bf0c3e064cf86fc73393daf7c07e47 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 29 Jul 2021 16:26:42 -0400 Subject: [PATCH 01/28] refactor(NODE-3337): unwrap-WriteConcernError --- src/bulk/common.ts | 65 +++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index ef1eabefffb..ace58a75eb9 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -309,9 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError( - new MongoServerError({ errmsg: errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed }) - ); + return new WriteConcernError(errmsg, MONGODB_ERROR_CODES.WriteConcernFailed); } } @@ -333,29 +331,58 @@ export class BulkWriteResult { * @public * @category Error */ +// export class WriteConcernError { +// err: MongoServerError; +// code: number; + +// constructor(err: MongoServerError) { +// this.err = err; +// } + +// /** Write concern error code. */ +// get code(): number | undefined { +// return this.err.code; +// } + +// /** Write concern error message. */ +// get errmsg(): string { +// return this.err.errmsg; +// } + +// toJSON(): { code?: number; errmsg: string } { +// return { code: this.err.code, errmsg: this.err.errmsg }; +// } + +// toString(): string { +// return `WriteConcernError(${this.err.errmsg})`; +// } +// } + export class WriteConcernError { - err: MongoServerError; + _errmsg: string; + _code: number; - constructor(err: MongoServerError) { - this.err = err; + constructor(errmsg: string, code: number) { + this._errmsg = errmsg; + this._code = code; } /** Write concern error code. */ - get code(): number | undefined { - return this.err.code; + get code(): number { + return this._code; } /** Write concern error message. */ get errmsg(): string { - return this.err.errmsg; + return this._errmsg; } - toJSON(): { code?: number; errmsg: string } { - return { code: this.err.code, errmsg: this.err.errmsg }; + toJSON(): { code: number; errmsg: string } { + return { code: this.code, errmsg: this.errmsg }; } toString(): string { - return `WriteConcernError(${this.err.errmsg})`; + return `WriteConcernError(${this._errmsg})`; } } @@ -657,16 +684,12 @@ function handleMongoWriteConcernError( mergeBatchResults(batch, bulkResult, undefined, err.result); // TODO: Remove multiple levels of wrapping (NODE-3337) - const wrappedWriteConcernError = new WriteConcernError( - new MongoServerError({ - errmsg: err.result?.writeConcernError.errmsg, - code: err.result?.writeConcernError.result - }) - ); - callback( new MongoBulkWriteError( - new MongoServerError(wrappedWriteConcernError), + new WriteConcernError( + err.result?.writeConcernError.errmsg, + err.result?.writeConcernError.result + ), new BulkWriteResult(bulkResult) ) ); @@ -681,7 +704,7 @@ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError, result: BulkWriteResult) { + constructor(error: AnyError | WriteConcernError, result: BulkWriteResult) { super(error as Error); Object.assign(this, error); From 992c3c785c96fa39246e1999eebad0a7647648bc Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 29 Jul 2021 21:44:06 -0400 Subject: [PATCH 02/28] refactor(NODE-3337): potential unwrapping solution --- src/bulk/common.ts | 17 ++++++++++------- test/unit/core/write_concern_error.test.js | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index ace58a75eb9..f1efef5e481 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -333,7 +333,6 @@ export class BulkWriteResult { */ // export class WriteConcernError { // err: MongoServerError; -// code: number; // constructor(err: MongoServerError) { // this.err = err; @@ -359,8 +358,9 @@ export class BulkWriteResult { // } export class WriteConcernError { - _errmsg: string; - _code: number; + _errmsg?: string; + _code?: number; + _errInfo?: Document; constructor(errmsg: string, code: number) { this._errmsg = errmsg; @@ -704,8 +704,11 @@ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError | WriteConcernError, result: BulkWriteResult) { + constructor(error: AnyError | WriteConcernError | any, result: BulkWriteResult) { super(error as Error); + // for (const name in error) { + // (this as any)[name] = error[name]; + // } Object.assign(this, error); this.result = result; @@ -1248,11 +1251,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new MongoServerError({ + { message: msg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors - }), + }, writeResult ) ); @@ -1262,7 +1265,7 @@ export abstract class BulkOperationBase { const writeConcernError = writeResult.getWriteConcernError(); if (writeConcernError) { - callback(new MongoBulkWriteError(new MongoServerError(writeConcernError), writeResult)); + callback(new MongoBulkWriteError(writeConcernError, writeResult)); return true; } } diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index f5a14f3a769..a835e1fe29e 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -194,7 +194,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertOne({ x: /not a string/ }); + await collection.insertMany([{ x: /not a string/ }]); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoServerError); From 2269004dba3303cdfa3910a7fe2355996e1236cb Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 2 Aug 2021 13:21:45 -0400 Subject: [PATCH 03/28] refactor(NODE-3337): remove WriteConcernError wrapping --- src/bulk/common.ts | 75 ++++++++-------------- src/error.ts | 27 ++++---- src/operations/insert.ts | 4 +- test/unit/core/write_concern_error.test.js | 5 +- 4 files changed, 41 insertions(+), 70 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index f1efef5e481..91fd060a6f5 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -309,7 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError(errmsg, MONGODB_ERROR_CODES.WriteConcernFailed); + return new WriteConcernError(MONGODB_ERROR_CODES.WriteConcernFailed, errmsg); } } @@ -331,58 +331,38 @@ export class BulkWriteResult { * @public * @category Error */ -// export class WriteConcernError { -// err: MongoServerError; - -// constructor(err: MongoServerError) { -// this.err = err; -// } - -// /** Write concern error code. */ -// get code(): number | undefined { -// return this.err.code; -// } - -// /** Write concern error message. */ -// get errmsg(): string { -// return this.err.errmsg; -// } - -// toJSON(): { code?: number; errmsg: string } { -// return { code: this.err.code, errmsg: this.err.errmsg }; -// } - -// toString(): string { -// return `WriteConcernError(${this.err.errmsg})`; -// } -// } - export class WriteConcernError { - _errmsg?: string; - _code?: number; - _errInfo?: Document; + errCode?: number; + errMsg?: string; + errDetails?: Document; - constructor(errmsg: string, code: number) { - this._errmsg = errmsg; - this._code = code; + constructor(code?: number, errmsg?: string, errInfo?: Document) { + this.errMsg = errmsg; + this.errCode = code; + this.errDetails = errInfo; } /** Write concern error code. */ - get code(): number { - return this._code; + get code(): number | undefined { + return this.errCode; } /** Write concern error message. */ - get errmsg(): string { - return this._errmsg; + get errmsg(): string | undefined { + return this.errMsg; } - toJSON(): { code: number; errmsg: string } { - return { code: this.code, errmsg: this.errmsg }; + /** Write concern error info. */ + get errInfo(): Document | undefined { + return this.errDetails; + } + + toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { + return { code: this.code, errmsg: this.errmsg, errInfo: this.errInfo }; } toString(): string { - return `WriteConcernError(${this._errmsg})`; + return `WriteConcernError(${this.errmsg})`; } } @@ -571,7 +551,7 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push(new WriteConcernError(result.writeConcernError)); + bulkResult.writeConcernErrors.push(new WriteConcernError({ ...result.writeConcernError })); } } @@ -702,15 +682,12 @@ function handleMongoWriteConcernError( */ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; + err: WriteConcernError | AnyError | any; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError | WriteConcernError | any, result: BulkWriteResult) { + constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { super(error as Error); - // for (const name in error) { - // (this as any)[name] = error[name]; - // } - Object.assign(this, error); - + this.err = error; this.result = result; } @@ -1251,11 +1228,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - { + new MongoServerError({ message: msg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors - }, + }), writeResult ) ); diff --git a/src/error.ts b/src/error.ts index 4ca00ad1838..566141e2c19 100644 --- a/src/error.ts +++ b/src/error.ts @@ -84,7 +84,6 @@ export class MongoError extends Error { constructor(message: string | Error) { if (message instanceof Error) { super(message.message); - this.stack = message.stack; } else { super(message); } @@ -137,22 +136,18 @@ export class MongoServerError extends MongoError { codeName?: string; writeConcernError?: Document; - constructor(message: Error | ErrorDescription) { - if (message instanceof Error) { - super(message); - } else { - super(message.message || message.errmsg || message.$err || 'n/a'); - if (message.errorLabels) { - this[kErrorLabels] = new Set(message.errorLabels); - } - - for (const name in message) { - if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - continue; - } + constructor(message: ErrorDescription) { + super(message.message || message.errmsg || message.$err || 'n/a'); + if (message.errorLabels) { + this[kErrorLabels] = new Set(message.errorLabels); + } - (this as any)[name] = message[name]; + for (const name in message) { + if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { + continue; } + + (this as any)[name] = message[name]; } } @@ -683,7 +678,7 @@ export class MongoWriteConcernError extends MongoServerError { /** The result document (provided if ok: 1) */ result?: Document; - constructor(message: ErrorDescription, result: Document) { + constructor(message: ErrorDescription, result?: Document) { if (result && Array.isArray(result.errorLabels)) { message.errorLabels = result.errorLabels; } diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 726cb921bc7..ae9bf614e5b 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -1,4 +1,4 @@ -import { MongoServerError, MongoInvalidArgumentError } from '../error'; +import { MongoServerError, MongoInvalidArgumentError, MongoWriteConcernError } from '../error'; import { defineAspects, Aspect, AbstractOperation } from './operation'; import { CommandOperation, CommandOperationOptions } from './command'; import { prepareDocs } from './common_functions'; @@ -70,7 +70,7 @@ export class InsertOneOperation extends InsertOperation { super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); - if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); + if (res.writeErrors) return callback(new MongoWriteConcernError(res.writeErrors[0])); callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index a835e1fe29e..ef18c75214f 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -6,7 +6,6 @@ const { MongoWriteConcernError } = require('../../../src/error'); const { expect } = require('chai'); const { ns } = require('../../../src/utils'); const { once } = require('events'); -const { MongoServerError } = require('../../../src'); describe('WriteConcernError', function () { let test; @@ -194,10 +193,10 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertMany([{ x: /not a string/ }]); + await collection.insertOne({ x: /not a string/ }); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { - expect(error).to.be.instanceOf(MongoServerError); + expect(error).to.be.instanceOf(MongoWriteConcernError); expect(error).to.have.property('code', 121); expect(error).to.have.property('errInfo').that.is.an('object'); errInfoFromError = error.errInfo; From d43977c33e39542255eb4192200a34cc876b7787 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 3 Aug 2021 15:13:54 -0400 Subject: [PATCH 04/28] refactor(NODE-3337): update more instances of WriteConcernError --- src/bulk/common.ts | 20 +++++++++++++------- src/error.ts | 1 - test/unit/core/write_concern_error.test.js | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 91fd060a6f5..460a18493c1 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -551,7 +551,13 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push(new WriteConcernError({ ...result.writeConcernError })); + bulkResult.writeConcernErrors.push( + new WriteConcernError( + result.writeConcernError.code, + result.writeConcernError.errmsg, + result.writeConcernError.errInfo + ) + ); } } @@ -687,7 +693,7 @@ export class MongoBulkWriteError extends MongoServerError { /** Creates a new MongoBulkWriteError */ constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { super(error as Error); - this.err = error; + Object.assign(this, error); this.result = result; } @@ -1228,11 +1234,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new MongoServerError({ - message: msg, - code: this.s.bulkResult.writeErrors[0].code, - writeErrors: this.s.bulkResult.writeErrors - }), + new WriteConcernError( + this.s.bulkResult.writeErrors[0].code, + msg, + this.s.bulkResult.writeErrors + ), writeResult ) ); diff --git a/src/error.ts b/src/error.ts index 566141e2c19..25bb7dd3596 100644 --- a/src/error.ts +++ b/src/error.ts @@ -134,7 +134,6 @@ export class MongoError extends Error { export class MongoServerError extends MongoError { code?: number; codeName?: string; - writeConcernError?: Document; constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index ef18c75214f..261858a3655 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -193,7 +193,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertOne({ x: /not a string/ }); + await collection.insertMany([{ x: /not a string/ }]); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoWriteConcernError); From 01fc75b93e09e3b82859750ec5862c2a329d3d73 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 16:38:23 -0400 Subject: [PATCH 05/28] refactor(NODE-3337): unwrap MongoBulkWriteError constructor --- src/bulk/common.ts | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 460a18493c1..8c8e3b329d4 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -26,7 +26,7 @@ import type { Collection } from '../collection'; import type { Topology } from '../sdam/topology'; import type { CommandOperationOptions, CollationOptions } from '../operations/command'; import type { Hint } from '../operations/operation'; -import type { Filter, OptionalId, UpdateFilter } from '../mongo_types'; +import type { Filter, OneOrMore, OptionalId, UpdateFilter } from '../mongo_types'; /** @public */ export const BatchType = Object.freeze({ @@ -309,7 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError(MONGODB_ERROR_CODES.WriteConcernFailed, errmsg); + return new WriteConcernError({ errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed }); } } @@ -672,10 +672,10 @@ function handleMongoWriteConcernError( // TODO: Remove multiple levels of wrapping (NODE-3337) callback( new MongoBulkWriteError( - new WriteConcernError( - err.result?.writeConcernError.errmsg, - err.result?.writeConcernError.result - ), + { + message: err.result?.writeConcernError.errmsg, + code: err.result?.writeConcernError.result + }, new BulkWriteResult(bulkResult) ) ); @@ -688,12 +688,22 @@ function handleMongoWriteConcernError( */ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; - err: WriteConcernError | AnyError | any; + writeErrors: OneOrMore = []; /** Creates a new MongoBulkWriteError */ - constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { - super(error as Error); - Object.assign(this, error); + constructor( + x: { message: string; code: number; writeErrors?: WriteError[] } | AnyError | WriteConcernError, + result: BulkWriteResult + ) { + super(x); + if (x instanceof Error) Object.assign(this, x); + else if (x instanceof WriteConcernError) this.err = x.err; + else { + this.message = x.message; + this.code = x.code; + this.writeErrors = x.writeErrors ?? []; + } + this.result = result; } @@ -1234,11 +1244,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new WriteConcernError( - this.s.bulkResult.writeErrors[0].code, - msg, - this.s.bulkResult.writeErrors - ), + { + message: msg, + code: this.s.bulkResult.writeErrors[0].code, + writeErrors: this.s.bulkResult.writeErrors + }, writeResult ) ); From 4f38794d6e6cb252a6fd6db85d24900a88b2c7b7 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 17:09:06 -0400 Subject: [PATCH 06/28] refactor(NODE-3337): unwrap WriteConcernError constructor --- src/bulk/common.ts | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 8c8e3b329d4..b5b3ffd0927 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -332,29 +332,25 @@ export class BulkWriteResult { * @category Error */ export class WriteConcernError { - errCode?: number; - errMsg?: string; - errDetails?: Document; + err: { code: number; errmsg: string; errInfo?: Document }; - constructor(code?: number, errmsg?: string, errInfo?: Document) { - this.errMsg = errmsg; - this.errCode = code; - this.errDetails = errInfo; + constructor(err: { code: number; errmsg: string; errInfo?: Document }) { + this.err = err; } /** Write concern error code. */ get code(): number | undefined { - return this.errCode; + return this.err.code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this.errMsg; + return this.err.errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this.errDetails; + return this.err.errInfo; } toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { @@ -551,13 +547,8 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push( - new WriteConcernError( - result.writeConcernError.code, - result.writeConcernError.errmsg, - result.writeConcernError.errInfo - ) - ); + const { code, errmsg, errInfo } = result.writeConcernError; + bulkResult.writeConcernErrors.push(new WriteConcernError({ code, errmsg, errInfo })); } } @@ -689,10 +680,11 @@ function handleMongoWriteConcernError( export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; writeErrors: OneOrMore = []; + err?: WriteConcernError; /** Creates a new MongoBulkWriteError */ constructor( - x: { message: string; code: number; writeErrors?: WriteError[] } | AnyError | WriteConcernError, + x: { message: string; code: number; writeErrors?: WriteError[] } | WriteConcernError | AnyError, result: BulkWriteResult ) { super(x); From 0fc0bbf10cd22b48a7b86b489ce9481f3a04f75f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 17:47:30 -0400 Subject: [PATCH 07/28] refactor(NODE-3337): update MongoBulkWriteError constructor --- src/bulk/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index b5b3ffd0927..d2f540da3a0 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -689,7 +689,7 @@ export class MongoBulkWriteError extends MongoServerError { ) { super(x); if (x instanceof Error) Object.assign(this, x); - else if (x instanceof WriteConcernError) this.err = x.err; + else if (x instanceof WriteConcernError) this.err = x; else { this.message = x.message; this.code = x.code; From ce1ab0a0ffea60f89e67a47160cb44a6a6a565c1 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 6 Aug 2021 10:07:05 -0400 Subject: [PATCH 08/28] test(NODE-3337): fix ts definitions test --- src/error.ts | 26 +++++++++++++++------- test/unit/core/write_concern_error.test.js | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/error.ts b/src/error.ts index 25bb7dd3596..d2bc6d11cdf 100644 --- a/src/error.ts +++ b/src/error.ts @@ -60,12 +60,12 @@ export const GET_MORE_RESUMABLE_CODES = new Set([ ]); /** @public */ -export interface ErrorDescription { +export interface ErrorDescription extends Document { message?: string; errmsg?: string; $err?: string; errorLabels?: string[]; - [key: string]: any; + errInfo?: Document; } /** @@ -134,6 +134,9 @@ export class MongoError extends Error { export class MongoServerError extends MongoError { code?: number; codeName?: string; + writeConcernError?: Document; + ok?: number; + // [key: string]: any; constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); @@ -141,13 +144,18 @@ export class MongoServerError extends MongoError { this[kErrorLabels] = new Set(message.errorLabels); } - for (const name in message) { - if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - continue; - } + this.code = message.code; + this.codeName = message.codeName; + this.writeConcernError = message.writeConcernError; + this.ok = message.ok; - (this as any)[name] = message[name]; - } + // for (const [name, value] of Object.entries(message)) { + // if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { + // continue; + // } + + // this[name] = value; + // } } get name(): string { @@ -676,6 +684,7 @@ function makeWriteConcernResultObject(input: any) { export class MongoWriteConcernError extends MongoServerError { /** The result document (provided if ok: 1) */ result?: Document; + errInfo?: Document; constructor(message: ErrorDescription, result?: Document) { if (result && Array.isArray(result.errorLabels)) { @@ -683,6 +692,7 @@ export class MongoWriteConcernError extends MongoServerError { } super(message); + this.errInfo = message.errInfo; if (result != null) { this.result = makeWriteConcernResultObject(result); diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index 261858a3655..ef18c75214f 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -193,7 +193,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertMany([{ x: /not a string/ }]); + await collection.insertOne({ x: /not a string/ }); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoWriteConcernError); From fa88f864ea61cc81c75caf869345d5026c07ade2 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 29 Jul 2021 16:26:42 -0400 Subject: [PATCH 09/28] refactor(NODE-3337): unwrap-WriteConcernError --- src/bulk/common.ts | 65 +++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index ef1eabefffb..ace58a75eb9 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -309,9 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError( - new MongoServerError({ errmsg: errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed }) - ); + return new WriteConcernError(errmsg, MONGODB_ERROR_CODES.WriteConcernFailed); } } @@ -333,29 +331,58 @@ export class BulkWriteResult { * @public * @category Error */ +// export class WriteConcernError { +// err: MongoServerError; +// code: number; + +// constructor(err: MongoServerError) { +// this.err = err; +// } + +// /** Write concern error code. */ +// get code(): number | undefined { +// return this.err.code; +// } + +// /** Write concern error message. */ +// get errmsg(): string { +// return this.err.errmsg; +// } + +// toJSON(): { code?: number; errmsg: string } { +// return { code: this.err.code, errmsg: this.err.errmsg }; +// } + +// toString(): string { +// return `WriteConcernError(${this.err.errmsg})`; +// } +// } + export class WriteConcernError { - err: MongoServerError; + _errmsg: string; + _code: number; - constructor(err: MongoServerError) { - this.err = err; + constructor(errmsg: string, code: number) { + this._errmsg = errmsg; + this._code = code; } /** Write concern error code. */ - get code(): number | undefined { - return this.err.code; + get code(): number { + return this._code; } /** Write concern error message. */ get errmsg(): string { - return this.err.errmsg; + return this._errmsg; } - toJSON(): { code?: number; errmsg: string } { - return { code: this.err.code, errmsg: this.err.errmsg }; + toJSON(): { code: number; errmsg: string } { + return { code: this.code, errmsg: this.errmsg }; } toString(): string { - return `WriteConcernError(${this.err.errmsg})`; + return `WriteConcernError(${this._errmsg})`; } } @@ -657,16 +684,12 @@ function handleMongoWriteConcernError( mergeBatchResults(batch, bulkResult, undefined, err.result); // TODO: Remove multiple levels of wrapping (NODE-3337) - const wrappedWriteConcernError = new WriteConcernError( - new MongoServerError({ - errmsg: err.result?.writeConcernError.errmsg, - code: err.result?.writeConcernError.result - }) - ); - callback( new MongoBulkWriteError( - new MongoServerError(wrappedWriteConcernError), + new WriteConcernError( + err.result?.writeConcernError.errmsg, + err.result?.writeConcernError.result + ), new BulkWriteResult(bulkResult) ) ); @@ -681,7 +704,7 @@ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError, result: BulkWriteResult) { + constructor(error: AnyError | WriteConcernError, result: BulkWriteResult) { super(error as Error); Object.assign(this, error); From 2f4e16e9b10c736521d776ca7257be78827d16b2 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 29 Jul 2021 21:44:06 -0400 Subject: [PATCH 10/28] refactor(NODE-3337): potential unwrapping solution --- src/bulk/common.ts | 17 ++++++++++------- test/unit/core/write_concern_error.test.js | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index ace58a75eb9..f1efef5e481 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -333,7 +333,6 @@ export class BulkWriteResult { */ // export class WriteConcernError { // err: MongoServerError; -// code: number; // constructor(err: MongoServerError) { // this.err = err; @@ -359,8 +358,9 @@ export class BulkWriteResult { // } export class WriteConcernError { - _errmsg: string; - _code: number; + _errmsg?: string; + _code?: number; + _errInfo?: Document; constructor(errmsg: string, code: number) { this._errmsg = errmsg; @@ -704,8 +704,11 @@ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError | WriteConcernError, result: BulkWriteResult) { + constructor(error: AnyError | WriteConcernError | any, result: BulkWriteResult) { super(error as Error); + // for (const name in error) { + // (this as any)[name] = error[name]; + // } Object.assign(this, error); this.result = result; @@ -1248,11 +1251,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new MongoServerError({ + { message: msg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors - }), + }, writeResult ) ); @@ -1262,7 +1265,7 @@ export abstract class BulkOperationBase { const writeConcernError = writeResult.getWriteConcernError(); if (writeConcernError) { - callback(new MongoBulkWriteError(new MongoServerError(writeConcernError), writeResult)); + callback(new MongoBulkWriteError(writeConcernError, writeResult)); return true; } } diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index f5a14f3a769..a835e1fe29e 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -194,7 +194,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertOne({ x: /not a string/ }); + await collection.insertMany([{ x: /not a string/ }]); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoServerError); From e1ce12ffdb31f46011ca80945f7d590377158798 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 2 Aug 2021 13:21:45 -0400 Subject: [PATCH 11/28] refactor(NODE-3337): remove WriteConcernError wrapping --- src/bulk/common.ts | 75 ++++++++-------------- src/error.ts | 27 ++++---- src/operations/insert.ts | 4 +- test/unit/core/write_concern_error.test.js | 5 +- 4 files changed, 41 insertions(+), 70 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index f1efef5e481..91fd060a6f5 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -309,7 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError(errmsg, MONGODB_ERROR_CODES.WriteConcernFailed); + return new WriteConcernError(MONGODB_ERROR_CODES.WriteConcernFailed, errmsg); } } @@ -331,58 +331,38 @@ export class BulkWriteResult { * @public * @category Error */ -// export class WriteConcernError { -// err: MongoServerError; - -// constructor(err: MongoServerError) { -// this.err = err; -// } - -// /** Write concern error code. */ -// get code(): number | undefined { -// return this.err.code; -// } - -// /** Write concern error message. */ -// get errmsg(): string { -// return this.err.errmsg; -// } - -// toJSON(): { code?: number; errmsg: string } { -// return { code: this.err.code, errmsg: this.err.errmsg }; -// } - -// toString(): string { -// return `WriteConcernError(${this.err.errmsg})`; -// } -// } - export class WriteConcernError { - _errmsg?: string; - _code?: number; - _errInfo?: Document; + errCode?: number; + errMsg?: string; + errDetails?: Document; - constructor(errmsg: string, code: number) { - this._errmsg = errmsg; - this._code = code; + constructor(code?: number, errmsg?: string, errInfo?: Document) { + this.errMsg = errmsg; + this.errCode = code; + this.errDetails = errInfo; } /** Write concern error code. */ - get code(): number { - return this._code; + get code(): number | undefined { + return this.errCode; } /** Write concern error message. */ - get errmsg(): string { - return this._errmsg; + get errmsg(): string | undefined { + return this.errMsg; } - toJSON(): { code: number; errmsg: string } { - return { code: this.code, errmsg: this.errmsg }; + /** Write concern error info. */ + get errInfo(): Document | undefined { + return this.errDetails; + } + + toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { + return { code: this.code, errmsg: this.errmsg, errInfo: this.errInfo }; } toString(): string { - return `WriteConcernError(${this._errmsg})`; + return `WriteConcernError(${this.errmsg})`; } } @@ -571,7 +551,7 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push(new WriteConcernError(result.writeConcernError)); + bulkResult.writeConcernErrors.push(new WriteConcernError({ ...result.writeConcernError })); } } @@ -702,15 +682,12 @@ function handleMongoWriteConcernError( */ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; + err: WriteConcernError | AnyError | any; /** Creates a new MongoBulkWriteError */ - constructor(error: AnyError | WriteConcernError | any, result: BulkWriteResult) { + constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { super(error as Error); - // for (const name in error) { - // (this as any)[name] = error[name]; - // } - Object.assign(this, error); - + this.err = error; this.result = result; } @@ -1251,11 +1228,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - { + new MongoServerError({ message: msg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors - }, + }), writeResult ) ); diff --git a/src/error.ts b/src/error.ts index 4ca00ad1838..566141e2c19 100644 --- a/src/error.ts +++ b/src/error.ts @@ -84,7 +84,6 @@ export class MongoError extends Error { constructor(message: string | Error) { if (message instanceof Error) { super(message.message); - this.stack = message.stack; } else { super(message); } @@ -137,22 +136,18 @@ export class MongoServerError extends MongoError { codeName?: string; writeConcernError?: Document; - constructor(message: Error | ErrorDescription) { - if (message instanceof Error) { - super(message); - } else { - super(message.message || message.errmsg || message.$err || 'n/a'); - if (message.errorLabels) { - this[kErrorLabels] = new Set(message.errorLabels); - } - - for (const name in message) { - if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - continue; - } + constructor(message: ErrorDescription) { + super(message.message || message.errmsg || message.$err || 'n/a'); + if (message.errorLabels) { + this[kErrorLabels] = new Set(message.errorLabels); + } - (this as any)[name] = message[name]; + for (const name in message) { + if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { + continue; } + + (this as any)[name] = message[name]; } } @@ -683,7 +678,7 @@ export class MongoWriteConcernError extends MongoServerError { /** The result document (provided if ok: 1) */ result?: Document; - constructor(message: ErrorDescription, result: Document) { + constructor(message: ErrorDescription, result?: Document) { if (result && Array.isArray(result.errorLabels)) { message.errorLabels = result.errorLabels; } diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 726cb921bc7..ae9bf614e5b 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -1,4 +1,4 @@ -import { MongoServerError, MongoInvalidArgumentError } from '../error'; +import { MongoServerError, MongoInvalidArgumentError, MongoWriteConcernError } from '../error'; import { defineAspects, Aspect, AbstractOperation } from './operation'; import { CommandOperation, CommandOperationOptions } from './command'; import { prepareDocs } from './common_functions'; @@ -70,7 +70,7 @@ export class InsertOneOperation extends InsertOperation { super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); - if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); + if (res.writeErrors) return callback(new MongoWriteConcernError(res.writeErrors[0])); callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index a835e1fe29e..ef18c75214f 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -6,7 +6,6 @@ const { MongoWriteConcernError } = require('../../../src/error'); const { expect } = require('chai'); const { ns } = require('../../../src/utils'); const { once } = require('events'); -const { MongoServerError } = require('../../../src'); describe('WriteConcernError', function () { let test; @@ -194,10 +193,10 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertMany([{ x: /not a string/ }]); + await collection.insertOne({ x: /not a string/ }); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { - expect(error).to.be.instanceOf(MongoServerError); + expect(error).to.be.instanceOf(MongoWriteConcernError); expect(error).to.have.property('code', 121); expect(error).to.have.property('errInfo').that.is.an('object'); errInfoFromError = error.errInfo; From d6a74304d6be64dc3f76219c0567d1ef7c836cdb Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 3 Aug 2021 15:13:54 -0400 Subject: [PATCH 12/28] refactor(NODE-3337): update more instances of WriteConcernError --- src/bulk/common.ts | 20 +++++++++++++------- src/error.ts | 1 - test/unit/core/write_concern_error.test.js | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 91fd060a6f5..460a18493c1 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -551,7 +551,13 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push(new WriteConcernError({ ...result.writeConcernError })); + bulkResult.writeConcernErrors.push( + new WriteConcernError( + result.writeConcernError.code, + result.writeConcernError.errmsg, + result.writeConcernError.errInfo + ) + ); } } @@ -687,7 +693,7 @@ export class MongoBulkWriteError extends MongoServerError { /** Creates a new MongoBulkWriteError */ constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { super(error as Error); - this.err = error; + Object.assign(this, error); this.result = result; } @@ -1228,11 +1234,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new MongoServerError({ - message: msg, - code: this.s.bulkResult.writeErrors[0].code, - writeErrors: this.s.bulkResult.writeErrors - }), + new WriteConcernError( + this.s.bulkResult.writeErrors[0].code, + msg, + this.s.bulkResult.writeErrors + ), writeResult ) ); diff --git a/src/error.ts b/src/error.ts index 566141e2c19..25bb7dd3596 100644 --- a/src/error.ts +++ b/src/error.ts @@ -134,7 +134,6 @@ export class MongoError extends Error { export class MongoServerError extends MongoError { code?: number; codeName?: string; - writeConcernError?: Document; constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index ef18c75214f..261858a3655 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -193,7 +193,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertOne({ x: /not a string/ }); + await collection.insertMany([{ x: /not a string/ }]); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoWriteConcernError); From 3df81f168d4861530ba9da48511f832387f7c899 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 16:38:23 -0400 Subject: [PATCH 13/28] refactor(NODE-3337): unwrap MongoBulkWriteError constructor --- src/bulk/common.ts | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 460a18493c1..8c8e3b329d4 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -26,7 +26,7 @@ import type { Collection } from '../collection'; import type { Topology } from '../sdam/topology'; import type { CommandOperationOptions, CollationOptions } from '../operations/command'; import type { Hint } from '../operations/operation'; -import type { Filter, OptionalId, UpdateFilter } from '../mongo_types'; +import type { Filter, OneOrMore, OptionalId, UpdateFilter } from '../mongo_types'; /** @public */ export const BatchType = Object.freeze({ @@ -309,7 +309,7 @@ export class BulkWriteResult { if (i === 0) errmsg = errmsg + ' and '; } - return new WriteConcernError(MONGODB_ERROR_CODES.WriteConcernFailed, errmsg); + return new WriteConcernError({ errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed }); } } @@ -672,10 +672,10 @@ function handleMongoWriteConcernError( // TODO: Remove multiple levels of wrapping (NODE-3337) callback( new MongoBulkWriteError( - new WriteConcernError( - err.result?.writeConcernError.errmsg, - err.result?.writeConcernError.result - ), + { + message: err.result?.writeConcernError.errmsg, + code: err.result?.writeConcernError.result + }, new BulkWriteResult(bulkResult) ) ); @@ -688,12 +688,22 @@ function handleMongoWriteConcernError( */ export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; - err: WriteConcernError | AnyError | any; + writeErrors: OneOrMore = []; /** Creates a new MongoBulkWriteError */ - constructor(error: WriteConcernError | AnyError | any, result: BulkWriteResult) { - super(error as Error); - Object.assign(this, error); + constructor( + x: { message: string; code: number; writeErrors?: WriteError[] } | AnyError | WriteConcernError, + result: BulkWriteResult + ) { + super(x); + if (x instanceof Error) Object.assign(this, x); + else if (x instanceof WriteConcernError) this.err = x.err; + else { + this.message = x.message; + this.code = x.code; + this.writeErrors = x.writeErrors ?? []; + } + this.result = result; } @@ -1234,11 +1244,11 @@ export abstract class BulkOperationBase { callback( new MongoBulkWriteError( - new WriteConcernError( - this.s.bulkResult.writeErrors[0].code, - msg, - this.s.bulkResult.writeErrors - ), + { + message: msg, + code: this.s.bulkResult.writeErrors[0].code, + writeErrors: this.s.bulkResult.writeErrors + }, writeResult ) ); From d177f29dcf5eb57dbfd7a753f4d7b77969ef466c Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 17:09:06 -0400 Subject: [PATCH 14/28] refactor(NODE-3337): unwrap WriteConcernError constructor --- src/bulk/common.ts | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 8c8e3b329d4..b5b3ffd0927 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -332,29 +332,25 @@ export class BulkWriteResult { * @category Error */ export class WriteConcernError { - errCode?: number; - errMsg?: string; - errDetails?: Document; + err: { code: number; errmsg: string; errInfo?: Document }; - constructor(code?: number, errmsg?: string, errInfo?: Document) { - this.errMsg = errmsg; - this.errCode = code; - this.errDetails = errInfo; + constructor(err: { code: number; errmsg: string; errInfo?: Document }) { + this.err = err; } /** Write concern error code. */ get code(): number | undefined { - return this.errCode; + return this.err.code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this.errMsg; + return this.err.errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this.errDetails; + return this.err.errInfo; } toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { @@ -551,13 +547,8 @@ function mergeBatchResults( } if (result.writeConcernError) { - bulkResult.writeConcernErrors.push( - new WriteConcernError( - result.writeConcernError.code, - result.writeConcernError.errmsg, - result.writeConcernError.errInfo - ) - ); + const { code, errmsg, errInfo } = result.writeConcernError; + bulkResult.writeConcernErrors.push(new WriteConcernError({ code, errmsg, errInfo })); } } @@ -689,10 +680,11 @@ function handleMongoWriteConcernError( export class MongoBulkWriteError extends MongoServerError { result: BulkWriteResult; writeErrors: OneOrMore = []; + err?: WriteConcernError; /** Creates a new MongoBulkWriteError */ constructor( - x: { message: string; code: number; writeErrors?: WriteError[] } | AnyError | WriteConcernError, + x: { message: string; code: number; writeErrors?: WriteError[] } | WriteConcernError | AnyError, result: BulkWriteResult ) { super(x); From d6cb94f58a5c8b2ccb3baa090b0127870044a6a1 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 4 Aug 2021 17:47:30 -0400 Subject: [PATCH 15/28] refactor(NODE-3337): update MongoBulkWriteError constructor --- src/bulk/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index b5b3ffd0927..d2f540da3a0 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -689,7 +689,7 @@ export class MongoBulkWriteError extends MongoServerError { ) { super(x); if (x instanceof Error) Object.assign(this, x); - else if (x instanceof WriteConcernError) this.err = x.err; + else if (x instanceof WriteConcernError) this.err = x; else { this.message = x.message; this.code = x.code; From 759e6c626c600566165d3c86234337e7dffb7209 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 6 Aug 2021 10:07:05 -0400 Subject: [PATCH 16/28] test(NODE-3337): fix ts definitions test --- src/error.ts | 26 +++++++++++++++------- test/unit/core/write_concern_error.test.js | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/error.ts b/src/error.ts index 25bb7dd3596..d2bc6d11cdf 100644 --- a/src/error.ts +++ b/src/error.ts @@ -60,12 +60,12 @@ export const GET_MORE_RESUMABLE_CODES = new Set([ ]); /** @public */ -export interface ErrorDescription { +export interface ErrorDescription extends Document { message?: string; errmsg?: string; $err?: string; errorLabels?: string[]; - [key: string]: any; + errInfo?: Document; } /** @@ -134,6 +134,9 @@ export class MongoError extends Error { export class MongoServerError extends MongoError { code?: number; codeName?: string; + writeConcernError?: Document; + ok?: number; + // [key: string]: any; constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); @@ -141,13 +144,18 @@ export class MongoServerError extends MongoError { this[kErrorLabels] = new Set(message.errorLabels); } - for (const name in message) { - if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - continue; - } + this.code = message.code; + this.codeName = message.codeName; + this.writeConcernError = message.writeConcernError; + this.ok = message.ok; - (this as any)[name] = message[name]; - } + // for (const [name, value] of Object.entries(message)) { + // if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { + // continue; + // } + + // this[name] = value; + // } } get name(): string { @@ -676,6 +684,7 @@ function makeWriteConcernResultObject(input: any) { export class MongoWriteConcernError extends MongoServerError { /** The result document (provided if ok: 1) */ result?: Document; + errInfo?: Document; constructor(message: ErrorDescription, result?: Document) { if (result && Array.isArray(result.errorLabels)) { @@ -683,6 +692,7 @@ export class MongoWriteConcernError extends MongoServerError { } super(message); + this.errInfo = message.errInfo; if (result != null) { this.result = makeWriteConcernResultObject(result); diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index 261858a3655..ef18c75214f 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -193,7 +193,7 @@ describe('WriteConcernError', function () { let errInfoFromError; try { - await collection.insertMany([{ x: /not a string/ }]); + await collection.insertOne({ x: /not a string/ }); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { expect(error).to.be.instanceOf(MongoWriteConcernError); From 687e4a97258f98a861f65126b3d230355c7cb895 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 6 Aug 2021 13:28:29 -0400 Subject: [PATCH 17/28] refactor(NODE-3337): add topologyVersion to MongoServerError --- src/error.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/error.ts b/src/error.ts index d2bc6d11cdf..d05fb19020d 100644 --- a/src/error.ts +++ b/src/error.ts @@ -136,6 +136,7 @@ export class MongoServerError extends MongoError { codeName?: string; writeConcernError?: Document; ok?: number; + topologyVersion?: TopologyVersion; // [key: string]: any; constructor(message: ErrorDescription) { @@ -148,8 +149,11 @@ export class MongoServerError extends MongoError { this.codeName = message.codeName; this.writeConcernError = message.writeConcernError; this.ok = message.ok; + this.topologyVersion = message.topologyVersion; // for (const [name, value] of Object.entries(message)) { + // // eslint-disable-next-line @typescript-eslint/no-var-requires + // console.log(require('util').inspect(this, { depth: Infinity })); // if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { // continue; // } From b5616f54b8b7ccc0c0fb404681d6475a46818f38 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 6 Aug 2021 16:41:35 -0400 Subject: [PATCH 18/28] refactor(NODE-3337): reintroduce loop in MongoServerError constructor --- src/error.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/error.ts b/src/error.ts index d05fb19020d..be61c6b10a0 100644 --- a/src/error.ts +++ b/src/error.ts @@ -137,7 +137,7 @@ export class MongoServerError extends MongoError { writeConcernError?: Document; ok?: number; topologyVersion?: TopologyVersion; - // [key: string]: any; + [key: string]: any; constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); @@ -151,15 +151,13 @@ export class MongoServerError extends MongoError { this.ok = message.ok; this.topologyVersion = message.topologyVersion; - // for (const [name, value] of Object.entries(message)) { - // // eslint-disable-next-line @typescript-eslint/no-var-requires - // console.log(require('util').inspect(this, { depth: Infinity })); - // if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - // continue; - // } + for (const [name, value] of Object.entries(message)) { + if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { + continue; + } - // this[name] = value; - // } + this[name] = value; + } } get name(): string { From 7914e291f905bcaebe21e5c8213092db5113e0f2 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Fri, 6 Aug 2021 17:33:53 -0400 Subject: [PATCH 19/28] refactor(NODE-3404): prevent double work in MongoServerError constructor --- src/error.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/error.ts b/src/error.ts index be61c6b10a0..d406aec7753 100644 --- a/src/error.ts +++ b/src/error.ts @@ -151,11 +151,17 @@ export class MongoServerError extends MongoError { this.ok = message.ok; this.topologyVersion = message.topologyVersion; - for (const [name, value] of Object.entries(message)) { - if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - continue; - } - + const currentProps = Object.getOwnPropertyNames(this); + const filteredProps = Object.entries(message).filter(([name]) => { + return ( + name !== 'errorLabels' && + name !== 'errmsg' && + name !== 'message' && + !currentProps.includes(name) + ); + }); + + for (const [name, value] of filteredProps) { this[name] = value; } } From 487c8b7bcffa5052f5147fb45199e360dc2d7b47 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 9 Aug 2021 12:25:28 -0400 Subject: [PATCH 20/28] fix(NODE-3337): fix duplicate declaration typo --- src/error.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/error.ts b/src/error.ts index 4fa83b7ba67..88ae4a93b58 100644 --- a/src/error.ts +++ b/src/error.ts @@ -159,19 +159,6 @@ export class MongoServerError extends MongoError { for (const name in message) { if (!propsToFilter.has(name)) this[name] = message[name]; } - - this.code = message.code; - this.codeName = message.codeName; - this.writeConcernError = message.writeConcernError; - this.ok = message.ok; - - // for (const [name, value] of Object.entries(message)) { - // if (name === 'errorLabels' || name === 'errmsg' || name === 'message') { - // continue; - // } - - // this[name] = value; - // } } get name(): string { From eb5e0c149eb28b19ca3710c2c4e4e136af438e49 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 9 Aug 2021 16:18:53 -0400 Subject: [PATCH 21/28] refactor(NODE-3337): revert incorrect usage of MongoWriteConcernError --- src/bulk/common.ts | 24 ++++++++++++---------- src/error.ts | 2 ++ src/operations/insert.ts | 4 ++-- test/unit/core/write_concern_error.test.js | 4 ++-- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index d2f540da3a0..2fe10c9c182 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -547,8 +547,7 @@ function mergeBatchResults( } if (result.writeConcernError) { - const { code, errmsg, errInfo } = result.writeConcernError; - bulkResult.writeConcernErrors.push(new WriteConcernError({ code, errmsg, errInfo })); + bulkResult.writeConcernErrors.push(new WriteConcernError(result.writeConcernError)); } } @@ -660,7 +659,6 @@ function handleMongoWriteConcernError( ) { mergeBatchResults(batch, bulkResult, undefined, err.result); - // TODO: Remove multiple levels of wrapping (NODE-3337) callback( new MongoBulkWriteError( { @@ -684,19 +682,23 @@ export class MongoBulkWriteError extends MongoServerError { /** Creates a new MongoBulkWriteError */ constructor( - x: { message: string; code: number; writeErrors?: WriteError[] } | WriteConcernError | AnyError, + error: + | { message: string; code: number; writeErrors?: WriteError[] } + | WriteConcernError + | AnyError, result: BulkWriteResult ) { - super(x); - if (x instanceof Error) Object.assign(this, x); - else if (x instanceof WriteConcernError) this.err = x; - else { - this.message = x.message; - this.code = x.code; - this.writeErrors = x.writeErrors ?? []; + super(error); + + if (error instanceof WriteConcernError) this.err = error; + else if (!(error instanceof Error)) { + this.message = error.message; + this.code = error.code; + this.writeErrors = error.writeErrors ?? []; } this.result = result; + Object.assign(this, error); } get name(): string { diff --git a/src/error.ts b/src/error.ts index 88ae4a93b58..31373457712 100644 --- a/src/error.ts +++ b/src/error.ts @@ -135,6 +135,7 @@ export class MongoServerError extends MongoError { code?: number; codeName?: string; writeConcernError?: Document; + errInfo?: Document; ok?: number; topologyVersion?: TopologyVersion; [key: string]: any; @@ -148,6 +149,7 @@ export class MongoServerError extends MongoError { this.code = message.code; this.codeName = message.codeName; this.writeConcernError = message.writeConcernError; + this.errInfo = message.errInfo; this.ok = message.ok; this.topologyVersion = message.topologyVersion; diff --git a/src/operations/insert.ts b/src/operations/insert.ts index ae9bf614e5b..726cb921bc7 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -1,4 +1,4 @@ -import { MongoServerError, MongoInvalidArgumentError, MongoWriteConcernError } from '../error'; +import { MongoServerError, MongoInvalidArgumentError } from '../error'; import { defineAspects, Aspect, AbstractOperation } from './operation'; import { CommandOperation, CommandOperationOptions } from './command'; import { prepareDocs } from './common_functions'; @@ -70,7 +70,7 @@ export class InsertOneOperation extends InsertOperation { super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); - if (res.writeErrors) return callback(new MongoWriteConcernError(res.writeErrors[0])); + if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, diff --git a/test/unit/core/write_concern_error.test.js b/test/unit/core/write_concern_error.test.js index ef18c75214f..e490c921162 100644 --- a/test/unit/core/write_concern_error.test.js +++ b/test/unit/core/write_concern_error.test.js @@ -2,7 +2,7 @@ const { Topology } = require('../../../src/sdam/topology'); const mock = require('../../tools/mock'); const { ReplSetFixture } = require('./common'); -const { MongoWriteConcernError } = require('../../../src/error'); +const { MongoServerError, MongoWriteConcernError } = require('../../../src/error'); const { expect } = require('chai'); const { ns } = require('../../../src/utils'); const { once } = require('events'); @@ -196,7 +196,7 @@ describe('WriteConcernError', function () { await collection.insertOne({ x: /not a string/ }); expect.fail('The insert should fail the validation that x must be a string'); } catch (error) { - expect(error).to.be.instanceOf(MongoWriteConcernError); + expect(error).to.be.instanceOf(MongoServerError); expect(error).to.have.property('code', 121); expect(error).to.have.property('errInfo').that.is.an('object'); errInfoFromError = error.errInfo; From d701150bc021fee12998f72ce7917f38d556f680 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 10 Aug 2021 12:02:32 -0400 Subject: [PATCH 22/28] docs(NODE-3337): add comment explaining MongoServerError usage --- src/operations/insert.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/operations/insert.ts b/src/operations/insert.ts index 726cb921bc7..10af5cee81a 100644 --- a/src/operations/insert.ts +++ b/src/operations/insert.ts @@ -70,7 +70,10 @@ export class InsertOneOperation extends InsertOperation { super.execute(server, session, (err, res) => { if (err || res == null) return callback(err); if (res.code) return callback(new MongoServerError(res)); - if (res.writeErrors) return callback(new MongoServerError(res.writeErrors[0])); + if (res.writeErrors) { + // This should be a WriteError but we can't change it now because of error hierarchy + return callback(new MongoServerError(res.writeErrors[0])); + } callback(undefined, { acknowledged: this.writeConcern?.w !== 0 ?? true, From 430e8ec9a1eedaaa441c7dfa1f27202b537a3c6c Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 16:42:50 -0400 Subject: [PATCH 23/28] refactor(NODE-3337): add symbol to WriteConcernError and revert MongoServerError --- src/bulk/common.ts | 26 ++++++++++++++++++++------ src/error.ts | 15 ++------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 2fe10c9c182..32c99525f7a 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -28,6 +28,9 @@ import type { CommandOperationOptions, CollationOptions } from '../operations/co import type { Hint } from '../operations/operation'; import type { Filter, OneOrMore, OptionalId, UpdateFilter } from '../mongo_types'; +/** @internal */ +const kErr = Symbol('err'); + /** @public */ export const BatchType = Object.freeze({ INSERT: 1, @@ -332,25 +335,36 @@ export class BulkWriteResult { * @category Error */ export class WriteConcernError { - err: { code: number; errmsg: string; errInfo?: Document }; + errCode: number; + errMsg: string; + errDetails?: Document; + [kErr]: { code: number; errmsg: string; errInfo?: Document }; - constructor(err: { code: number; errmsg: string; errInfo?: Document }) { - this.err = err; + constructor(error: { code: number; errmsg: string; errInfo?: Document }) { + this[kErr] = error; + this.errCode = error.code; + this.errMsg = error.errmsg; + this.errDetails = error.errInfo; } /** Write concern error code. */ get code(): number | undefined { - return this.err.code; + return this.code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this.err.errmsg; + return this.errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this.err.errInfo; + return this.errInfo; + } + + /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ + get err(): { code: number; errmsg: string; errInfo?: Document } { + return this[kErr]; } toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { diff --git a/src/error.ts b/src/error.ts index 31373457712..b44308111f3 100644 --- a/src/error.ts +++ b/src/error.ts @@ -146,20 +146,9 @@ export class MongoServerError extends MongoError { this[kErrorLabels] = new Set(message.errorLabels); } - this.code = message.code; - this.codeName = message.codeName; - this.writeConcernError = message.writeConcernError; - this.errInfo = message.errInfo; - this.ok = message.ok; - this.topologyVersion = message.topologyVersion; - - const propsToFilter = new Set(Object.getOwnPropertyNames(this)); - propsToFilter.add('errorLabels'); - propsToFilter.add('errmsg'); - propsToFilter.add('message'); - for (const name in message) { - if (!propsToFilter.has(name)) this[name] = message[name]; + if (name !== 'errorLabels' && name !== 'errmsg' && name !== 'message') + this[name] = message[name]; } } From 524ee39921b47832039600700ee4839366b03646 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 17:20:10 -0400 Subject: [PATCH 24/28] refactor(NODE-3337): remove explicit props in WriteConcernError --- src/bulk/common.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 32c99525f7a..b07e85cd2bc 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -335,31 +335,25 @@ export class BulkWriteResult { * @category Error */ export class WriteConcernError { - errCode: number; - errMsg: string; - errDetails?: Document; - [kErr]: { code: number; errmsg: string; errInfo?: Document }; + [kErr]: Document; constructor(error: { code: number; errmsg: string; errInfo?: Document }) { this[kErr] = error; - this.errCode = error.code; - this.errMsg = error.errmsg; - this.errDetails = error.errInfo; } /** Write concern error code. */ get code(): number | undefined { - return this.code; + return this[kErr].code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this.errmsg; + return this[kErr].errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this.errInfo; + return this[kErr].errInfo; } /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ From 3015451fa701a5e0127016820c4120a9b0f01ba7 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 17:51:17 -0400 Subject: [PATCH 25/28] refactor(NODE-3337): add WriteConcernErrorData interface --- src/bulk/common.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index b07e85cd2bc..af07fe72ffb 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -329,15 +329,23 @@ export class BulkWriteResult { } } +/** @internal */ +export interface WriteConcernErrorData { + code: number; + errmsg: string; + errInfo?: Document; +} + /** * An error representing a failure by the server to apply the requested write concern to the bulk operation. * @public * @category Error */ export class WriteConcernError { - [kErr]: Document; + [kErr]: WriteConcernErrorData; - constructor(error: { code: number; errmsg: string; errInfo?: Document }) { + /** @internal */ + constructor(error: WriteConcernErrorData) { this[kErr] = error; } @@ -357,12 +365,12 @@ export class WriteConcernError { } /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ - get err(): { code: number; errmsg: string; errInfo?: Document } { + get err(): WriteConcernErrorData { return this[kErr]; } - toJSON(): { code?: number; errmsg?: string; errInfo?: Document } { - return { code: this.code, errmsg: this.errmsg, errInfo: this.errInfo }; + toJSON(): WriteConcernErrorData { + return this[kErr]; } toString(): string { From 8b6ea51c400762f90a2358d41d91da9b4d30558e Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 20:05:23 -0400 Subject: [PATCH 26/28] style(NODE-3337): mark kServerError internal --- src/bulk/common.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index af07fe72ffb..e264df275c7 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -29,7 +29,7 @@ import type { Hint } from '../operations/operation'; import type { Filter, OneOrMore, OptionalId, UpdateFilter } from '../mongo_types'; /** @internal */ -const kErr = Symbol('err'); +const kServerError = Symbol('serverError'); /** @public */ export const BatchType = Object.freeze({ @@ -342,35 +342,36 @@ export interface WriteConcernErrorData { * @category Error */ export class WriteConcernError { - [kErr]: WriteConcernErrorData; + /** @internal */ + [kServerError]: WriteConcernErrorData; /** @internal */ constructor(error: WriteConcernErrorData) { - this[kErr] = error; + this[kServerError] = error; } /** Write concern error code. */ get code(): number | undefined { - return this[kErr].code; + return this[kServerError].code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this[kErr].errmsg; + return this[kServerError].errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this[kErr].errInfo; + return this[kServerError].errInfo; } /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ get err(): WriteConcernErrorData { - return this[kErr]; + return this[kServerError]; } toJSON(): WriteConcernErrorData { - return this[kErr]; + return this[kServerError]; } toString(): string { From f5a1a7d211a3116aeff76cf819708c2a7ad2b827 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 16 Aug 2021 11:23:51 -0400 Subject: [PATCH 27/28] refactor(NODE-3337): inline return typeof toJSON --- src/bulk/common.ts | 12 ++++++++++-- src/index.ts | 8 +++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index e264df275c7..65ba10bb116 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -365,12 +365,20 @@ export class WriteConcernError { return this[kServerError].errInfo; } - /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ + /** + * @internal + * @deprecated The `err` prop that contained a MongoServerError has been deprecated. + */ get err(): WriteConcernErrorData { return this[kServerError]; } - toJSON(): WriteConcernErrorData { + toJSON(): { + code: number; + errmsg: string; + errInfo?: Document; + } { + // WriteConcernErrorData has been inlined as the return to avoid making it public return this[kServerError]; } diff --git a/src/index.ts b/src/index.ts index 870bb63a117..1d85e38f72b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -360,7 +360,13 @@ export type { export type { W, WriteConcernOptions, WriteConcernSettings } from './write_concern'; export type { ExecutionResult } from './operations/execute_operation'; export type { InternalAbstractCursorOptions } from './cursor/abstract_cursor'; -export type { BulkOperationBase, BulkOperationPrivate, FindOperators, Batch } from './bulk/common'; +export type { + BulkOperationBase, + BulkOperationPrivate, + FindOperators, + Batch, + WriteConcernErrorData +} from './bulk/common'; export type { OrderedBulkOperation } from './bulk/ordered'; export type { UnorderedBulkOperation } from './bulk/unordered'; export type { Encrypter, EncrypterOptions } from './encrypter'; From 3b6f4917c61f2cf5a03d4a54840ffd18c5e2c763 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 16 Aug 2021 14:45:15 -0400 Subject: [PATCH 28/28] refactor(NODE-3337): make WriteConcernErrorData public --- src/bulk/common.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 65ba10bb116..6db513596a6 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -329,7 +329,7 @@ export class BulkWriteResult { } } -/** @internal */ +/** @public */ export interface WriteConcernErrorData { code: number; errmsg: string; @@ -345,7 +345,6 @@ export class WriteConcernError { /** @internal */ [kServerError]: WriteConcernErrorData; - /** @internal */ constructor(error: WriteConcernErrorData) { this[kServerError] = error; } @@ -365,20 +364,12 @@ export class WriteConcernError { return this[kServerError].errInfo; } - /** - * @internal - * @deprecated The `err` prop that contained a MongoServerError has been deprecated. - */ + /** @deprecated The `err` prop that contained a MongoServerError has been deprecated. */ get err(): WriteConcernErrorData { return this[kServerError]; } - toJSON(): { - code: number; - errmsg: string; - errInfo?: Document; - } { - // WriteConcernErrorData has been inlined as the return to avoid making it public + toJSON(): WriteConcernErrorData { return this[kServerError]; }