From acb2c67b60aad8354c34d18cf369bab25cd7ba88 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 10 Apr 2024 22:49:41 -0600 Subject: [PATCH 1/4] fix(NODE-5530): make topology descriptions JSON stringifiable (#4070) Co-authored-by: Aditi Khare <106987683+aditi-khare-mongoDB@users.noreply.github.com> --- src/sdam/topology_description.ts | 12 ++++- test/integration/node-specific/errors.test.ts | 46 +++++++++++++++++++ .../topology_description.test.ts | 43 +++++++++++++---- 3 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 test/integration/node-specific/errors.test.ts diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index 380e6c8f0c9..ff5f9de3b74 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -1,4 +1,4 @@ -import type { ObjectId } from '../bson'; +import { EJSON, type ObjectId } from '../bson'; import * as WIRE_CONSTANTS from '../cmap/wire_protocol/constants'; import { MongoRuntimeError, type MongoServerError } from '../error'; import { compareObjectId, shuffle } from '../utils'; @@ -342,6 +342,16 @@ export class TopologyDescription { hasServer(address: string): boolean { return this.servers.has(address); } + + /** + * Returns a JSON-serializable representation of the TopologyDescription. This is primarily + * intended for use with JSON.stringify(). + * + * This method will not throw. + */ + toJSON() { + return EJSON.serialize(this); + } } function topologyTypeForServerType(serverType: ServerType): TopologyType { diff --git a/test/integration/node-specific/errors.test.ts b/test/integration/node-specific/errors.test.ts new file mode 100644 index 00000000000..ff00db65647 --- /dev/null +++ b/test/integration/node-specific/errors.test.ts @@ -0,0 +1,46 @@ +import { expect } from 'chai'; + +import { MongoClient, MongoServerSelectionError, ReadPreference } from '../../mongodb'; + +describe('Error (Integration)', function () { + 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(MongoServerSelectionError); + expect(error.message).not.to.be.empty; + }); + + context('when a server selection error is stringified', function () { + it( + 'the error"s topology description correctly displays the `servers`', + { requires: { topology: 'replicaset' } }, + async function () { + const client: MongoClient = this.configuration.newClient({ + serverSelectionTimeoutMS: 1000 + }); + try { + await client.connect(); + + const error = await client + .db('foo') + .collection('bar') + .find( + {}, + { + // Use meaningless read preference tags to ensure that the server selection fails + readPreference: new ReadPreference('secondary', [{ ny: 'ny' }]) + } + ) + .toArray() + .catch(e => JSON.parse(JSON.stringify(e))); + + const servers = error.reason.servers; + expect(Object.keys(servers).length > 0).to.be.true; + } finally { + await client.close(); + } + } + ); + }); +}); diff --git a/test/integration/server-discovery-and-monitoring/topology_description.test.ts b/test/integration/server-discovery-and-monitoring/topology_description.test.ts index 1169176ffb8..291e13dd70e 100644 --- a/test/integration/server-discovery-and-monitoring/topology_description.test.ts +++ b/test/integration/server-discovery-and-monitoring/topology_description.test.ts @@ -14,7 +14,22 @@ describe('TopologyDescription (integration tests)', function () { await client.close(); }); + beforeEach(async function () { + client = this.configuration.newClient(); + await client.connect(); + }); + context('options', function () { + let client: MongoClient; + + afterEach(async function () { + await client.close(); + }); + + beforeEach(async function () { + client = this.configuration.newClient(); + }); + context('localThresholdMS', function () { it('should default to 15ms', async function () { const options: MongoClientOptions = {}; @@ -35,15 +50,6 @@ describe('TopologyDescription (integration tests)', function () { }); context('topology types', function () { - let client: MongoClient; - beforeEach(async function () { - client = this.configuration.newClient(); - }); - - afterEach(async function () { - await client.close(); - }); - const topologyTypesMap = new Map([ ['single', TopologyType.Single], ['replicaset', TopologyType.ReplicaSetWithPrimary], @@ -65,4 +71,23 @@ describe('TopologyDescription (integration tests)', function () { ); } }); + + describe('json stringification', function () { + it('can be stringified without error', function () { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-non-null-asserted-optional-chain + const description = client.topology?.description!; + expect(description).to.exist; + + expect(() => JSON.stringify(description)).not.to.throw; + }); + + it('properly stringifies the server description map', function () { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-non-null-asserted-optional-chain + const description = client.topology?.description!; + expect(description).to.exist; + + const { servers } = JSON.parse(JSON.stringify(description)); + expect(Object.keys(servers).length > 0, '`servers` stringified with no servers.').to.be.true; + }); + }); }); From ea1ba818478a3fdb574b17f7b4ccb4278e46a7ff Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 11 Apr 2024 10:31:17 -0600 Subject: [PATCH 2/4] Remove accidentally added test --- test/integration/node-specific/errors.test.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/integration/node-specific/errors.test.ts b/test/integration/node-specific/errors.test.ts index ff00db65647..bc5a77ead78 100644 --- a/test/integration/node-specific/errors.test.ts +++ b/test/integration/node-specific/errors.test.ts @@ -1,16 +1,8 @@ import { expect } from 'chai'; -import { MongoClient, MongoServerSelectionError, ReadPreference } from '../../mongodb'; +import { type MongoClient, ReadPreference } from '../../mongodb'; describe('Error (Integration)', function () { - 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(MongoServerSelectionError); - expect(error.message).not.to.be.empty; - }); - context('when a server selection error is stringified', function () { it( 'the error"s topology description correctly displays the `servers`', From 2fb73c55ad401c2c30eae00b13a99e5eaa66fe58 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 11 Apr 2024 14:27:55 -0600 Subject: [PATCH 3/4] fix lint --- src/sdam/topology_description.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index ff5f9de3b74..b5f7581b70f 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -1,4 +1,6 @@ -import { EJSON, type ObjectId } from '../bson'; +import { EJSON } from 'bson'; + +import { type ObjectId } from '../bson'; import * as WIRE_CONSTANTS from '../cmap/wire_protocol/constants'; import { MongoRuntimeError, type MongoServerError } from '../error'; import { compareObjectId, shuffle } from '../utils'; From b01e5eff04cf3a29001e1fde86876c6309e772b1 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 15 Apr 2024 11:19:10 -0600 Subject: [PATCH 4/4] fix failing tests --- test/integration/node-specific/errors.test.ts | 38 ------------------- test/integration/node-specific/errors.ts | 35 ++++++++++++++++- 2 files changed, 34 insertions(+), 39 deletions(-) delete mode 100644 test/integration/node-specific/errors.test.ts diff --git a/test/integration/node-specific/errors.test.ts b/test/integration/node-specific/errors.test.ts deleted file mode 100644 index bc5a77ead78..00000000000 --- a/test/integration/node-specific/errors.test.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { expect } from 'chai'; - -import { type MongoClient, ReadPreference } from '../../mongodb'; - -describe('Error (Integration)', function () { - context('when a server selection error is stringified', function () { - it( - 'the error"s topology description correctly displays the `servers`', - { requires: { topology: 'replicaset' } }, - async function () { - const client: MongoClient = this.configuration.newClient({ - serverSelectionTimeoutMS: 1000 - }); - try { - await client.connect(); - - const error = await client - .db('foo') - .collection('bar') - .find( - {}, - { - // Use meaningless read preference tags to ensure that the server selection fails - readPreference: new ReadPreference('secondary', [{ ny: 'ny' }]) - } - ) - .toArray() - .catch(e => JSON.parse(JSON.stringify(e))); - - const servers = error.reason.servers; - expect(Object.keys(servers).length > 0).to.be.true; - } finally { - await client.close(); - } - } - ); - }); -}); diff --git a/test/integration/node-specific/errors.ts b/test/integration/node-specific/errors.ts index 7e86cf5ca34..853cd8943aa 100644 --- a/test/integration/node-specific/errors.ts +++ b/test/integration/node-specific/errors.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { MongoClient, MongoError, MongoServerSelectionError } from '../../mongodb'; +import { MongoClient, MongoError, MongoServerSelectionError, ReadPreference } from '../../mongodb'; describe('Error (Integration)', function () { describe('AggregateErrors', function () { @@ -51,4 +51,37 @@ describe('Error (Integration)', function () { expect(error).to.be.instanceOf(MongoServerSelectionError); expect(error.message).not.to.be.empty; }); + + context('when a server selection error is stringified', function () { + it( + 'the error"s topology description correctly displays the `servers`', + { requires: { topology: 'replicaset' } }, + async function () { + const client: MongoClient = this.configuration.newClient({ + serverSelectionTimeoutMS: 1000 + }); + try { + await client.connect(); + + const error = await client + .db('foo') + .collection('bar') + .find( + {}, + { + // Use meaningless read preference tags to ensure that the server selection fails + readPreference: new ReadPreference('secondary', [{ ny: 'ny' }]) + } + ) + .toArray() + .catch(e => JSON.parse(JSON.stringify(e))); + + const servers = error.reason.servers; + expect(Object.keys(servers).length > 0).to.be.true; + } finally { + await client.close(); + } + } + ); + }); });