From 625be7585dba1f6f8ae8fce484c641a019286696 Mon Sep 17 00:00:00 2001 From: simon-id Date: Tue, 19 Oct 2021 11:56:42 +0200 Subject: [PATCH] lib: add return value for DC channel.unsubscribe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/40433 Reviewed-By: Vladimir de Turckheim Reviewed-By: Stephen Belanger Reviewed-By: Gerhard Stöbich Reviewed-By: Michael Dawson Reviewed-By: Bryan English Reviewed-By: Zijian Liu --- doc/api/diagnostics_channel.md | 10 ++++++++++ lib/diagnostics_channel.js | 16 +++++++++------- ...diagnostics-channel-object-channel-pub-sub.js | 5 ++++- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/api/diagnostics_channel.md b/doc/api/diagnostics_channel.md index 16b47db3bf6cc4..90a3b1fb6ba41c 100644 --- a/doc/api/diagnostics_channel.md +++ b/doc/api/diagnostics_channel.md @@ -156,7 +156,17 @@ channel.subscribe((message, name) => { #### `channel.unsubscribe(onMessage)` + + * `onMessage` {Function} The previous subscribed handler to remove +* Returns: {boolean} `true` if the handler was found, `false` otherwise. Remove a message handler previously registered to this channel with [`channel.subscribe(onMessage)`][]. diff --git a/lib/diagnostics_channel.js b/lib/diagnostics_channel.js index c29c9ff0052405..7860e8934494d2 100644 --- a/lib/diagnostics_channel.js +++ b/lib/diagnostics_channel.js @@ -32,15 +32,17 @@ class ActiveChannel { unsubscribe(subscription) { const index = ArrayPrototypeIndexOf(this._subscribers, subscription); - if (index >= 0) { - ArrayPrototypeSplice(this._subscribers, index, 1); + if (index === -1) return false; - // When there are no more active subscribers, restore to fast prototype. - if (!this._subscribers.length) { - // eslint-disable-next-line no-use-before-define - ObjectSetPrototypeOf(this, Channel.prototype); - } + ArrayPrototypeSplice(this._subscribers, index, 1); + + // When there are no more active subscribers, restore to fast prototype. + if (!this._subscribers.length) { + // eslint-disable-next-line no-use-before-define + ObjectSetPrototypeOf(this, Channel.prototype); } + + return true; } get hasSubscribers() { diff --git a/test/parallel/test-diagnostics-channel-object-channel-pub-sub.js b/test/parallel/test-diagnostics-channel-object-channel-pub-sub.js index cbc5b4d2e9a953..9498419b806ca8 100644 --- a/test/parallel/test-diagnostics-channel-object-channel-pub-sub.js +++ b/test/parallel/test-diagnostics-channel-object-channel-pub-sub.js @@ -35,9 +35,12 @@ assert.ok(channel instanceof Channel); channel.publish(input); // Should not publish after subscriber is unsubscribed -channel.unsubscribe(subscriber); +assert.ok(channel.unsubscribe(subscriber)); assert.ok(!channel.hasSubscribers); +// unsubscribe() should return false when subscriber is not found +assert.ok(!channel.unsubscribe(subscriber)); + assert.throws(() => { channel.subscribe(null); }, { code: 'ERR_INVALID_ARG_TYPE' });