Skip to content

Commit fc03004

Browse files
authored
fix: do not send Complete after Error for subscriptions (#667)
1 parent 6a31f46 commit fc03004

File tree

3 files changed

+66
-0
lines changed

3 files changed

+66
-0
lines changed

.changeset/khaki-buses-trade.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'graphql-ws': patch
3+
---
4+
5+
Fix the server sending a `Complete` message after an `Error` message for subscriptions.
6+
7+
Previously, when a subscription's async iterable threw an error, the server would send:
8+
9+
```
10+
{"id":"1","type":"error","payload":[{"message":"..."}]}
11+
{"id":"1","type":"complete"}
12+
```
13+
14+
Per the protocol spec:
15+
16+
> **Error:** This message terminates the operation and no further messages will be sent.
17+
18+
> **Complete (Server → Client):** If the server dispatched the `Error` message relative to the original `Subscribe` message, no `Complete` message will be emitted.
19+
20+
The server now correctly sends only the `Error` message:
21+
22+
```
23+
{"id":"1","type":"error","payload":[{"message":"..."}]}
24+
```
25+
26+
Clients that correctly follow the spec should be unaffected, as they are expected to ignore messages for operations they consider already completed.

src/server.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,9 @@ export function makeServer<
880880
],
881881
message,
882882
);
883+
// error terminates the operation per spec — prevent
884+
// emit.complete from notifying the client
885+
delete ctx.subscriptions[id];
883886
}
884887
}
885888
} else {

tests/server.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,6 +2015,43 @@ describe.concurrent('Subscribe', () => {
20152015
);
20162016
});
20172017
});
2018+
2019+
it('should not send complete after error when subscription iterable throws', async ({
2020+
expect,
2021+
}) => {
2022+
const server = await startTServer();
2023+
2024+
const client = await createTClient(server.url);
2025+
2026+
client.ws.send(
2027+
stringifyMessage({
2028+
type: MessageType.ConnectionInit,
2029+
}),
2030+
);
2031+
await client.waitForMessage(); // MessageType.ConnectionAck
2032+
2033+
client.ws.send(
2034+
stringifyMessage({
2035+
id: '1',
2036+
type: MessageType.Subscribe,
2037+
payload: {
2038+
query: 'subscription { throwing }',
2039+
},
2040+
}),
2041+
);
2042+
await server.waitForOperation();
2043+
2044+
await client.waitForMessage((msg) => {
2045+
expect(msg.data).toMatchInlineSnapshot(
2046+
`"{"id":"1","type":"error","payload":[{"message":"Subscribe Kaboom!"}]}"`,
2047+
);
2048+
});
2049+
2050+
// per the protocol spec, no complete message should follow an error
2051+
await client.waitForMessage((msg) => {
2052+
throw new Error(`Unexpected message after error: ${msg.data}`);
2053+
}, 30);
2054+
});
20182055
});
20192056

20202057
describe.concurrent('Disconnect/close', () => {

0 commit comments

Comments
 (0)