Throwing from onSubscribe doesn't close the connection

[devunt]

Expected Behaviour
By documentation throwing from onSubscribe function makes the underlying connection get closed.

Actual Behaviour
Throwing from onSubscribe does not close the underlying connection.

Debug Information
Minimal reproduction:

const server = makeServer({
  onSubscribe: async () => {
    throw new Error('hello');
  },
});

// websocket connection established ...

server.opened({
  // ...
  close: (code, reason) => {
    console.log('close', code, reason); // <- Does not get called
    ws.close(code, reason);
  },
  // ...
});

[enisdenjo]

This behaviour is intentional. It is up to the implementor to decide what throwing from any of the hooks, including onSubscribe, does.

When you throw there, the onMessage(callback) callback function will throw and you decide whether to close the connection or gracefully handle it (because it might be just one specific subscription failing, not needing to kick off the client complately).

In all of the built-in adapters, graphql-ws will close the connection after throwing, but it's a choice. For example, here's the use/ws adapter:

graphql-ws/src/use/ws.ts

Lines 144 to 161 in 85985ee

	try {
	await cb(String(event));
	} catch (err) {
	console.error(
	'Internal error occurred during message handling. ' +
	'Please check your implementation.',
	err,
	);
	socket.close(
	CloseCode.InternalServerError,
	isProd
	? 'Internal server error'
	: limitCloseReason(
	err instanceof Error ? err.message : String(err),
	'Internal server error',
	),
	);
	}

[devunt]

Ah, thanks for the clarification! I was using my custom implementation of adapter that uses Hono as a websocket server.
Wrapping onMessage callback call with try/catch and calling websocket's close() on error indeed does the trick.

Maybe documentation can be improved? Currently every hooks' documentation says it closes the connection when an error occurs:

/**
   ...
   * Throwing an error from within this function will
   * close the socket with the `Error` message
   * in the close event reason.
   ...
   */

But as this behavior is only applied to the built-in adapters, clarifying that in the comment would help preventing future confusions.

After all, I really appreciate for the protocol and the reference implementation! Protocol specification and actual implementation was so helpful while implementing the whole client and server adapter from the scratch, as we are using dart/flutter as client and hono/bun as the server and sadly neither has the official implementation. It was fun to implement the protocol!

[enisdenjo]

You're absolutely right that the docs can be improved about this. Would you like to open a PR with the documentation improvements? I'd really appreciate it. 🙏

Super happy that the protocol helped you get graphql-ws running on a different stack! Awesome job!