From 3a5832b242628aeac0b415ba8eb039cd3227d08b Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 16 Apr 2025 17:50:54 +0200 Subject: [PATCH 01/12] feat(NODE-6245): add keepAliveInitialDelay config --- src/cmap/connect.ts | 3 +- src/connection_string.ts | 3 + src/mongo_client.ts | 6 +- .../node-specific/mongo_client.test.ts | 88 +++++++++++++++++++ 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 394b70689ca..9b6e311bc82 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -289,6 +289,7 @@ export const LEGAL_TLS_SOCKET_OPTIONS = [ export const LEGAL_TCP_SOCKET_OPTIONS = [ 'autoSelectFamily', 'autoSelectFamilyAttemptTimeout', + 'keepAliveInitialDelay', 'family', 'hints', 'localAddress', @@ -376,7 +377,7 @@ export async function makeSocket(options: MakeConnectionOptions): Promise; diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 4a97035d55c..8b47ef0f356 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { once } from 'events'; import * as net from 'net'; +import { Socket } from 'net'; import * as sinon from 'sinon'; import { @@ -135,6 +136,93 @@ describe('class MongoClient', function () { expect(error).to.be.instanceOf(MongoServerSelectionError); }); + describe('#connect', function () { + context('when keepAliveInitialDelay is provided', function () { + context('when the value is 0', function () { + const options = { keepAliveInitialDelay: 0 }; + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(Socket.prototype, 'setKeepAlive'); + client = this.configuration.newClient(options); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('passes through the option', function () { + expect(spy).to.have.been.calledWith(true, 0); + }); + }); + + context('when the value is positive', function () { + const options = { keepAliveInitialDelay: 100 }; + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(Socket.prototype, 'setKeepAlive'); + client = this.configuration.newClient(options); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('passes through the option', function () { + expect(spy).to.have.been.calledWith(true, 100); + }); + }); + + context('when the value is negative', function () { + const options = { keepAliveInitialDelay: -100 }; + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(Socket.prototype, 'setKeepAlive'); + client = this.configuration.newClient(options); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('passes through the option', function () { + expect(spy).to.have.been.calledWith(true, -100); + }); + }); + }); + + context('when keepAliveInitialDelay is not provided', function () { + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(Socket.prototype, 'setKeepAlive'); + client = this.configuration.newClient(); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('sets keepalive to 120000', function () { + expect(spy).to.have.been.calledWith(true, 120000); + }); + }); + }); + it('Should correctly pass through appname', { metadata: { requires: { From 4c1365276a73112bbe710e2c78506da41f956258 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 16 Apr 2025 19:33:49 +0200 Subject: [PATCH 02/12] test: fix assertion --- test/integration/node-specific/mongo_client.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 8b47ef0f356..8951a5cad5f 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -188,7 +188,6 @@ describe('class MongoClient', function () { beforeEach(async function () { spy = sinon.spy(Socket.prototype, 'setKeepAlive'); client = this.configuration.newClient(options); - await client.connect(); }); afterEach(async function () { @@ -196,8 +195,10 @@ describe('class MongoClient', function () { spy.restore(); }); - it('passes through the option', function () { - expect(spy).to.have.been.calledWith(true, -100); + it('raises an error', function () { + expect(async () => { + await client.connect(); + }).to.throw(/keepAliveInitialDelay can only be a positive int value/); }); }); }); From 5e4c8bccbb831ec3eecdafa958fcf1a5b45f2e33 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 16 Apr 2025 19:56:43 +0200 Subject: [PATCH 03/12] test: fix test --- .../integration/node-specific/mongo_client.test.ts | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 8951a5cad5f..aa89884aedf 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -182,22 +182,10 @@ describe('class MongoClient', function () { context('when the value is negative', function () { const options = { keepAliveInitialDelay: -100 }; - let client; - let spy; - - beforeEach(async function () { - spy = sinon.spy(Socket.prototype, 'setKeepAlive'); - client = this.configuration.newClient(options); - }); - - afterEach(async function () { - await client?.close(); - spy.restore(); - }); it('raises an error', function () { expect(async () => { - await client.connect(); + this.configuration.newClient(options); }).to.throw(/keepAliveInitialDelay can only be a positive int value/); }); }); From 4c5838396a0ffdb8cde5e2cdc9b1726e27c60565 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 16 Apr 2025 20:10:13 +0200 Subject: [PATCH 04/12] test: fix test --- test/integration/node-specific/mongo_client.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index aa89884aedf..2f87a7a846f 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -184,7 +184,7 @@ describe('class MongoClient', function () { const options = { keepAliveInitialDelay: -100 }; it('raises an error', function () { - expect(async () => { + expect(() => { this.configuration.newClient(options); }).to.throw(/keepAliveInitialDelay can only be a positive int value/); }); From 6ec4660c4d928320dd0730d9b3d4c4c176fc2b78 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Thu, 17 Apr 2025 14:04:40 +0200 Subject: [PATCH 05/12] chore: add api doc to option --- src/mongo_client.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 6ea0e07824c..852c4a50e65 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -118,6 +118,7 @@ export type SupportedSocketOptions = Pick< TcpNetConnectOpts & { autoSelectFamily?: boolean; autoSelectFamilyAttemptTimeout?: number; + /** Node.JS socket option to set the time the first keepalive probe is sent on an idle socket. */ keepAliveInitialDelay?: number; }, (typeof LEGAL_TCP_SOCKET_OPTIONS)[number] From 4c0e2fbaaba2b52d3bfb7b4f8e48cafaaddec832 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 18:01:59 +0200 Subject: [PATCH 06/12] feat(NODE-6245): pass through as any --- src/cmap/connect.ts | 5 +- src/connection_string.ts | 4 +- .../node-specific/mongo_client.test.ts | 59 +++++++++++++++---- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 9b6e311bc82..5a53b5214e4 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -307,6 +307,10 @@ function parseConnectOptions(options: ConnectionOptions): SocketConnectOpts { (result as Document)[name] = options[name]; } } + if (result.keepAliveInitialDelay == null) { + result.keepAliveInitialDelay = 120000; + } + result.keepAlive = true; if (typeof hostAddress.socketPath === 'string') { result.path = hostAddress.socketPath; @@ -377,7 +381,6 @@ export async function makeSocket(options: MakeConnectionOptions): Promise { - this.configuration.newClient(options); - }).to.throw(/keepAliveInitialDelay can only be a positive int value/); + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('sets the option to 0', function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + keepAlive: true, + keepAliveInitialDelay: 0 + }) + ); }); }); }); @@ -196,7 +224,7 @@ describe('class MongoClient', function () { let spy; beforeEach(async function () { - spy = sinon.spy(Socket.prototype, 'setKeepAlive'); + spy = sinon.spy(net, 'createConnection'); client = this.configuration.newClient(); await client.connect(); }); @@ -207,7 +235,12 @@ describe('class MongoClient', function () { }); it('sets keepalive to 120000', function () { - expect(spy).to.have.been.calledWith(true, 120000); + expect(spy).to.have.been.calledWith( + sinon.match({ + keepAlive: true, + keepAliveInitialDelay: 120000 + }) + ); }); }); }); From 564449caa167f83de3f7e4249069027d76661e85 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 18:25:54 +0200 Subject: [PATCH 07/12] chore: add nodelay --- src/cmap/connect.ts | 3 +- .../node-specific/mongo_client.test.ts | 72 ++++++++++++++++--- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 5a53b5214e4..b55e5f52a00 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -311,6 +311,7 @@ function parseConnectOptions(options: ConnectionOptions): SocketConnectOpts { result.keepAliveInitialDelay = 120000; } result.keepAlive = true; + result.noDelay = options.noDelay ?? true; if (typeof hostAddress.socketPath === 'string') { result.path = hostAddress.socketPath; @@ -352,7 +353,6 @@ function parseSslOptions(options: MakeConnectionOptions): TLSConnectionOpts { export async function makeSocket(options: MakeConnectionOptions): Promise { const useTLS = options.tls ?? false; - const noDelay = options.noDelay ?? true; const connectTimeoutMS = options.connectTimeoutMS ?? 30000; const existingSocket = options.existingSocket; @@ -382,7 +382,6 @@ export async function makeSocket(options: MakeConnectionOptions): Promise void) | null = null; diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index c0749e8dd64..9977bf9413f 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -243,6 +243,56 @@ describe('class MongoClient', function () { ); }); }); + + context('when noDelay is not provided', function () { + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(net, 'createConnection'); + client = this.configuration.newClient(); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('sets noDelay to true', function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + noDelay: true + }) + ); + }); + }); + + context('when noDelay is provided', function () { + let client; + let spy; + + beforeEach(async function () { + const options = { noDelay: false }; + spy = sinon.spy(net, 'createConnection'); + const uri = this.configuration.url(); + client = new MongoClient(uri, options); + await client.connect(); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('sets noDelay', function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + noDelay: false + }) + ); + }); + }); }); it('Should correctly pass through appname', { @@ -999,12 +1049,12 @@ describe('class MongoClient', function () { metadata: { requires: { topology: ['single'] } }, test: async function () { await client.connect(); - expect(netSpy).to.have.been.calledWith({ - autoSelectFamily: false, - autoSelectFamilyAttemptTimeout: 100, - host: 'localhost', - port: 27017 - }); + expect(netSpy).to.have.been.calledWith( + sinon.match({ + autoSelectFamily: false, + autoSelectFamilyAttemptTimeout: 100 + }) + ); } }); }); @@ -1018,11 +1068,11 @@ describe('class MongoClient', function () { metadata: { requires: { topology: ['single'] } }, test: async function () { await client.connect(); - expect(netSpy).to.have.been.calledWith({ - autoSelectFamily: true, - host: 'localhost', - port: 27017 - }); + expect(netSpy).to.have.been.calledWith( + sinon.match({ + autoSelectFamily: true + }) + ); } }); }); From 923e49623ef14a29cf818f9975f855d38ba6e62a Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 18:33:02 +0200 Subject: [PATCH 08/12] Update src/cmap/connect.ts Co-authored-by: Neal Beeken --- src/cmap/connect.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index b55e5f52a00..6449c288fc8 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -307,9 +307,7 @@ function parseConnectOptions(options: ConnectionOptions): SocketConnectOpts { (result as Document)[name] = options[name]; } } - if (result.keepAliveInitialDelay == null) { - result.keepAliveInitialDelay = 120000; - } +result.keepAliveInitialDelay ??= 120000; result.keepAlive = true; result.noDelay = options.noDelay ?? true; From a32baa18f969ab21bf8030b2ace7a82306d3d457 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 18:39:10 +0200 Subject: [PATCH 09/12] test: fix tls tests --- test/manual/tls_support.test.ts | 37 +++++++++++++++------------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/test/manual/tls_support.test.ts b/test/manual/tls_support.test.ts index 53597086263..5448c0a02eb 100644 --- a/test/manual/tls_support.test.ts +++ b/test/manual/tls_support.test.ts @@ -77,19 +77,17 @@ describe('TLS Support', function () { it('sets the default options', async function () { await client.connect(); - expect(tlsSpy).to.have.been.calledWith({ - autoSelectFamily: true, - host: 'localhost', - port: 27017, - servername: 'localhost', - ca: sinon.match.defined, - cert: sinon.match.defined, - key: sinon.match.defined - }); + expect(tlsSpy).to.have.been.calledWith( + sinon.match({ + ca: sinon.match.defined, + cert: sinon.match.defined, + key: sinon.match.defined + }) + ); }); }); - context('when auto family options are set', function () { + context('when auto select family options are set', function () { let tlsSpy; afterEach(function () { @@ -107,16 +105,15 @@ describe('TLS Support', function () { it('sets the provided options', async function () { await client.connect(); - expect(tlsSpy).to.have.been.calledWith({ - autoSelectFamily: false, - autoSelectFamilyAttemptTimeout: 100, - host: 'localhost', - port: 27017, - servername: 'localhost', - ca: sinon.match.defined, - cert: sinon.match.defined, - key: sinon.match.defined - }); + expect(tlsSpy).to.have.been.calledWith( + sinon.match({ + autoSelectFamily: false, + autoSelectFamilyAttemptTimeout: 100, + ca: sinon.match.defined, + cert: sinon.match.defined, + key: sinon.match.defined + }) + ); }); }); From 0204f4a2da120d798080139431281d947cc96077 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 18:43:35 +0200 Subject: [PATCH 10/12] fix: lint --- src/cmap/connect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 6449c288fc8..aa73e88e5fc 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -307,7 +307,7 @@ function parseConnectOptions(options: ConnectionOptions): SocketConnectOpts { (result as Document)[name] = options[name]; } } -result.keepAliveInitialDelay ??= 120000; + result.keepAliveInitialDelay ??= 120000; result.keepAlive = true; result.noDelay = options.noDelay ?? true; From 270da1160619af6f235330cb60eb57220c78f539 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 19:22:15 +0200 Subject: [PATCH 11/12] chore: fix api version tests --- .../node-specific/mongo_client.test.ts | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 9977bf9413f..b0381454d65 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -154,13 +154,16 @@ describe('class MongoClient', function () { spy.restore(); }); - it('passes through the option', function () { - expect(spy).to.have.been.calledWith( - sinon.match({ - keepAlive: true, - keepAliveInitialDelay: 0 - }) - ); + it('passes through the option', { + metadata: { requires: { apiVersion: false } }, + test: function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + keepAlive: true, + keepAliveInitialDelay: 0 + }) + ); + } }); }); @@ -181,13 +184,16 @@ describe('class MongoClient', function () { spy.restore(); }); - it('passes through the option', function () { - expect(spy).to.have.been.calledWith( - sinon.match({ - keepAlive: true, - keepAliveInitialDelay: 100 - }) - ); + it('passes through the option', { + metadata: { requires: { apiVersion: false } }, + test: function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + keepAlive: true, + keepAliveInitialDelay: 100 + }) + ); + } }); }); @@ -208,13 +214,16 @@ describe('class MongoClient', function () { spy.restore(); }); - it('sets the option to 0', function () { - expect(spy).to.have.been.calledWith( - sinon.match({ - keepAlive: true, - keepAliveInitialDelay: 0 - }) - ); + it('sets the option to 0', { + metadata: { requires: { apiVersion: false } }, + test: function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + keepAlive: true, + keepAliveInitialDelay: 0 + }) + ); + } }); }); }); @@ -285,12 +294,15 @@ describe('class MongoClient', function () { spy.restore(); }); - it('sets noDelay', function () { - expect(spy).to.have.been.calledWith( - sinon.match({ - noDelay: false - }) - ); + it('sets noDelay', { + metadata: { requires: { apiVersion: false } }, + test: function () { + expect(spy).to.have.been.calledWith( + sinon.match({ + noDelay: false + }) + ); + } }); }); }); From 02cbb515e542cae3c98339b9de606d0930af2bbe Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 23 Apr 2025 19:59:08 +0200 Subject: [PATCH 12/12] chore: comments --- src/mongo_client.ts | 2 +- .../node-specific/mongo_client.test.ts | 30 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 852c4a50e65..e77afc4026a 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -118,7 +118,7 @@ export type SupportedSocketOptions = Pick< TcpNetConnectOpts & { autoSelectFamily?: boolean; autoSelectFamilyAttemptTimeout?: number; - /** Node.JS socket option to set the time the first keepalive probe is sent on an idle socket. */ + /** Node.JS socket option to set the time the first keepalive probe is sent on an idle socket. Defaults to 120000ms */ keepAliveInitialDelay?: number; }, (typeof LEGAL_TCP_SOCKET_OPTIONS)[number] diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index b0381454d65..9a0cea014bd 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -214,7 +214,7 @@ describe('class MongoClient', function () { spy.restore(); }); - it('sets the option to 0', { + it('the Node.js runtime sets the option to 0', { metadata: { requires: { apiVersion: false } }, test: function () { expect(spy).to.have.been.calledWith( @@ -226,6 +226,34 @@ describe('class MongoClient', function () { } }); }); + + context('when the value is mistyped', function () { + // Set server selection timeout to get the error quicker. + const options = { keepAliveInitialDelay: 'test', serverSelectionTimeoutMS: 1000 }; + let client; + let spy; + + beforeEach(async function () { + spy = sinon.spy(net, 'createConnection'); + const uri = this.configuration.url(); + client = new MongoClient(uri, options); + }); + + afterEach(async function () { + await client?.close(); + spy.restore(); + }); + + it('throws an error', { + metadata: { requires: { apiVersion: false } }, + test: async function () { + const error = await client.connect().catch(error => error); + expect(error.message).to.include( + 'property must be of type number. Received type string' + ); + } + }); + }); }); context('when keepAliveInitialDelay is not provided', function () {