various bug fixes

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ### vNEXT
 
+### 0.9.3
+- fix unhandledRejection error in GQL_STOP handling if initPromise rejected [PR #309](https://github.com/apollographql/subscriptions-transport-ws/pull/309)
+- fix return of init error message to legacy clients [PR #309](https://github.com/apollographql/subscriptions-transport-ws/pull/309)
+
 ### 0.9.2
 - fix format of keep alive message sent to legacy clients. [PR #297](https://github.com/apollographql/subscriptions-transport-ws/pull/297)
 - fix(isPromise): Made checks for promises in server.ts loose to allow for augmented and polyfilled promises. [PR #304](https://github.com/apollographql/subscriptions-transport-ws/pull/304)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "subscriptions-transport-ws",
-  "version": "0.9.2",
+  "version": "0.9.3",
   "description": "A websocket transport for GraphQL subscriptions",
   "main": "dist/index.js",
   "browser": "dist/client.js",

src/legacy/parse-legacy-protocol.ts

@@ -32,7 +32,7 @@ export const parseLegacyProtocolMessage = (connectionContext: ConnectionContext,
       if (connectionContext.isLegacy) {
         messageToReturn = {
           ...message, type: MessageTypes.INIT_FAIL,
-          payload: message.payload.message ? message.payload.message : message.payload,
+          payload: message.payload.message ? { error: message.payload.message } : message.payload,
         };
       }
       break;

src/server.ts

@@ -142,6 +142,7 @@ export class SubscriptionServer {
       }
 
       const connectionContext: ConnectionContext = Object.create(null);
+      connectionContext.initPromise = Promise.resolve(true);
       connectionContext.isLegacy = false;
       connectionContext.socket = socket;
       connectionContext.operations = {};
@@ -225,12 +226,6 @@ export class SubscriptionServer {
   }
 
   private onMessage(connectionContext: ConnectionContext) {
-    let onInitResolve: any = null;
-
-    connectionContext.initPromise = new Promise((resolve, reject) => {
-      onInitResolve = resolve;
-    });
-
     return (message: any) => {
       let parsedMessage: OperationMessage;
       try {
@@ -243,9 +238,8 @@ export class SubscriptionServer {
       const opId = parsedMessage.id;
       switch (parsedMessage.type) {
         case MessageTypes.GQL_CONNECTION_INIT:
-          let onConnectPromise = Promise.resolve(true);
           if (this.onConnect) {
-            onConnectPromise = new Promise((resolve, reject) => {
+            connectionContext.initPromise = new Promise((resolve, reject) => {
               try {
                 // TODO - this should become a function call with just 2 arguments in the future
                 // when we release the breaking change api: parsedMessage.payload and connectionContext
@@ -256,8 +250,6 @@ export class SubscriptionServer {
             });
           }
 
-          onInitResolve(onConnectPromise);
-
           connectionContext.initPromise.then((result) => {
             if (result === false) {
               throw new Error('Prohibited connection!');
@@ -441,10 +433,8 @@ export class SubscriptionServer {
           break;
 
         case MessageTypes.GQL_STOP:
-          connectionContext.initPromise.then(() => {
-            // Find subscription id. Call unsubscribe.
-            this.unsubscribe(connectionContext, opId);
-          });
+          // Find subscription id. Call unsubscribe.
+          this.unsubscribe(connectionContext, opId);
           break;
 
         default:

src/test/tests.ts

@@ -198,8 +198,9 @@ const onConnectErrorOptions = {
   schema,
   subscribe,
   execute,
+  isLegacy: true,
   onConnect: (msg: any, connection: any, connectionContext: any) => {
-    connectionContext.isLegacy = true;
+    connectionContext.isLegacy = onConnectErrorOptions.isLegacy;
     throw new Error('Error');
   },
 };
@@ -1480,25 +1481,45 @@ describe('Server', function () {
   it('should trigger onConnect and return GQL_CONNECTION_ERROR with error', (done) => {
     const connectionCallbackSpy = sinon.spy();
 
+    onConnectErrorOptions.isLegacy = false;
+    const subscriptionsClient = new SubscriptionClient(`ws://localhost:${ONCONNECT_ERROR_TEST_PORT}/`, {
+      connectionCallback: connectionCallbackSpy,
+    });
+
+    setTimeout(() => {
+      expect(connectionCallbackSpy.calledOnce).to.be.true;
+      expect(connectionCallbackSpy.getCall(0).args[0]).to.eql({ message: 'Error' });
+      subscriptionsClient.close();
+      done();
+    }, 200);
+  });
+
+  it('should trigger onConnect and return INIT_FAIL with error', (done) => {
+    const connectionCallbackSpy = sinon.spy();
+
+    onConnectErrorOptions.isLegacy = true;
     const subscriptionsClient = new SubscriptionClient(`ws://localhost:${ONCONNECT_ERROR_TEST_PORT}/`, {
       connectionCallback: connectionCallbackSpy,
     });
 
     const originalOnMessage = subscriptionsClient.client.onmessage;
     subscriptionsClient.client.onmessage = (dataReceived: any) => {
       let messageData = JSON.parse(dataReceived.data);
-
+      // Reformat message to avoid unknown message type
       if (messageData.type === MessageTypes.INIT_FAIL) {
         messageData.type = MessageTypes.GQL_CONNECTION_ERROR;
       }
-
       dataReceived.data = JSON.stringify(messageData);
       originalOnMessage(dataReceived);
     };
 
     setTimeout(() => {
       expect(connectionCallbackSpy.calledOnce).to.be.true;
-      expect(connectionCallbackSpy.getCall(0).args[0]).to.equal('Error');
+      // Old client used: connectionCallback(parsedMessage.payload.error)
+      // But new client uses: connectionCallback(parsedMessage.payload)
+      // So check complete payload
+      expect(connectionCallbackSpy.getCall(0).args[0]).to.eql({ error: 'Error' });
+      subscriptionsClient.close();
       done();
     }, 200);
   });