From da58b09ccf3aa583cdaa8fdec965ecefeb230e14 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 18 Mar 2025 11:22:46 -0400 Subject: [PATCH 1/5] fix(NODE-6864): socket errors are not always converted to MongoNetworkErrors --- src/cmap/connection.ts | 13 +- .../convert_socket_errors.test.ts | 138 ++++++++++++++++++ 2 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 test/integration/node-specific/convert_socket_errors.test.ts diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index bbe65a2510..c1cbf68b2d 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -247,9 +247,9 @@ export class Connection extends TypedEventEmitter { this.lastUseTime = now(); this.messageStream = this.socket - .on('error', this.onError.bind(this)) + .on('error', this.onSocketError.bind(this)) .pipe(new SizedMessageTransform({ connection: this })) - .on('error', this.onError.bind(this)); + .on('error', this.onTransformError.bind(this)); this.socket.on('close', this.onClose.bind(this)); this.socket.on('timeout', this.onTimeout.bind(this)); @@ -304,6 +304,14 @@ export class Connection extends TypedEventEmitter { this.lastUseTime = now(); } + private onSocketError(cause: Error) { + this.onError(new MongoNetworkError(cause.message, { cause })); + } + + private onTransformError(error: Error) { + this.onError(error); + } + public onError(error: Error) { this.cleanup(error); } @@ -769,7 +777,6 @@ export class Connection extends TypedEventEmitter { } finally { this.dataEvents = null; this.messageStream.pause(); - this.throwIfAborted(); } } } diff --git a/test/integration/node-specific/convert_socket_errors.test.ts b/test/integration/node-specific/convert_socket_errors.test.ts new file mode 100644 index 0000000000..c3d8c80d6f --- /dev/null +++ b/test/integration/node-specific/convert_socket_errors.test.ts @@ -0,0 +1,138 @@ +import { expect } from 'chai'; +import * as sinon from 'sinon'; + +import { ConnectionPool, type MongoClient, MongoNetworkError } from '../../mongodb'; +import { clearFailPoint, configureFailPoint } from '../../tools/utils'; + +describe('Socket Errors', () => { + describe('when destroyed after write', () => { + let client: MongoClient; + let collection; + + beforeEach(async function () { + client = this.configuration.newClient({}, { appName: 'failInserts' }); + await client.connect(); + const db = client.db('closeConn'); + collection = db.collection('closeConn'); + + const checkOut = sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(fakeCheckout); + async function fakeCheckout(...args) { + const connection = await checkOut.wrappedMethod.call(this, ...args); + + const write = sinon.stub(connection.socket, 'write').callsFake(function (...args) { + queueMicrotask(() => { + this.destroy(new Error('read ECONNRESET')); + }); + return write.wrappedMethod.call(this, ...args); + }); + + return connection; + } + }); + + afterEach(async function () { + sinon.restore(); + await client.close(); + }); + + it('throws a MongoNetworkError', async () => { + const error = await collection.insertOne({ name: 'test' }).catch(error => error); + expect(error).to.be.instanceOf(MongoNetworkError); + }); + }); + + describe('when destroyed after read', () => { + let client: MongoClient; + let collection; + + const metadata: MongoDBMetadataUI = { requires: { mongodb: '>=4.4' } }; + + beforeEach(async function () { + if (!this.configuration.filters.NodeVersionFilter.filter({ metadata })) { + return; + } + + await configureFailPoint(this.configuration, { + configureFailPoint: 'failCommand', + mode: 'alwaysOn', + data: { + appName: 'failInserts', + failCommands: ['insert'], + blockConnection: true, + blockTimeMS: 1000 // just so the server doesn't reply super fast. + } + }); + + client = this.configuration.newClient({}, { appName: 'failInserts' }); + await client.connect(); + const db = client.db('closeConn'); + collection = db.collection('closeConn'); + + const checkOut = sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(fakeCheckout); + async function fakeCheckout(...args) { + const connection = await checkOut.wrappedMethod.call(this, ...args); + + const on = sinon.stub(connection.messageStream, 'on').callsFake(function (...args) { + if (args[0] === 'data') { + queueMicrotask(() => { + connection.socket.destroy(new Error('read ECONNRESET')); + }); + } + return on.wrappedMethod.call(this, ...args); + }); + + return connection; + } + }); + + afterEach(async function () { + sinon.restore(); + await clearFailPoint(this.configuration); + await client.close(); + }); + + it('throws a MongoNetworkError', metadata, async () => { + const error = await collection.insertOne({ name: 'test' }).catch(error => error); + expect(error).to.be.instanceOf(MongoNetworkError); + }); + }); + + describe('when destroyed by failpoint', () => { + let client: MongoClient; + let collection; + + const metadata: MongoDBMetadataUI = { requires: { mongodb: '>=4.4' } }; + + beforeEach(async function () { + if (!this.configuration.filters.NodeVersionFilter.filter({ metadata })) { + return; + } + + await configureFailPoint(this.configuration, { + configureFailPoint: 'failCommand', + mode: 'alwaysOn', + data: { + appName: 'failInserts2', + failCommands: ['insert'], + closeConnection: true + } + }); + + client = this.configuration.newClient({}, { appName: 'failInserts2' }); + await client.connect(); + const db = client.db('closeConn'); + collection = db.collection('closeConn'); + }); + + afterEach(async function () { + sinon.restore(); + await clearFailPoint(this.configuration); + await client.close(); + }); + + it('throws a MongoNetworkError', metadata, async () => { + const error = await collection.insertOne({ name: 'test' }).catch(error => error); + expect(error, error.stack).to.be.instanceOf(MongoNetworkError); + }); + }); +}); From 7b0caf9b65e73d2e9b7f29bed6f9d33478624a7d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 18 Apr 2025 15:52:24 -0400 Subject: [PATCH 2/5] chore: fix less promises left over on windows --- test/integration/node-specific/resource_clean_up.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/integration/node-specific/resource_clean_up.test.ts b/test/integration/node-specific/resource_clean_up.test.ts index 1e1256f3d4..dac0502344 100644 --- a/test/integration/node-specific/resource_clean_up.test.ts +++ b/test/integration/node-specific/resource_clean_up.test.ts @@ -107,7 +107,11 @@ describe('Driver Resources', () => { await sleep(10); const promiseCountAfter = v8.queryObjects(Promise, { format: 'count' }); - expect(promiseCountAfter).to.be.within(promiseCountBefore - 5, promiseCountBefore + 5); + const offset = process.platform === 'win32' ? 30 : 5; + expect(promiseCountAfter).to.be.within( + promiseCountBefore - offset, + promiseCountBefore + offset + ); }); }); }); From 60a8c665bf031c4d255477064f6fc04780f57f1d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 21 Apr 2025 11:57:46 -0400 Subject: [PATCH 3/5] test: network error wrapping --- .../convert_socket_errors.test.ts | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/test/integration/node-specific/convert_socket_errors.test.ts b/test/integration/node-specific/convert_socket_errors.test.ts index c3d8c80d6f..aa87e0fb21 100644 --- a/test/integration/node-specific/convert_socket_errors.test.ts +++ b/test/integration/node-specific/convert_socket_errors.test.ts @@ -1,10 +1,42 @@ +import { Duplex } from 'node:stream'; + import { expect } from 'chai'; import * as sinon from 'sinon'; -import { ConnectionPool, type MongoClient, MongoNetworkError } from '../../mongodb'; +import { Connection, ConnectionPool, type MongoClient, MongoNetworkError, ns } from '../../mongodb'; import { clearFailPoint, configureFailPoint } from '../../tools/utils'; describe('Socket Errors', () => { + describe('when a socket emits an error', () => { + it('command throws a MongoNetworkError', async () => { + const socket = new Duplex(); + // @ts-expect-error: not a real socket + const connection = new Connection(socket, {}); + const testError = new Error('blah'); + socket.emit('error', testError); + const commandRes = connection.command(ns('a.b'), { ping: 1 }, {}); + + const error = await commandRes.catch(error => error); + expect(error).to.be.instanceOf(MongoNetworkError); + expect(error.cause).to.equal(testError); + }); + }); + + describe('when the sized message stream emits an error', () => { + it('command throws the same error', async () => { + const socket = new Duplex(); + // @ts-expect-error: not a real socket + const connection = new Connection(socket, {}); + const testError = new Error('blah'); + // @ts-expect-error: private field + connection.messageStream.emit('error', testError); + const commandRes = connection.command(ns('a.b'), { ping: 1 }, {}); + + const error = await commandRes.catch(error => error); + expect(error).to.equal(testError); + }); + }); + describe('when destroyed after write', () => { let client: MongoClient; let collection; From 2283c3862bd6b7cf6821147e430fdbbc9981c191 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 21 Apr 2025 15:58:17 -0400 Subject: [PATCH 4/5] chore: remove tests --- .../convert_socket_errors.test.ts | 92 ------------------- 1 file changed, 92 deletions(-) diff --git a/test/integration/node-specific/convert_socket_errors.test.ts b/test/integration/node-specific/convert_socket_errors.test.ts index aa87e0fb21..171f899a0f 100644 --- a/test/integration/node-specific/convert_socket_errors.test.ts +++ b/test/integration/node-specific/convert_socket_errors.test.ts @@ -37,98 +37,6 @@ describe('Socket Errors', () => { }); }); - describe('when destroyed after write', () => { - let client: MongoClient; - let collection; - - beforeEach(async function () { - client = this.configuration.newClient({}, { appName: 'failInserts' }); - await client.connect(); - const db = client.db('closeConn'); - collection = db.collection('closeConn'); - - const checkOut = sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(fakeCheckout); - async function fakeCheckout(...args) { - const connection = await checkOut.wrappedMethod.call(this, ...args); - - const write = sinon.stub(connection.socket, 'write').callsFake(function (...args) { - queueMicrotask(() => { - this.destroy(new Error('read ECONNRESET')); - }); - return write.wrappedMethod.call(this, ...args); - }); - - return connection; - } - }); - - afterEach(async function () { - sinon.restore(); - await client.close(); - }); - - it('throws a MongoNetworkError', async () => { - const error = await collection.insertOne({ name: 'test' }).catch(error => error); - expect(error).to.be.instanceOf(MongoNetworkError); - }); - }); - - describe('when destroyed after read', () => { - let client: MongoClient; - let collection; - - const metadata: MongoDBMetadataUI = { requires: { mongodb: '>=4.4' } }; - - beforeEach(async function () { - if (!this.configuration.filters.NodeVersionFilter.filter({ metadata })) { - return; - } - - await configureFailPoint(this.configuration, { - configureFailPoint: 'failCommand', - mode: 'alwaysOn', - data: { - appName: 'failInserts', - failCommands: ['insert'], - blockConnection: true, - blockTimeMS: 1000 // just so the server doesn't reply super fast. - } - }); - - client = this.configuration.newClient({}, { appName: 'failInserts' }); - await client.connect(); - const db = client.db('closeConn'); - collection = db.collection('closeConn'); - - const checkOut = sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(fakeCheckout); - async function fakeCheckout(...args) { - const connection = await checkOut.wrappedMethod.call(this, ...args); - - const on = sinon.stub(connection.messageStream, 'on').callsFake(function (...args) { - if (args[0] === 'data') { - queueMicrotask(() => { - connection.socket.destroy(new Error('read ECONNRESET')); - }); - } - return on.wrappedMethod.call(this, ...args); - }); - - return connection; - } - }); - - afterEach(async function () { - sinon.restore(); - await clearFailPoint(this.configuration); - await client.close(); - }); - - it('throws a MongoNetworkError', metadata, async () => { - const error = await collection.insertOne({ name: 'test' }).catch(error => error); - expect(error).to.be.instanceOf(MongoNetworkError); - }); - }); - describe('when destroyed by failpoint', () => { let client: MongoClient; let collection; From 901f878f7472c8bc8a68d7273f559fb739edcadc Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 21 Apr 2025 16:04:58 -0400 Subject: [PATCH 5/5] test: remove lower bound --- .../integration/node-specific/convert_socket_errors.test.ts | 2 +- test/integration/node-specific/resource_clean_up.test.ts | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/test/integration/node-specific/convert_socket_errors.test.ts b/test/integration/node-specific/convert_socket_errors.test.ts index 171f899a0f..0f3508632e 100644 --- a/test/integration/node-specific/convert_socket_errors.test.ts +++ b/test/integration/node-specific/convert_socket_errors.test.ts @@ -3,7 +3,7 @@ import { Duplex } from 'node:stream'; import { expect } from 'chai'; import * as sinon from 'sinon'; -import { Connection, ConnectionPool, type MongoClient, MongoNetworkError, ns } from '../../mongodb'; +import { Connection, type MongoClient, MongoNetworkError, ns } from '../../mongodb'; import { clearFailPoint, configureFailPoint } from '../../tools/utils'; describe('Socket Errors', () => { diff --git a/test/integration/node-specific/resource_clean_up.test.ts b/test/integration/node-specific/resource_clean_up.test.ts index dac0502344..41ec740831 100644 --- a/test/integration/node-specific/resource_clean_up.test.ts +++ b/test/integration/node-specific/resource_clean_up.test.ts @@ -107,11 +107,7 @@ describe('Driver Resources', () => { await sleep(10); const promiseCountAfter = v8.queryObjects(Promise, { format: 'count' }); - const offset = process.platform === 'win32' ? 30 : 5; - expect(promiseCountAfter).to.be.within( - promiseCountBefore - offset, - promiseCountBefore + offset - ); + expect(promiseCountAfter).to.be.lessThan(promiseCountBefore + 5); }); }); });