From 057e9d913ad268ea2e148c6a5fe41be24e150ba1 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Thu, 8 Apr 2021 20:17:48 +0200 Subject: [PATCH 1/2] fix: correct removeAllListeners in case no event is passed Correct handling of patched removeAllListeners() in AbstractAsyncHooksContextManager in case it's called without argument. In this case all listener types should be removed. Additionally it's imporant to forward arguments instead of passing undefined as node.js checks arguments.length. --- .../src/AbstractAsyncHooksContextManager.ts | 10 +++++++--- .../test/AsyncHooksContextManager.test.ts | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts index 1a4db11c93e..c263ad3a24d 100644 --- a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts @@ -146,10 +146,14 @@ export abstract class AbstractAsyncHooksContextManager const contextManager = this; return function (this: never, event: string) { const map = contextManager._getPatchMap(ee); - if (map?.[event] !== undefined) { - delete map[event]; + if (map != null) { + if (arguments.length === 0) { + contextManager._createPatchMap(ee); + } else if (map[event] !== undefined) { + delete map[event]; + } } - return original.call(this, event); + return original.apply(this, arguments); }; } diff --git a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts index 5926c9485b3..ba0a169e915 100644 --- a/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts +++ b/packages/opentelemetry-context-async-hooks/test/AsyncHooksContextManager.test.ts @@ -415,6 +415,24 @@ for (const contextManagerClass of [ patchedEE.emit('test'); }); + it('should return current context and removeAllListeners (when enabled)', done => { + const ee = new EventEmitter(); + const context = ROOT_CONTEXT.setValue(key1, 1); + const patchedEE = contextManager.bind(ee, context); + const handler = () => { + assert.deepStrictEqual(contextManager.active(), context); + patchedEE.removeAllListeners(); + assert.strictEqual(patchedEE.listeners('test').length, 0); + assert.strictEqual(patchedEE.listeners('test1').length, 0); + return done(); + }; + patchedEE.on('test', handler); + patchedEE.on('test1', handler); + assert.strictEqual(patchedEE.listeners('test').length, 1); + assert.strictEqual(patchedEE.listeners('test1').length, 1); + patchedEE.emit('test'); + }); + /** * Even if asynchooks is disabled, the context propagation will * still works but it might be lost after any async op. From ab8683a7a55409723106ec0d98ae71fbda07870a Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Thu, 8 Apr 2021 20:30:12 +0200 Subject: [PATCH 2/2] fixup! use strict equal and Object.create(null) --- .../src/AbstractAsyncHooksContextManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts index c263ad3a24d..0f540b62bb8 100644 --- a/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts +++ b/packages/opentelemetry-context-async-hooks/src/AbstractAsyncHooksContextManager.ts @@ -146,7 +146,7 @@ export abstract class AbstractAsyncHooksContextManager const contextManager = this; return function (this: never, event: string) { const map = contextManager._getPatchMap(ee); - if (map != null) { + if (map !== undefined) { if (arguments.length === 0) { contextManager._createPatchMap(ee); } else if (map[event] !== undefined) { @@ -188,7 +188,7 @@ export abstract class AbstractAsyncHooksContextManager } private _createPatchMap(ee: EventEmitter): PatchMap { - const map = {}; + const map = Object.create(null); // eslint-disable-next-line @typescript-eslint/no-explicit-any (ee as any)[this._kOtListeners] = map; return map;