Skip to content

Commit

Permalink
test: check that event emitters have error listeners in the same tick…
Browse files Browse the repository at this point in the history
… of their creation
  • Loading branch information
nbbeeken committed Jan 28, 2025
1 parent 654069f commit fbad7ba
Show file tree
Hide file tree
Showing 21 changed files with 54 additions and 49 deletions.
2 changes: 1 addition & 1 deletion .mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions src/change_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,11 @@ export class ChangeStream<
options: ChangeStreamOptions = {}
) {
super();
const ctorCallSite = new Error('ChangeStream must add an error listener synchronously');
ctorCallSite.stack;

const squishErrorListenerRequirement = () => null;
this.on('error', squishErrorListenerRequirement);

this.pipeline = pipeline;
this.options = { ...options };
Expand Down Expand Up @@ -667,7 +672,11 @@ export class ChangeStream<
// Listen for any `change` listeners being added to ChangeStream
this.on('newListener', eventName => {
if (eventName === 'change' && this.cursor && this.listenerCount('change') === 0) {
this.off('error', squishErrorListenerRequirement);
this._streamEvents(this.cursor);
process.nextTick(() => {
if (this.listenerCount('error') === 0) throw ctorCallSite;
});
}
});

Expand Down
1 change: 1 addition & 0 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {

constructor(stream: Stream, options: ConnectionOptions) {
super();
this.on('error', () => null);

this.socket = stream;
this.id = options.id;
Expand Down
1 change: 1 addition & 0 deletions src/cmap/connection_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {

constructor(server: Server, options: ConnectionPoolOptions) {
super();
this.on('error', () => null);

this.options = Object.freeze({
connectionType: Connection,
Expand Down
1 change: 1 addition & 0 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ export abstract class AbstractCursor<
options: AbstractCursorOptions & Abortable = {}
) {
super();
this.on('error', () => null);

if (!client.s.isMongoClient) {
throw new MongoRuntimeError('Cursor must be constructed with MongoClient');
Expand Down
1 change: 1 addition & 0 deletions src/gridfs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {

constructor(db: Db, options?: GridFSBucketOptions) {
super();
this.on('error', () => null);
this.setMaxListeners(0);
const privateOptions = resolveOptions(db, {
...DEFAULT_GRIDFS_BUCKET_OPTIONS,
Expand Down
1 change: 1 addition & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements

constructor(url: string, options?: MongoClientOptions) {
super();
this.on('error', () => null);

this.options = parseOptions(url, this, options);

Expand Down
7 changes: 6 additions & 1 deletion src/mongo_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,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', () => null);
}
}

/** @public */
export type Abortable = {
Expand Down
1 change: 1 addition & 0 deletions src/sdam/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export class Monitor extends TypedEventEmitter<MonitorEvents> {

constructor(server: Server, options: MonitorOptions) {
super();
this.on('error', () => null);

this.server = server;
this.connection = null;
Expand Down
1 change: 1 addition & 0 deletions src/sdam/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class Server extends TypedEventEmitter<ServerEvents> {
*/
constructor(topology: Topology, description: ServerDescription, options: ServerOptions) {
super();
this.on('error', () => null);

this.serverApi = options.serverApi;

Expand Down
1 change: 1 addition & 0 deletions src/sdam/srv_polling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export class SrvPoller extends TypedEventEmitter<SrvPollerEvents> {

constructor(options: SrvPollerOptions) {
super();
this.on('error', () => null);

if (!options || !options.srvHost) {
throw new MongoRuntimeError('Options for SrvPoller must exist and include srvHost');
Expand Down
1 change: 1 addition & 0 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
options: TopologyOptions
) {
super();
this.on('error', () => null);

this.client = client;
// Options should only be undefined in tests, MongoClient will always have defined options
Expand Down
1 change: 1 addition & 0 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ export class ClientSession
clientOptions: MongoOptions
) {
super();
this.on('error', () => null);

if (client == null) {
// TODO(NODE-3483)
Expand Down
3 changes: 2 additions & 1 deletion test/integration/change-streams/change_stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/integration/change-streams/change_streams.prose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion test/mocha_mongodb.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
],
Expand Down
1 change: 0 additions & 1 deletion test/tools/reporter/mongodb_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ class MongoDBMochaReporter extends mocha.reporters.Spec {
*/
fail(test, error) {
if (REPORT_TO_STDIO) console.log(chalk.red(`⨯ ${test.fullTitle()} -- ${error.message}`));
test.error = error;
}

/**
Expand Down
17 changes: 17 additions & 0 deletions test/tools/runner/ee_checker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// eslint-disable-next-line @typescript-eslint/no-require-imports
const events = require('events');

const EventEmitter = events.EventEmitter;

events.EventEmitter = class RequireErrorListenerEventEmitter extends EventEmitter {
constructor(...args) {
super(...args);
const ctorCallSite = new Error('EventEmitter must add an error listener synchronously');
ctorCallSite.stack;
process.nextTick(() => {
if (this.listenerCount('error') === 0) {
throw ctorCallSite;
}
});
}
};
44 changes: 0 additions & 44 deletions test/tools/runner/hooks/unhandled_checker.ts

This file was deleted.

2 changes: 2 additions & 0 deletions test/tools/unified-spec-runner/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ export class UnifiedMongoClient extends MongoClient {
// TODO(NODE-5785): We need to increase the truncation length because signature.hash is a Buffer making hellos too long
mongodbLogMaxDocumentLength: 1250
} as any);

this.observedEventEmitter.on('error', () => null);
this.logCollector = logCollector;
this.observeSensitiveCommands = description.observeSensitiveCommands ?? false;

Expand Down
4 changes: 4 additions & 0 deletions test/unit/sdam/srv_polling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ describe('Mongos SRV Polling', function () {

describe('topology', function () {
class FakeSrvPoller extends EventEmitter {
constructor() {
super();
this.on('error', () => null);
}
start() {
// ignore
}
Expand Down

0 comments on commit fbad7ba

Please sign in to comment.