From c332837c50da09174a9741a1903a5471f8a1c42d Mon Sep 17 00:00:00 2001 From: enisdenjo Date: Fri, 19 Aug 2022 15:30:25 +0200 Subject: [PATCH] fix(client): Debounce close by `lazyCloseTimeout` Closes #388 --- src/__tests__/client.ts | 35 +++++++++++++++++++++++++++++++++++ src/client.ts | 19 +++++++++++-------- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 0d5ce1dc..9f30767b 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -1393,6 +1393,41 @@ describe('lazy', () => { await server.waitForClientClose(); }); + it('should debounce close by lazyCloseTimeout', async () => { + const { url, ...server } = await startTServer(); + + const client = createClient({ + url, + lazy: true, // default + lazyCloseTimeout: 10, + retryAttempts: 0, + onNonLazyError: noop, + }); + + // loop subscriptions and delay them by 5ms (while lazy close is 10ms) + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + + await new Promise((resolve, reject) => { + client.subscribe( + { query: '{ getValue }' }, + { + next: () => { + // noop + }, + error: reject, + complete: resolve, + }, + ); + }); + } + + // if the debounce is set up incorrectly, a leftover timeout might close the connection earlier + await server.waitForClientClose(() => { + fail("Client shouldn't have closed"); + }, 5); + }); + it('should report errors to the `onNonLazyError` callback', async (done) => { const { url, ...server } = await startTServer(); diff --git a/src/client.ts b/src/client.ts index 2bc66559..000e7dae 100644 --- a/src/client.ts +++ b/src/client.ts @@ -468,7 +468,7 @@ export function createClient< connectionParams, lazy = true, onNonLazyError = console.error, - lazyCloseTimeout = 0, + lazyCloseTimeout: lazyCloseTimeoutMs = 0, keepAlive = 0, disablePong, connectionAckWaitTimeout = 0, @@ -600,6 +600,7 @@ export function createClient< type Connected = [socket: WebSocket, throwOnClose: Promise]; let connecting: Promise | undefined, locks = 0, + lazyCloseTimeout: ReturnType, retrying = false, retries = 0, disposed = false; @@ -610,6 +611,10 @@ export function createClient< waitForReleaseOrThrowOnClose: Promise, ] > { + // clear the lazy close timeout immediatelly so that close gets debounced + // see: https://github.com/enisdenjo/graphql-ws/issues/388 + clearTimeout(lazyCloseTimeout); + const [socket, throwOnClose] = await (connecting ?? (connecting = new Promise((connected, denied) => (async () => { @@ -787,14 +792,12 @@ export function createClient< if (!locks) { // and if no more locks are present, complete the connection const complete = () => socket.close(1000, 'Normal Closure'); - if (isFinite(lazyCloseTimeout) && lazyCloseTimeout > 0) { + if (isFinite(lazyCloseTimeoutMs) && lazyCloseTimeoutMs > 0) { // if the keepalive is set, allow for the specified calmdown time and - // then complete. but only if no lock got created in the meantime and - // if the socket is still open - setTimeout(() => { - if (!locks && socket.readyState === WebSocketImpl.OPEN) - complete(); - }, lazyCloseTimeout); + // then complete if the socket is still open. + lazyCloseTimeout = setTimeout(() => { + if (socket.readyState === WebSocketImpl.OPEN) complete(); + }, lazyCloseTimeoutMs); } else { // otherwise complete immediately complete();