From 62836c83111750b23c783be857c88be410d1faa0 Mon Sep 17 00:00:00 2001 From: Alexander Kureniov Date: Mon, 16 Dec 2019 00:35:35 +0200 Subject: [PATCH 1/4] Fix TypeError: this.queue.close is not a function In some rare cases there's a possibility when `close` will be called before `subscribe` will happen. In this case that error could cause even app server crash. Also it prevents `this.queue.destroy()` to be called. I don't know how to properly test this case, cause it's caused by some abnormal behaviour of client side. Tried some options with closing ws client right after establishing connection, sending some crappy staff into it - nothing caused this bug to appear. Anyway it's present and needed to be fixed. --- lib/subscriber.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/subscriber.js b/lib/subscriber.js index 8761046b..f0bce6e1 100644 --- a/lib/subscriber.js +++ b/lib/subscriber.js @@ -61,7 +61,9 @@ class SubscriptionContext { } close () { - this.queue.close() + if (typeof this.queue.close === 'function') { + this.queue.close() + } this.queue.destroy() } } From 7a1b3ea90ad71f7c2cde4a5be9b57089e58e7f2a Mon Sep 17 00:00:00 2001 From: Alexander Kureniov Date: Mon, 16 Dec 2019 00:49:50 +0200 Subject: [PATCH 2/4] fix: ignore coverage --- lib/subscriber.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/subscriber.js b/lib/subscriber.js index f0bce6e1..03273dab 100644 --- a/lib/subscriber.js +++ b/lib/subscriber.js @@ -61,6 +61,7 @@ class SubscriptionContext { } close () { + /* istanbul ignore else */ if (typeof this.queue.close === 'function') { this.queue.close() } From a0b2e796311e2856e04bd7d49344aee7f41065db Mon Sep 17 00:00:00 2001 From: Alexander Kureniov Date: Mon, 16 Dec 2019 10:47:28 +0200 Subject: [PATCH 3/4] chore: add comments --- lib/subscriber.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/subscriber.js b/lib/subscriber.js index 03273dab..d37e28cd 100644 --- a/lib/subscriber.js +++ b/lib/subscriber.js @@ -61,6 +61,8 @@ class SubscriptionContext { } close () { + // In rare cases when `subscribe()` not called (e.g. some network error) + // `close` will be `undefined`. /* istanbul ignore else */ if (typeof this.queue.close === 'function') { this.queue.close() From 3482b5bc31951dd760132350219d9ed80f6c7456 Mon Sep 17 00:00:00 2001 From: Alexander Kureniov <928021-m03geek@users.noreply.gitlab.com> Date: Wed, 18 Dec 2019 19:05:26 +0200 Subject: [PATCH 4/4] test: add test --- lib/subscriber.js | 1 - test/subscriber.js | 10 ++++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/subscriber.js b/lib/subscriber.js index d37e28cd..bc7cdb12 100644 --- a/lib/subscriber.js +++ b/lib/subscriber.js @@ -63,7 +63,6 @@ class SubscriptionContext { close () { // In rare cases when `subscribe()` not called (e.g. some network error) // `close` will be `undefined`. - /* istanbul ignore else */ if (typeof this.queue.close === 'function') { this.queue.close() } diff --git a/test/subscriber.js b/test/subscriber.js index 2cbdac68..baa0e886 100644 --- a/test/subscriber.js +++ b/test/subscriber.js @@ -19,6 +19,16 @@ test('subscriber published an event', async (t) => { }) }) +test('subscription context not throw error on close', t => { + t.plan(1) + const pubsub = new PubSub(mq()) + + const sc = new SubscriptionContext({ pubsub }) + + sc.close() + t.pass() +}) + test('subscription context publish event returns a promise', t => { t.plan(1) const pubsub = new PubSub(mq())