From 50b6d276a1ed449219be4afc0cb664a2cc3c0d26 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 18 Feb 2022 09:47:17 -0500 Subject: [PATCH 1/6] Add error code to MongoSystemError --- src/error.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/error.ts b/src/error.ts index cfd70f32aa..02170dcd87 100644 --- a/src/error.ts +++ b/src/error.ts @@ -625,6 +625,8 @@ export class MongoSystemError extends MongoError { if (reason) { this.reason = reason; } + + this.code = reason.error?.code; } get name(): string { From e1596c9bb7e9ad9591faad5c1e76520140129a8e Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 18 Feb 2022 10:13:45 -0500 Subject: [PATCH 2/6] Convert error tests to Typescript --- test/unit/{error.test.js => error.test.ts} | 62 +++++++++++----------- 1 file changed, 31 insertions(+), 31 deletions(-) rename test/unit/{error.test.js => error.test.ts} (91%) diff --git a/test/unit/error.test.js b/test/unit/error.test.ts similarity index 91% rename from test/unit/error.test.js rename to test/unit/error.test.ts index 134d309aa8..bb6fd384fe 100644 --- a/test/unit/error.test.js +++ b/test/unit/error.test.ts @@ -1,43 +1,43 @@ -/* eslint no-empty: ["error", { "allowEmptyCatch": true }] */ -'use strict'; - -const expect = require('chai').expect; -const mock = require('../tools/mongodb-mock/index'); -const { getSymbolFrom } = require('../tools/utils'); -const { ReplSetFixture } = require('../tools/common'); -const { ns, isHello } = require('../../src/utils'); -const { Topology } = require('../../src/sdam/topology'); -const { - MongoNetworkError, - MongoWriteConcernError, +// /* eslint no-empty: ["error", { "allowEmptyCatch": true }] */ + +import { expect } from 'chai'; + +import { + PoolClosedError as MongoPoolClosedError, + WaitQueueTimeoutError as MongoWaitQueueTimeoutError +} from '../../src/cmap/errors'; +import { + isRetryableEndTransactionError, + isSDAMUnrecoverableError, + LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE, + LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE, + NODE_IS_RECOVERING_ERROR_MESSAGE +} from '../../src/error'; +import { MongoError, + MongoNetworkError, + MongoParseError, MongoServerError, - MongoParseError -} = require('../../src/index'); -const { - LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE, - LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE, - NODE_IS_RECOVERING_ERROR_MESSAGE, - isRetryableEndTransactionError, - isSDAMUnrecoverableError -} = require('../../src/error'); -const { - PoolClosedError: MongoPoolClosedError, - WaitQueueTimeoutError: MongoWaitQueueTimeoutError -} = require('../../src/cmap/errors'); + MongoWriteConcernError +} from '../../src/index'; +import * as EverythingFromDriver from '../../src/index'; +import { Topology } from '../../src/sdam/topology'; +import { isHello, ns } from '../../src/utils'; +import { ReplSetFixture } from '../tools/common'; +import { cleanup } from '../tools/mongodb-mock/index'; +import { getSymbolFrom } from '../tools/utils'; describe('MongoErrors', () => { - // import errors as object let errorClasses = Object.fromEntries( - Object.entries(require('../../src/index')).filter(([key]) => key.endsWith('Error')) + Object.entries(EverythingFromDriver).filter(([key]) => key.endsWith('Error')) ); errorClasses = { ...errorClasses, MongoPoolClosedError, MongoWaitQueueTimeoutError }; - for (const errorName in errorClasses) { + for (const [errorName, errorClass] of Object.entries(errorClasses)) { describe(errorName, () => { it(`name should be read-only`, () => { // Dynamically create error class with message - let error = new errorClasses[errorName]('generated by test'); + const error = new errorClass('generated by test', {}); // expect name property to be class name expect(error).to.have.property('name', errorName); @@ -254,14 +254,14 @@ describe('MongoErrors', () => { }; before(() => (test = new ReplSetFixture())); - afterEach(() => mock.cleanup()); + afterEach(() => cleanup()); beforeEach(() => test.setup()); function makeAndConnectReplSet(cb) { let invoked = false; const replSet = new Topology( [test.primaryServer.hostAddress(), test.firstSecondaryServer.hostAddress()], - { replicaSet: 'rs' } + { replicaSet: 'rs' } as any ); replSet.once('error', err => { From 83cd18316c318bf84b55284f0b4a9a64571be9ab Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 22 Feb 2022 11:32:08 -0500 Subject: [PATCH 3/6] add test for error scenario --- test/unit/error.test.ts | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index bb6fd384fe..ff3511db2e 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -11,6 +11,7 @@ import { isSDAMUnrecoverableError, LEGACY_NOT_PRIMARY_OR_SECONDARY_ERROR_MESSAGE, LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE, + MongoSystemError, NODE_IS_RECOVERING_ERROR_MESSAGE } from '../../src/error'; import { @@ -30,19 +31,20 @@ import { getSymbolFrom } from '../tools/utils'; describe('MongoErrors', () => { let errorClasses = Object.fromEntries( Object.entries(EverythingFromDriver).filter(([key]) => key.endsWith('Error')) - ); + ) as any; errorClasses = { ...errorClasses, MongoPoolClosedError, MongoWaitQueueTimeoutError }; for (const [errorName, errorClass] of Object.entries(errorClasses)) { describe(errorName, () => { it(`name should be read-only`, () => { // Dynamically create error class with message - const error = new errorClass('generated by test', {}); + const error = new (errorClass as any)('generated by test', {}); // expect name property to be class name expect(error).to.have.property('name', errorName); try { error.name = 'renamed by test'; + // eslint-disable-next-line no-empty } catch (err) {} expect(error).to.have.property('name', errorName); }); @@ -89,6 +91,30 @@ describe('MongoErrors', () => { }); }); + describe('MongoSystemError#constructor', () => { + context('when the topology description contains a code', () => { + it('contains the code as a top level property', () => { + const topologyDescription = { + error: { + code: 123 + } + } as any as EverythingFromDriver.TopologyDescription; + + const error = new MongoSystemError('something went wrong', topologyDescription); + expect(error).to.haveOwnProperty('code', 123); + }); + }); + + context('when the topology description does not contain a code', () => { + it('contains the code as a top level property', () => { + const topologyDescription = {} as any as EverythingFromDriver.TopologyDescription; + + const error = new MongoSystemError('something went wrong', topologyDescription); + expect(error).to.haveOwnProperty('code', undefined); + }); + }); + }); + describe('#isRetryableEndTransactionError', function () { context('when the error has a RetryableWriteError label', function () { const error = new MongoNetworkError(''); @@ -202,7 +228,9 @@ describe('MongoErrors', () => { const errorWithOptionFalse = new MongoNetworkError('', { beforeHandshake: false }); expect(getSymbolFrom(errorWithOptionFalse, 'beforeHandshake', false)).to.be.a('symbol'); - const errorWithBadOption = new MongoNetworkError('', { beforeHandshake: 'not boolean' }); + const errorWithBadOption = new MongoNetworkError('', { + beforeHandshake: 'not boolean' as any + }); expect(getSymbolFrom(errorWithBadOption, 'beforeHandshake', false)).to.be.an('undefined'); const errorWithoutOption = new MongoNetworkError(''); From f934f64d9c7cb7a6c1e6b34e711cb2240981676e Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 22 Feb 2022 14:40:38 -0500 Subject: [PATCH 4/6] Refactor tests to align with https://github.com/mongodb/node-mongodb-native/pull/3144 --- src/index.ts | 1 + test/unit/error.test.ts | 59 ++++++++++++++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/index.ts b/src/index.ts index 27c8ac944c..0502109519 100644 --- a/src/index.ts +++ b/src/index.ts @@ -62,6 +62,7 @@ export { MongoServerError, MongoServerSelectionError, MongoSystemError, + MongoTailableCursorError, MongoTopologyClosedError, MongoTransactionError, MongoWriteConcernError diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index ff3511db2e..2639dc260f 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -14,29 +14,53 @@ import { MongoSystemError, NODE_IS_RECOVERING_ERROR_MESSAGE } from '../../src/error'; +import * as importsFromErrorSrc from '../../src/error'; import { MongoError, MongoNetworkError, MongoParseError, MongoServerError, - MongoWriteConcernError + MongoWriteConcernError, + TopologyDescription } from '../../src/index'; -import * as EverythingFromDriver from '../../src/index'; +import * as importsFromEntryPoint from '../../src/index'; import { Topology } from '../../src/sdam/topology'; -import { isHello, ns } from '../../src/utils'; +import { isHello, ns, setDifference } from '../../src/utils'; import { ReplSetFixture } from '../tools/common'; import { cleanup } from '../tools/mongodb-mock/index'; import { getSymbolFrom } from '../tools/utils'; describe('MongoErrors', () => { - let errorClasses = Object.fromEntries( - Object.entries(EverythingFromDriver).filter(([key]) => key.endsWith('Error')) + let errorClassesFromEntryPoint = Object.fromEntries( + Object.entries(importsFromEntryPoint).filter( + ([key, value]) => key.endsWith('Error') && value.toString().startsWith('class') + ) ) as any; - errorClasses = { ...errorClasses, MongoPoolClosedError, MongoWaitQueueTimeoutError }; + errorClassesFromEntryPoint = { + ...errorClassesFromEntryPoint, + MongoPoolClosedError, + MongoWaitQueueTimeoutError + }; + + const errorClassesFromErrorSrc = Object.fromEntries( + Object.entries(importsFromErrorSrc).filter( + ([key, value]) => key.endsWith('Error') && value.toString().startsWith('class') + ) + ); + + it('all defined errors should be public', () => { + expect( + setDifference(Object.keys(errorClassesFromEntryPoint), Object.keys(errorClassesFromErrorSrc)) + ).to.have.property('size', 3); + + expect( + setDifference(Object.keys(errorClassesFromErrorSrc), Object.keys(errorClassesFromEntryPoint)) + ).to.have.property('size', 0); + }); - for (const [errorName, errorClass] of Object.entries(errorClasses)) { - describe(errorName, () => { - it(`name should be read-only`, () => { + describe('error names should be read-only', () => { + for (const [errorName, errorClass] of Object.entries(errorClassesFromEntryPoint)) { + it(`${errorName} should be read-only`, () => { // Dynamically create error class with message const error = new (errorClass as any)('generated by test', {}); // expect name property to be class name @@ -48,8 +72,8 @@ describe('MongoErrors', () => { } catch (err) {} expect(error).to.have.property('name', errorName); }); - }); - } + } + }); describe('MongoError#constructor', () => { it('should accept a string', function () { @@ -98,7 +122,7 @@ describe('MongoErrors', () => { error: { code: 123 } - } as any as EverythingFromDriver.TopologyDescription; + } as any as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); expect(error).to.haveOwnProperty('code', 123); @@ -107,7 +131,16 @@ describe('MongoErrors', () => { context('when the topology description does not contain a code', () => { it('contains the code as a top level property', () => { - const topologyDescription = {} as any as EverythingFromDriver.TopologyDescription; + const topologyDescription = { error: {} } as any as TopologyDescription; + + const error = new MongoSystemError('something went wrong', topologyDescription); + expect(error).to.haveOwnProperty('code', undefined); + }); + }); + + context('when the topology description does not contain an error property', () => { + it('contains the code as a top level property', () => { + const topologyDescription = {} as any as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); expect(error).to.haveOwnProperty('code', undefined); From 1eda3f1d9bcbea15ffdb0ea821e91a4017aca4e1 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 22 Feb 2022 16:30:42 -0500 Subject: [PATCH 5/6] Address PR comments - use ts-expect-error instead of `as any` when purposely using an incorrect type - clean up new `it` block descriptions --- test/unit/error.test.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index 2639dc260f..9748067c69 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -1,5 +1,3 @@ -// /* eslint no-empty: ["error", { "allowEmptyCatch": true }] */ - import { expect } from 'chai'; import { @@ -116,8 +114,8 @@ describe('MongoErrors', () => { }); describe('MongoSystemError#constructor', () => { - context('when the topology description contains a code', () => { - it('contains the code as a top level property', () => { + context('when the topology description contains an error code', () => { + it('contains the specified code as a top level property', () => { const topologyDescription = { error: { code: 123 @@ -129,8 +127,8 @@ describe('MongoErrors', () => { }); }); - context('when the topology description does not contain a code', () => { - it('contains the code as a top level property', () => { + context('when the topology description does not contain an error code', () => { + it('contains the code as a top level property that is undefined', () => { const topologyDescription = { error: {} } as any as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); @@ -139,7 +137,7 @@ describe('MongoErrors', () => { }); context('when the topology description does not contain an error property', () => { - it('contains the code as a top level property', () => { + it('contains the code as a top level property that is undefined', () => { const topologyDescription = {} as any as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); @@ -262,7 +260,8 @@ describe('MongoErrors', () => { expect(getSymbolFrom(errorWithOptionFalse, 'beforeHandshake', false)).to.be.a('symbol'); const errorWithBadOption = new MongoNetworkError('', { - beforeHandshake: 'not boolean' as any + // @ts-expect-error: beforeHandshake must be a boolean value + beforeHandshake: 'not boolean' }); expect(getSymbolFrom(errorWithBadOption, 'beforeHandshake', false)).to.be.an('undefined'); From 950eb7b2e11a5214c19c8af639b20daaf0a08f7a Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 22 Feb 2022 17:04:35 -0500 Subject: [PATCH 6/6] remove 'as any' casts --- test/unit/error.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index 9748067c69..2bdb5f8479 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -22,7 +22,7 @@ import { TopologyDescription } from '../../src/index'; import * as importsFromEntryPoint from '../../src/index'; -import { Topology } from '../../src/sdam/topology'; +import { Topology, TopologyOptions } from '../../src/sdam/topology'; import { isHello, ns, setDifference } from '../../src/utils'; import { ReplSetFixture } from '../tools/common'; import { cleanup } from '../tools/mongodb-mock/index'; @@ -120,7 +120,7 @@ describe('MongoErrors', () => { error: { code: 123 } - } as any as TopologyDescription; + } as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); expect(error).to.haveOwnProperty('code', 123); @@ -129,7 +129,7 @@ describe('MongoErrors', () => { context('when the topology description does not contain an error code', () => { it('contains the code as a top level property that is undefined', () => { - const topologyDescription = { error: {} } as any as TopologyDescription; + const topologyDescription = { error: {} } as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); expect(error).to.haveOwnProperty('code', undefined); @@ -138,7 +138,7 @@ describe('MongoErrors', () => { context('when the topology description does not contain an error property', () => { it('contains the code as a top level property that is undefined', () => { - const topologyDescription = {} as any as TopologyDescription; + const topologyDescription = {} as TopologyDescription; const error = new MongoSystemError('something went wrong', topologyDescription); expect(error).to.haveOwnProperty('code', undefined); @@ -321,7 +321,7 @@ describe('MongoErrors', () => { let invoked = false; const replSet = new Topology( [test.primaryServer.hostAddress(), test.firstSecondaryServer.hostAddress()], - { replicaSet: 'rs' } as any + { replicaSet: 'rs' } as TopologyOptions ); replSet.once('error', err => {