From edf6bf045016687ceca497d2d71dd8b0ecd21def Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 23 May 2023 14:02:48 -0600 Subject: [PATCH 1/6] asdf --- global.d.ts | 1 + src/error.ts | 18 ++++++++- test/integration/node-specific/errors.ts | 40 +++++++++++++++++++ .../runner/filters/node_version_filter.js | 26 ++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 test/integration/node-specific/errors.ts create mode 100644 test/tools/runner/filters/node_version_filter.js diff --git a/global.d.ts b/global.d.ts index 010e123336..7c8dc818cc 100644 --- a/global.d.ts +++ b/global.d.ts @@ -12,6 +12,7 @@ declare global { serverless?: 'forbid' | 'allow' | 'require'; auth?: 'enabled' | 'disabled'; idmsMockServer?: true; + nodejs?: string; }; sessions?: { diff --git a/src/error.ts b/src/error.ts index f40dfd5229..aaa55264d0 100644 --- a/src/error.ts +++ b/src/error.ts @@ -108,6 +108,16 @@ export interface ErrorDescription extends Document { errInfo?: Document; } +function isAggregateError(e: unknown): e is Error & { errors: Error[] } { + return ( + typeof e === 'object' && + e != null && + e instanceof Error && + 'errors' in e && + Array.isArray(e.errors) + ); +} + /** * @public * @category Error @@ -132,7 +142,13 @@ export class MongoError extends Error { constructor(message: string | Error) { if (message instanceof Error) { - super(message.message); + if (isAggregateError(message)) { + const combinedMessage = 'Aggregate Error: ' + message.errors.map(e => e.message).join(', '); + super(combinedMessage); + } else { + super(message.message); + } + this.cause = message; } else { super(message); diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts new file mode 100644 index 0000000000..8b229e7ac6 --- /dev/null +++ b/test/integration/node-specific/errors.ts @@ -0,0 +1,40 @@ +import { expect } from 'chai'; + +import { MongoClient, MongoError } from '../../mongodb'; + +describe('Error (Integration)', function () { + describe('AggregateErrors', function () { + it('constructs the message properly', { requires: { nodejs: '>=16' } }, () => { + for (const { errors, message } of [ + { errors: [], message: 'Aggregate Error: ' }, + { errors: [new Error('message 1')], message: 'Aggregate Error: message 1' }, + { + errors: [new Error('message 1'), new Error('message 2')], + message: 'Aggregate Error: message 1, message 2' + } + ]) { + const error = new AggregateError(errors); + const mongoError = new MongoError(error); + + expect( + mongoError.message, + `built the message properly with an array of ${errors.length} errors` + ).to.equal(message); + } + }); + + it('sets the aggregate error to the cause property', { requires: { nodejs: '>=16' } }, () => { + const error = new AggregateError([new Error('error 1')]); + const mongoError = new MongoError(error); + expect(mongoError.cause).to.equal(error); + }); + }); + + it('NODE-5296: handles aggregate errors from dns lookup', async function () { + const error = await MongoClient.connect('mongodb://localhost:27222', { + serverSelectionTimeoutMS: 1000 + }).catch(e => e); + expect(error).to.be.instanceOf(Error); + expect(error.message).to.match(/ECONNREFUSED/); + }); +}); diff --git a/test/tools/runner/filters/node_version_filter.js b/test/tools/runner/filters/node_version_filter.js new file mode 100644 index 0000000000..0c16139f71 --- /dev/null +++ b/test/tools/runner/filters/node_version_filter.js @@ -0,0 +1,26 @@ +'use strict'; + +const { satisfies } = require('semver'); + +/** + * Filter for specific nodejs versions + * + * example: + * metadata: { + * requires: { + * nodejs: '>=14' + * } + * } + */ +class NodeVersionFilter { + filter(test) { + const nodeVersionRange = test?.metadata?.requires?.nodejs; + if (!nodeVersionRange) { + return true; + } + + return satisfies(process.version, nodeVersionRange); + } +} + +module.exports = NodeVersionFilter; From 10b9f9ed2115ee5b55dbeba8ac220963f2612754 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 24 May 2023 07:32:07 -0600 Subject: [PATCH 2/6] improve assertion --- test/integration/node-specific/errors.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index 8b229e7ac6..1cd5635768 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import { inspect } from 'util'; import { MongoClient, MongoError } from '../../mongodb'; @@ -35,6 +36,6 @@ describe('Error (Integration)', function () { serverSelectionTimeoutMS: 1000 }).catch(e => e); expect(error).to.be.instanceOf(Error); - expect(error.message).to.match(/ECONNREFUSED/); + expect(error.message).not.to.be.empty; }); }); From 819e4723d15ff764822b04f428a9a37237b3d5fe Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 24 May 2023 07:35:03 -0600 Subject: [PATCH 3/6] remove unused import --- test/integration/node-specific/errors.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index 1cd5635768..5ba490f6ba 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -1,5 +1,4 @@ import { expect } from 'chai'; -import { inspect } from 'util'; import { MongoClient, MongoError } from '../../mongodb'; From 045606a331723464736a1fc417f5ac45089d5ac4 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 24 May 2023 11:55:10 -0600 Subject: [PATCH 4/6] add additional logic to the error constructor --- src/error.ts | 35 ++++++++++++------------ test/integration/node-specific/errors.ts | 15 ++++++++-- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/error.ts b/src/error.ts index aaa55264d0..e24722d0ef 100644 --- a/src/error.ts +++ b/src/error.ts @@ -108,14 +108,8 @@ export interface ErrorDescription extends Document { errInfo?: Document; } -function isAggregateError(e: unknown): e is Error & { errors: Error[] } { - return ( - typeof e === 'object' && - e != null && - e instanceof Error && - 'errors' in e && - Array.isArray(e.errors) - ); +function isAggregateError(e: unknown): e is { errors: Error[] } { + return typeof e === 'object' && e != null && 'errors' in e && Array.isArray(e.errors); } /** @@ -141,21 +135,28 @@ export class MongoError extends Error { cause?: Error; // depending on the node version, this may or may not exist on the base constructor(message: string | Error) { + super(MongoError.buildErrorMessage(message)); if (message instanceof Error) { - if (isAggregateError(message)) { - const combinedMessage = 'Aggregate Error: ' + message.errors.map(e => e.message).join(', '); - super(combinedMessage); - } else { - super(message.message); - } - this.cause = message; - } else { - super(message); } + this[kErrorLabels] = new Set(); } + /** @internal */ + private static buildErrorMessage(e: Error | string): string { + if (typeof e === 'string') { + return e; + } + if (isAggregateError(e) && e.message.length === 0) { + return e.errors.length === 0 + ? 'AggregateError: no suberrors have error messages. please check the `cause` property for more error information.' + : 'Aggregate Error: ' + e.errors.map(({ message }) => message).join(', '); + } + + return e.message; + } + override get name(): string { return 'MongoError'; } diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index 5ba490f6ba..a8bd89e3a0 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -2,11 +2,15 @@ import { expect } from 'chai'; import { MongoClient, MongoError } from '../../mongodb'; -describe('Error (Integration)', function () { +describe.only('Error (Integration)', function () { describe('AggregateErrors', function () { it('constructs the message properly', { requires: { nodejs: '>=16' } }, () => { for (const { errors, message } of [ - { errors: [], message: 'Aggregate Error: ' }, + { + errors: [], + message: + 'AggregateError: no suberrors have error messages. please check the `cause` property for more error information.' + }, { errors: [new Error('message 1')], message: 'Aggregate Error: message 1' }, { errors: [new Error('message 1'), new Error('message 2')], @@ -23,6 +27,13 @@ describe('Error (Integration)', function () { } }); + it('uses the message on AggregateErrors, if non-empty', () => { + const error = new AggregateError([new Error('non-empty')]); + error.message = 'custom error message'; + const mongoError = new MongoError(error); + expect(mongoError.message).to.equal('custom error message'); + }); + it('sets the aggregate error to the cause property', { requires: { nodejs: '>=16' } }, () => { const error = new AggregateError([new Error('error 1')]); const mongoError = new MongoError(error); From 8c913589f2ba0ff666ea394badff18a88795e3e2 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 24 May 2023 14:27:34 -0600 Subject: [PATCH 5/6] address comments --- src/error.ts | 8 +-- test/integration/node-specific/errors.ts | 63 +++++++++++++----------- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/error.ts b/src/error.ts index e24722d0ef..3357adfa77 100644 --- a/src/error.ts +++ b/src/error.ts @@ -108,8 +108,8 @@ export interface ErrorDescription extends Document { errInfo?: Document; } -function isAggregateError(e: unknown): e is { errors: Error[] } { - return typeof e === 'object' && e != null && 'errors' in e && Array.isArray(e.errors); +function isAggregateError(e: Error): e is Error & { errors: Error[] } { + return 'errors' in e && Array.isArray(e.errors); } /** @@ -150,8 +150,8 @@ export class MongoError extends Error { } if (isAggregateError(e) && e.message.length === 0) { return e.errors.length === 0 - ? 'AggregateError: no suberrors have error messages. please check the `cause` property for more error information.' - : 'Aggregate Error: ' + e.errors.map(({ message }) => message).join(', '); + ? 'AggregateError has an empty errors array. Please check the `cause` property for more information.' + : e.errors.map(({ message }) => message).join(', '); } return e.message; diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index a8bd89e3a0..f8b5649476 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -1,40 +1,43 @@ import { expect } from 'chai'; -import { MongoClient, MongoError } from '../../mongodb'; +import { MongoClient, MongoError, MongoServerSelectionError } from '../../mongodb'; -describe.only('Error (Integration)', function () { +describe('Error (Integration)', function () { describe('AggregateErrors', function () { - it('constructs the message properly', { requires: { nodejs: '>=16' } }, () => { - for (const { errors, message } of [ - { - errors: [], - message: - 'AggregateError: no suberrors have error messages. please check the `cause` property for more error information.' - }, - { errors: [new Error('message 1')], message: 'Aggregate Error: message 1' }, - { - errors: [new Error('message 1'), new Error('message 2')], - message: 'Aggregate Error: message 1, message 2' - } - ]) { - const error = new AggregateError(errors); - const mongoError = new MongoError(error); - - expect( - mongoError.message, - `built the message properly with an array of ${errors.length} errors` - ).to.equal(message); + for (const { errors, message } of [ + { + errors: [], + message: + 'AggregateError has an empty errors array. Please check the `cause` property for more information.' + }, + { errors: [new Error('message 1')], message: 'message 1' }, + { + errors: [new Error('message 1'), new Error('message 2')], + message: 'message 1, message 2' } - }); + ]) { + it( + `constructs the message properly with an array of ${errors.length} errors`, + { requires: { nodejs: '>=16' } }, + () => { + const error = new AggregateError(errors); + const mongoError = new MongoError(error); - it('uses the message on AggregateErrors, if non-empty', () => { - const error = new AggregateError([new Error('non-empty')]); - error.message = 'custom error message'; - const mongoError = new MongoError(error); - expect(mongoError.message).to.equal('custom error message'); + expect(mongoError.message).to.equal(message); + } + ); + } + + context('when the message on the AggregateError is non-empty', () => { + it(`uses the AggregateError's message`, () => { + const error = new AggregateError([new Error('non-empty')]); + error.message = 'custom error message'; + const mongoError = new MongoError(error); + expect(mongoError.message).to.equal('custom error message'); + }); }); - it('sets the aggregate error to the cause property', { requires: { nodejs: '>=16' } }, () => { + it('sets the AggregateError to the cause property', { requires: { nodejs: '>=16' } }, () => { const error = new AggregateError([new Error('error 1')]); const mongoError = new MongoError(error); expect(mongoError.cause).to.equal(error); @@ -45,7 +48,7 @@ describe.only('Error (Integration)', function () { const error = await MongoClient.connect('mongodb://localhost:27222', { serverSelectionTimeoutMS: 1000 }).catch(e => e); - expect(error).to.be.instanceOf(Error); + expect(error).to.be.instanceOf(MongoServerSelectionError); expect(error.message).not.to.be.empty; }); }); From 34f19e1d2dbd4ce55ddad1d4ae44e503c9461e9e Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 25 May 2023 10:01:49 -0600 Subject: [PATCH 6/6] skip on node16- --- test/integration/node-specific/errors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index f8b5649476..7e86cf5ca3 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -29,7 +29,7 @@ describe('Error (Integration)', function () { } context('when the message on the AggregateError is non-empty', () => { - it(`uses the AggregateError's message`, () => { + it(`uses the AggregateError's message`, { requires: { nodejs: '>=16' } }, () => { const error = new AggregateError([new Error('non-empty')]); error.message = 'custom error message'; const mongoError = new MongoError(error);