Subscriptions-transport-ws: UnhandledPromiseRejectionWarning if onConnect rejects or throws

[WesSouza]

If onConnect returns a promise that rejects, or if it throws internally, there is no catch on either MessageTypes.GQL_START or MessageTypes.GQL_STOP promise handling.

This is throwing a UnhandledPromiseRejectionWarning, which will terminate the Node.js process.

(node:63913) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 3): Missing jwt
(node:63913) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[raeesaa]

+1

[NeoPhi]

Both of these should now be properly handled based on:
https://github.com/apollographql/subscriptions-transport-ws/commit/bd86fcfd887f026eafd83fa58b5a067317c7de71
and
https://github.com/apollographql/subscriptions-transport-ws/commit/ebb98b26aa72327b61f5c4d9232e6c5874d1a3c0

[maxaggedon]

I'm still having this problem on 0.9.16

[memignone]

+1

[javierfernandes]

Just tested again with latest version 0.9.16 and it is still happening for me.

@NeoPhi could you considere reopening the issue ?

This is pretty annoying because it creates a lot of logs. In our app we expect WS communication to start already with an auth token, so we check that onConnect and reject it. So our logs gets full of this stacktrace when users have expired tokens.

I tried writting it in many ways like async/await, promises, regular JS throw outside of the promise, etc, and it fails for all of them.
The compiled code seems to have catchs but is difficult to follow.

Here an example with async await

const onConnect = async ({ authToken }) => {
      if (!authToken) {
        throw new Error('Missing auth token!')
      }
      return {
        user: await verifyTokenAndGetUser(authToken),
      }
    }

with a regular throw

    const onConnect = ({ authToken }) => {
      if (!authToken) {
        throw new Error('Missing auth token!')
      }
      return verifyTokenAndGetUser(authToken).then(user => ({
        user,
      }))
    }

What is funny is that the library correctly disconnects those users, so it catches the original error, but somehow it seems like you are wrapping it in some other promises that get rejected and not handled. So the onConnect is handled in surface, but something is wrong later.

Thanks !

[jbelej]

Still experiencing this issue!

[simone]

I'm still having this problem...

[OmgImAlexis]

@NeoPhi can this please be reopened.

[OmgImAlexis]

I'm going to try and get this fixed as I'm seeing the error quite often on my server while testing.

https://github.com/apollographql/subscriptions-transport-ws/pull/736

[asmeikal]

We're having the same problem, and for us it seems to appear only when onDisconnect returns a Promise that fails.

In our setup, the onConnect looks something like the one in @javierfernandes comment, and the onDisconnect does await connectionContext.initPromise before doing stuff with the user that's disconnecting.

There is no try/catch in our onDisconnect, so if the onConnect fails, onDisconnect fails too, and this line does not catch it.

The stack trace of the UnhandledPromiseRejectionWarning is misleading for us, as it points to our onConnect, when it's actually the onDisconnect rejection that is not handled.

Wrapping our onDisconnect in a try { ... } catch { // do nothing } removes the warning, but doesn't solve the root of the problem.

This test highlights the problem, if the --throw-deprecation flag is added to mocha:

it('should work with async onConnect', (done) => {
  const connectionCallbackSpy = sinon.spy();

  const httpServerForError = createServer(notFoundRequestListener);
  httpServerForError.listen(ERROR_TEST_PORT);

  new SubscriptionServer({
    schema,
    execute,
    onConnect: () => {
      throw new Error('failed');
    },
    onDisconnect: async (_ws: WebSocket, connectionContext: ConnectionContext) => {
      await connectionContext.initPromise;
    },
  }, { server: httpServerForError });

  onConnectErrorOptions.isLegacy = false;
  const subscriptionsClient = new SubscriptionClient(`ws://localhost:${ERROR_TEST_PORT}/`, {
    connectionCallback: connectionCallbackSpy,
  });

  setTimeout(() => {
    expect(connectionCallbackSpy.calledOnce).to.be.true;
    expect(connectionCallbackSpy.getCall(0).args[0]).to.eql({ message: 'failed' });
    subscriptionsClient.close();
    done();
  }, 200);
});

[OmgImAlexis]

Can this please be reopened?

I've been playing around with this and I think this fix works with all cases.

https://github.com/apollographql/subscriptions-transport-ws/pull/823

[lPaths]

the above fix is not working for me in typescript, anyone can give me a remedy for this please?