test(NODE-6692): check that event emitters have error listeners

nbbeeken · nbbeeken · commit ba36dea2f630 · 2025-01-30T12:48:41.000-05:00
diff --git a/.mocharc.json b/.mocharc.json
@@ -4,7 +4,7 @@
     "source-map-support/register",
     "ts-node/register",
     "test/tools/runner/chai_addons.ts",
-    "test/tools/runner/hooks/unhandled_checker.ts"
+    "test/tools/runner/ee_checker.ts"
   ],
   "extension": [
     "js",
diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts
@@ -46,6 +46,7 @@ import {
   HostAddress,
   maxWireVersion,
   type MongoDBNamespace,
+  noop,
   now,
   once,
   squashError,
@@ -229,6 +230,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
   constructor(stream: Stream, options: ConnectionOptions) {
     super();
+    this.on('error', noop);
 
     this.socket = stream;
     this.id = options.id;
diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts
@@ -34,6 +34,7 @@ import {
   kDispose,
   List,
   makeCounter,
+  noop,
   now,
   promiseWithResolvers
 } from '../utils';
@@ -200,6 +201,7 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
 
   constructor(server: Server, options: ConnectionPoolOptions) {
     super();
+    this.on('error', noop);
 
     this.options = Object.freeze({
       connectionType: Connection,
diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts
@@ -27,6 +27,7 @@ import {
   type Disposable,
   kDispose,
   type MongoDBNamespace,
+  noop,
   squashError
 } from '../utils';
 
@@ -267,6 +268,7 @@ export abstract class AbstractCursor<
     options: AbstractCursorOptions & Abortable = {}
   ) {
     super();
+    this.on('error', noop);
 
     if (!client.s.isMongoClient) {
       throw new MongoRuntimeError('Cursor must be constructed with MongoClient');
diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts
@@ -7,7 +7,7 @@ import { type Filter, TypedEventEmitter } from '../mongo_types';
 import type { ReadPreference } from '../read_preference';
 import type { Sort } from '../sort';
 import { CSOTTimeoutContext } from '../timeout';
-import { resolveOptions } from '../utils';
+import { noop, resolveOptions } from '../utils';
 import { WriteConcern, type WriteConcernOptions } from '../write_concern';
 import type { FindOptions } from './../operations/find';
 import {
@@ -87,6 +87,7 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {
 
   constructor(db: Db, options?: GridFSBucketOptions) {
     super();
+    this.on('error', noop);
     this.setMaxListeners(0);
     const privateOptions = resolveOptions(db, {
       ...DEFAULT_GRIDFS_BUCKET_OPTIONS,
diff --git a/src/mongo_client.ts b/src/mongo_client.ts
@@ -58,6 +58,7 @@ import {
   hostMatchesWildcards,
   isHostMatch,
   type MongoDBNamespace,
+  noop,
   ns,
   resolveOptions,
   squashError
@@ -386,6 +387,7 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
 
   constructor(url: string, options?: MongoClientOptions) {
     super();
+    this.on('error', noop);
 
     this.options = parseOptions(url, this, options);
 
diff --git a/src/mongo_types.ts b/src/mongo_types.ts
@@ -24,6 +24,7 @@ import {
   type MongoLogger
 } from './mongo_logger';
 import type { Sort } from './sort';
+import { noop } from './utils';
 
 /** @internal */
 export type TODO_NODE_3286 = any;
@@ -472,7 +473,12 @@ export class TypedEventEmitter<Events extends EventsDescription> extends EventEm
 }
 
 /** @public */
-export class CancellationToken extends TypedEventEmitter<{ cancel(): void }> {}
+export class CancellationToken extends TypedEventEmitter<{ cancel(): void }> {
+  constructor(...args: any[]) {
+    super(...args);
+    this.on('error', noop);
+  }
+}
 
 /** @public */
 export type Abortable = {
diff --git a/src/sdam/monitor.ts b/src/sdam/monitor.ts
@@ -13,6 +13,7 @@ import {
   type Callback,
   type EventEmitterWithState,
   makeStateMachine,
+  noop,
   now,
   ns
 } from '../utils';
@@ -102,6 +103,7 @@ export class Monitor extends TypedEventEmitter<MonitorEvents> {
 
   constructor(server: Server, options: MonitorOptions) {
     super();
+    this.on('error', noop);
 
     this.server = server;
     this.connection = null;
diff --git a/src/sdam/server.ts b/src/sdam/server.ts
@@ -47,6 +47,7 @@ import {
   makeStateMachine,
   maxWireVersion,
   type MongoDBNamespace,
+  noop,
   supportsRetryableWrites
 } from '../utils';
 import { throwIfWriteConcernError } from '../write_concern';
@@ -142,6 +143,7 @@ export class Server extends TypedEventEmitter<ServerEvents> {
    */
   constructor(topology: Topology, description: ServerDescription, options: ServerOptions) {
     super();
+    this.on('error', noop);
 
     this.serverApi = options.serverApi;
 
diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts
@@ -3,7 +3,7 @@ import { clearTimeout, setTimeout } from 'timers';
 
 import { MongoRuntimeError } from '../error';
 import { TypedEventEmitter } from '../mongo_types';
-import { checkParentDomainMatch, HostAddress, squashError } from '../utils';
+import { checkParentDomainMatch, HostAddress, noop, squashError } from '../utils';
 
 /**
  * @internal
@@ -49,6 +49,7 @@ export class SrvPoller extends TypedEventEmitter<SrvPollerEvents> {
 
   constructor(options: SrvPollerOptions) {
     super();
+    this.on('error', noop);
 
     if (!options || !options.srvHost) {
       throw new MongoRuntimeError('Options for SrvPoller must exist and include srvHost');
diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts
@@ -44,6 +44,7 @@ import {
   kDispose,
   List,
   makeStateMachine,
+  noop,
   now,
   ns,
   promiseWithResolvers,
@@ -248,6 +249,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
     options: TopologyOptions
   ) {
     super();
+    this.on('error', noop);
 
     this.client = client;
     // Options should only be undefined in tests, MongoClient will always have defined options
diff --git a/src/sessions.ts b/src/sessions.ts
@@ -43,6 +43,7 @@ import {
   isPromiseLike,
   List,
   maxWireVersion,
+  noop,
   now,
   squashError,
   uuidV4
@@ -161,6 +162,7 @@ export class ClientSession
     clientOptions: MongoOptions
   ) {
     super();
+    this.on('error', noop);
 
     if (client == null) {
       // TODO(NODE-3483)
diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts
@@ -736,6 +736,7 @@ describe('Change Streams', function () {
           // ChangeStream detects emitter usage via 'newListener' event
           // so this covers all emitter methods
         });
+        changeStream.on('error', () => null); // one must listen for errors if they use EE mode.
 
         await once(changeStream.cursor, 'init');
         expect(changeStream).to.have.property('mode', 'emitter');
@@ -971,7 +972,7 @@ describe('Change Streams', function () {
           { requires: { topology: '!single' } },
           async function () {
             changeStream = collection.watch([]);
-            changeStream.on('change', sinon.stub());
+            changeStream.on('change', sinon.stub()).on('error', () => null);
 
             try {
               // eslint-disable-next-line @typescript-eslint/no-unused-vars
diff --git a/test/integration/change-streams/change_streams.prose.test.ts b/test/integration/change-streams/change_streams.prose.test.ts
@@ -858,6 +858,7 @@ describe('Change Stream prose tests', function () {
         expect(err).to.not.exist;
         coll = client.db('integration_tests').collection('setupAfterTest');
         const changeStream = coll.watch();
+        changeStream.on('error', done);
         waitForStarted(changeStream, () => {
           coll.insertOne({ x: 1 }, { writeConcern: { w: 'majority', j: true } }, err => {
             expect(err).to.not.exist;
@@ -932,6 +933,7 @@ describe('Change Stream prose tests', function () {
         let events = [];
         client.on('commandStarted', e => recordEvent(events, e));
         const changeStream = coll.watch([], { startAfter });
+        changeStream.on('error', done);
         this.defer(() => changeStream.close());
 
         changeStream.on('change', change => {
diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts
@@ -1,7 +1,8 @@
 import { expect } from 'chai';
-import { on } from 'events';
+import { once } from 'events';
 import * as semver from 'semver';
 import * as sinon from 'sinon';
+import { finished } from 'stream/promises';
 
 import {
   Collection,
@@ -238,7 +239,7 @@ describe('CRUD API', function () {
   });
 
   context('when creating a cursor with find', () => {
-    let collection;
+    let collection: Collection;
 
     beforeEach(async () => {
       collection = client.db().collection('t');
@@ -307,13 +308,14 @@ describe('CRUD API', function () {
 
     describe('#stream()', () => {
       it('creates a node stream that emits data events', async () => {
-        const count = 0;
-        const cursor = makeCursor();
-        const stream = cursor.stream();
-        on(stream, 'data');
-        cursor.once('close', function () {
-          expect(count).to.equal(2);
+        let count = 0;
+        const stream = makeCursor().stream();
+        const willFinish = finished(stream, { cleanup: true });
+        stream.on('data', () => {
+          count++;
         });
+        await willFinish;
+        expect(count).to.equal(2);
       });
     });
 
diff --git a/test/integration/crud/misc_cursors.test.js b/test/integration/crud/misc_cursors.test.js
@@ -1993,15 +1993,15 @@ describe('Cursor', function () {
             expect(res).property('insertedId').to.exist;
           }, 300);
 
-          const start = new Date();
+          const start = performance.now();
           const doc1 = await cursor.next();
           expect(doc1).to.have.property('b', 2);
-          const end = new Date();
+          const end = performance.now();
 
           await later; // make sure this finished, without a failure
 
           // We should see here that cursor.next blocked for at least 300ms
-          expect(end.getTime() - start.getTime()).to.be.at.least(300);
+          expect(end - start).to.be.at.least(290);
         }
       }
     );
diff --git a/test/integration/node-specific/examples/change_streams.test.js b/test/integration/node-specific/examples/change_streams.test.js
@@ -66,9 +66,13 @@ maybeDescribe('examples(change-stream):', function () {
       // Start Changestream Example 1
       const collection = db.collection('inventory');
       const changeStream = collection.watch();
-      changeStream.on('change', next => {
-        // process next document
-      });
+      changeStream
+        .on('change', next => {
+          // process next document
+        })
+        .once('error', () => {
+          // handle error
+        });
       // End Changestream Example 1
 
       const changeStreamIterator = collection.watch();
@@ -113,9 +117,13 @@ maybeDescribe('examples(change-stream):', function () {
       // Start Changestream Example 2
       const collection = db.collection('inventory');
       const changeStream = collection.watch([], { fullDocument: 'updateLookup' });
-      changeStream.on('change', next => {
-        // process next document
-      });
+      changeStream
+        .on('change', next => {
+          // process next document
+        })
+        .once('error', error => {
+          // handle error
+        });
       // End Changestream Example 2
 
       // Start Changestream Example 2 Alternative
@@ -151,15 +159,23 @@ maybeDescribe('examples(change-stream):', function () {
       const changeStream = collection.watch();
 
       let newChangeStream;
-      changeStream.once('change', next => {
-        const resumeToken = changeStream.resumeToken;
-        changeStream.close();
-
-        newChangeStream = collection.watch([], { resumeAfter: resumeToken });
-        newChangeStream.on('change', next => {
-          processChange(next);
+      changeStream
+        .once('change', next => {
+          const resumeToken = changeStream.resumeToken;
+          changeStream.close();
+
+          newChangeStream = collection.watch([], { resumeAfter: resumeToken });
+          newChangeStream
+            .on('change', next => {
+              processChange(next);
+            })
+            .once('error', error => {
+              // handle error
+            });
+        })
+        .once('error', error => {
+          // handle error
         });
-      });
       // End Changestream Example 3
 
       // Start Changestream Example 3 Alternative
@@ -200,9 +216,13 @@ maybeDescribe('examples(change-stream):', function () {
 
       const collection = db.collection('inventory');
       const changeStream = collection.watch(pipeline);
-      changeStream.on('change', next => {
-        // process next document
-      });
+      changeStream
+        .on('change', next => {
+          // process next document
+        })
+        .once('error', error => {
+          // handle error
+        });
       // End Changestream Example 4
 
       // Start Changestream Example 4 Alternative
diff --git a/test/integration/sessions/sessions.test.ts b/test/integration/sessions/sessions.test.ts
@@ -430,13 +430,14 @@ describe('Sessions Spec', function () {
     });
   });
 
-  context('when using a LegacyMongoClient', () => {
+  context.skip('when using a LegacyMongoClient', () => {
     let legacyClient;
     beforeEach(async function () {
       const options = this.configuration.serverApi
         ? { serverApi: this.configuration.serverApi }
         : {};
       legacyClient = new LegacyMongoClient(this.configuration.url(), options);
+      legacyClient.on('error', () => null); // Uses released version of the driver so it won't be fixed until the error listeners are published
     });
 
     afterEach(async function () {
diff --git a/test/mocha_mongodb.json b/test/mocha_mongodb.json
@@ -3,9 +3,9 @@
   "require": [
     "source-map-support/register",
     "ts-node/register",
+    "test/tools/runner/ee_checker.ts",
     "test/tools/runner/chai_addons.ts",
     "test/tools/runner/hooks/configuration.ts",
-    "test/tools/runner/hooks/unhandled_checker.ts",
     "test/tools/runner/hooks/leak_checker.ts",
     "test/tools/runner/hooks/legacy_crud_shims.ts"
   ],
diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts
diff --git a/test/tools/reporter/mongodb_reporter.js b/test/tools/reporter/mongodb_reporter.js
diff --git a/test/tools/runner/ee_checker.ts b/test/tools/runner/ee_checker.ts
diff --git a/test/tools/runner/hooks/unhandled_checker.ts b/test/tools/runner/hooks/unhandled_checker.ts
diff --git a/test/tools/unified-spec-runner/entities.ts b/test/tools/unified-spec-runner/entities.ts
diff --git a/test/unit/sdam/srv_polling.test.ts b/test/unit/sdam/srv_polling.test.ts
