diff --git a/src/__tests__/server.ts b/src/__tests__/server.ts index ea2b1de8..74758e7b 100644 --- a/src/__tests__/server.ts +++ b/src/__tests__/server.ts @@ -635,6 +635,61 @@ describe('Connect', () => { expect(event.wasClean).toBeTruthy(); }); }); + + it("should have acknowledged connection even if ack message send didn't resolve", (done) => { + let sent: Promise | null = null; + let resolveSend = () => { + // noop + }; + makeServer({ + schema, + onSubscribe(ctx) { + expect(ctx.acknowledged).toBeTruthy(); + resolveSend(); + done(); + }, + }).opened( + { + protocol: GRAPHQL_TRANSPORT_WS_PROTOCOL, + send: async () => { + // if already set, this is a subsequent send happening after the test + if (sent) { + return; + } + + // message was sent and delivered to the client... + sent = new Promise((resolve) => { + resolve(); + }); + await sent; + + // ...but something else is slow - leading to a potential race condition on the `acknowledged` flag + await new Promise((resolve) => (resolveSend = resolve)); + }, + close: (code, reason) => { + fail(`Unexpected close with ${code}: ${reason}`); + }, + onMessage: async (cb) => { + cb(stringifyMessage({ type: MessageType.ConnectionInit })); + await sent; + cb( + stringifyMessage({ + id: '1', + type: MessageType.Subscribe, + payload: { query: '{ getValue }' }, + }), + ); + }, + onPing: () => { + /**/ + }, + onPong: () => { + /**/ + }, + }, + {}, + ); + }); }); describe('Ping/Pong', () => { diff --git a/src/server.ts b/src/server.ts index 32860347..94b585cb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -613,6 +613,11 @@ export function makeServer< if (permittedOrPayload === false) return socket.close(CloseCode.Forbidden, 'Forbidden'); + // we should acknowledge before send to avoid race conditions (like those exampled in https://github.com/enisdenjo/graphql-ws/issues/501) + // even if the send fails/throws, the connection should be closed because its malfunctioning + // @ts-expect-error: I can write + ctx.acknowledged = true; + await socket.send( stringifyMessage( isObject(permittedOrPayload) @@ -627,9 +632,6 @@ export function makeServer< replacer, ), ); - - // @ts-expect-error: I can write - ctx.acknowledged = true; return; } case MessageType.Ping: {