From 44cea164ccc147ad69a87294f925621d8197f93e Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Tue, 30 Jun 2020 11:59:15 -0300 Subject: [PATCH 1/4] fix: do not send GQL_STOP after GQL_COMPLETED is received --- src/client.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client.ts b/src/client.ts index 459d67d26..ce4d7fad6 100644 --- a/src/client.ts +++ b/src/client.ts @@ -42,6 +42,7 @@ export type FormatedError = Error & { export interface Operation { options: OperationOptions; handler: (error: Error[], result?: any) => void; + completed?: boolean; } export interface Operations { @@ -623,6 +624,7 @@ export class SubscriptionClient { break; case MessageTypes.GQL_COMPLETE: + this.operations[opId].completed = true; this.operations[opId].handler(null, null); delete this.operations[opId]; break; @@ -662,7 +664,9 @@ export class SubscriptionClient { if (this.operations[opId]) { delete this.operations[opId]; this.setInactivityTimeout(); - this.sendMessage(opId, MessageTypes.GQL_STOP, undefined); + if (!this.operations[opId].completed) { + this.sendMessage(opId, MessageTypes.GQL_STOP, undefined); + } } } } From d166fcb8613cbb05fd04262ba738e28d2c342d90 Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Tue, 30 Jun 2020 13:41:10 -0300 Subject: [PATCH 2/4] added unit tests --- src/client.ts | 9 +++------ src/test/tests.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/client.ts b/src/client.ts index ce4d7fad6..67efc50b3 100644 --- a/src/client.ts +++ b/src/client.ts @@ -42,7 +42,6 @@ export type FormatedError = Error & { export interface Operation { options: OperationOptions; handler: (error: Error[], result?: any) => void; - completed?: boolean; } export interface Operations { @@ -624,9 +623,9 @@ export class SubscriptionClient { break; case MessageTypes.GQL_COMPLETE: - this.operations[opId].completed = true; - this.operations[opId].handler(null, null); + let handler = this.operations[opId].handler; delete this.operations[opId]; + handler.call(this, null, null); break; case MessageTypes.GQL_ERROR: @@ -664,9 +663,7 @@ export class SubscriptionClient { if (this.operations[opId]) { delete this.operations[opId]; this.setInactivityTimeout(); - if (!this.operations[opId].completed) { - this.sendMessage(opId, MessageTypes.GQL_STOP, undefined); - } + this.sendMessage(opId, MessageTypes.GQL_STOP, undefined); } } } diff --git a/src/test/tests.ts b/src/test/tests.ts index f72124e36..cc53f3396 100644 --- a/src/test/tests.ts +++ b/src/test/tests.ts @@ -1195,6 +1195,33 @@ describe('Client', function () { done(); }); + it('should not send GQL_STOP on unsubscribe after operation received a GQL_COMPLETE', (done) => { + const client = new SubscriptionClient(`ws://localhost:${TEST_PORT}/`); + const sendMessageSpy = sinon.spy(client as any, 'sendMessage'); + + client.onConnected(() => { + const subscription = client.request({ + query: `subscription { somethingChanged }`, + variables: {}, + }).subscribe({ + next: () => { + client.client.onmessage({ + data: JSON.stringify({id: 1, type: MessageTypes.GQL_COMPLETE}), + }); + }, + complete: () => { + subscription.unsubscribe(); + }, + }); + setTimeout(() => { + expect(sendMessageSpy.calledWith(undefined, 'connection_init', sinon.match.any)).to.be.true; + expect(sendMessageSpy.calledWith('1', 'start', sinon.match.any)).to.be.true; + expect(sendMessageSpy.calledWith('1', 'stop', sinon.match.any)).to.be.false; + done(); + }, 1000); + }); + }); + it('should delete operation when receive a GQL_COMPLETE', (done) => { const subscriptionsClient = new SubscriptionClient(`ws://localhost:${RAW_TEST_PORT}/`); subscriptionsClient.operations['1'] = { From 3a544812d704bd5a842afba0869ea1d063748425 Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Tue, 30 Jun 2020 13:42:46 -0300 Subject: [PATCH 3/4] fix failed build --- src/client.ts | 2 +- src/utils/empty-iterable.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.ts b/src/client.ts index 67efc50b3..bd84375b5 100644 --- a/src/client.ts +++ b/src/client.ts @@ -623,7 +623,7 @@ export class SubscriptionClient { break; case MessageTypes.GQL_COMPLETE: - let handler = this.operations[opId].handler; + const handler = this.operations[opId].handler; delete this.operations[opId]; handler.call(this, null, null); break; diff --git a/src/utils/empty-iterable.ts b/src/utils/empty-iterable.ts index 9ddccc128..41f98868d 100644 --- a/src/utils/empty-iterable.ts +++ b/src/utils/empty-iterable.ts @@ -14,5 +14,5 @@ export const createEmptyIterable = (): AsyncIterator => { [$$asyncIterator]() { return this; }, - }; + } as any; }; From c3eada33023a16697d93034c4b7e79c1a4067b7d Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Tue, 30 Jun 2020 14:09:54 -0300 Subject: [PATCH 4/4] CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34ec97cab..99b21b7b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +### vNEXT +- Do not send GQL_STOP when unsubscribing after GQL_COMPLETE is received [Issue #765](https://github.com/apollographql/subscriptions-transport-ws/issues/711) + ### v0.9.16 - Add ability to set custom WebSocket protocols for client.
[@pkosiec](https://github.com/pkosiec) in [#477](https://github.com/apollographql/subscriptions-transport-ws/pull/477)