Skip to content

Commit

Permalink
fix(server): should not send error messages if socket closes before o…
Browse files Browse the repository at this point in the history
…nSubscribe hooks resolves

Closes #539
  • Loading branch information
enisdenjo committed Mar 27, 2024
1 parent f442288 commit db47a66
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 5 deletions.
59 changes: 59 additions & 0 deletions src/__tests__/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,65 @@ describe('Subscribe', () => {
fail("Shouldn't have received a message");
}, 20);
});

it('should not send error messages if socket closes before onSubscribe hooks resolves', async () => {
let resolveOnSubscribe: () => void = () => {
throw new Error('On subscribe resolved early');
};
const waitForOnSubscribe = new Promise<void>(
(resolve) => (resolveOnSubscribe = resolve),
);

let resolveSubscribe: () => void = () => {
throw new Error('Subscribe resolved early');
};

const sendFn = jest.fn();

const closed = makeServer({
schema,
async onSubscribe() {
resolveOnSubscribe();
await new Promise<void>((resolve) => (resolveSubscribe = resolve));
return [new GraphQLError('Oopsie!')];
},
}).opened(
{
protocol: GRAPHQL_TRANSPORT_WS_PROTOCOL,
send: sendFn,
close: () => {
// noop
},
onMessage: async (cb) => {
await cb(stringifyMessage({ type: MessageType.ConnectionInit }));
await cb(
stringifyMessage({
id: '1',
type: MessageType.Subscribe,
payload: { query: '{ getValue }' },
}),
);
},
onPing: () => {
/**/
},
onPong: () => {
/**/
},
},
{},
);

await waitForOnSubscribe;

closed(4321, 'Bye bye!');

resolveSubscribe();

await new Promise((resolve) => setTimeout(resolve, 20));

expect(sendFn).toBeCalledTimes(1); // only the ack message
});
});

describe('Disconnect/close', () => {
Expand Down
16 changes: 11 additions & 5 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,9 @@ export function makeServer<
const maybeExecArgsOrErrors = await onSubscribe?.(ctx, message);
if (maybeExecArgsOrErrors) {
if (areGraphQLErrors(maybeExecArgsOrErrors))
return await emit.error(maybeExecArgsOrErrors);
return id in ctx.subscriptions
? await emit.error(maybeExecArgsOrErrors)
: void 0;
else if (Array.isArray(maybeExecArgsOrErrors))
throw new Error(
'Invalid return value from onSubscribe hook, expected an array of GraphQLError objects',
Expand Down Expand Up @@ -760,17 +762,21 @@ export function makeServer<
execArgs.document,
);
if (validationErrors.length > 0)
return await emit.error(validationErrors);
return id in ctx.subscriptions
? await emit.error(validationErrors)
: void 0;
}

const operationAST = getOperationAST(
execArgs.document,
execArgs.operationName,
);
if (!operationAST)
return await emit.error([
new GraphQLError('Unable to identify operation'),
]);
return id in ctx.subscriptions
? await emit.error([
new GraphQLError('Unable to identify operation'),
])
: void 0;

// if `onSubscribe` didn't specify a rootValue, inject one
if (!('rootValue' in execArgs))
Expand Down

0 comments on commit db47a66

Please sign in to comment.