Promises getting rejected with non-Error objects

[gjbadros]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

Promises are getting rejected with non-Error objects, which cause bluebirdjs promise warnings/errors and is generally a bad practice. This is a minimal fix -- probably better to have a graphql-ws specific Error subclass, etc.

Here is the diff that solved my problem:

diff --git a/node_modules/graphql-ws/umd/graphql-ws.js b/node_modules/graphql-ws/umd/graphql-ws.js
index 3150aa0..38b2615 100644
--- a/node_modules/graphql-ws/umd/graphql-ws.js
+++ b/node_modules/graphql-ws/umd/graphql-ws.js
@@ -400,8 +400,9 @@
                     connecting = undefined;
                     clearTimeout(connectionAckTimeout);
                     clearTimeout(queuedPing);
-                    denied(errOrEvent);
-                    if (isLikeCloseEvent(errOrEvent) && errOrEvent.code === 4499) {
+                    const ilce = isLikeCloseEvent(errOrEvent)
+                    denied(new Error('graphql-ws: errorOrClosed', { ...errOrEvent, ilce }));
+                    if (ilce && errOrEvent.code === 4499) {
                         socket.close(4499, 'Terminated'); // close event is artificial and emitted manually, see `Client.terminate()` below
                         socket.onerror = null;
                         socket.onclose = null;
@@ -477,7 +478,8 @@
                         retries = 0; // reset the retries on connect
                         connected([
                             socket,
-                            new Promise((_, reject) => errorOrClosed(reject)),
+                            // new Promise((_, reject) => errorOrClosed(reject)),
+                            new Promise((_, reject) => errorOrClosed(() => reject(new Error('graphql-ws: Socket closed')))),
                         ]);
                     }
                     catch (err) {
@@ -738,11 +740,11 @@
             terminate() {
                 if (connecting) {
                     // only if there is a connection
-                    emitter.emit('closed', {
+                    emitter.emit('closed', new Error('graphql-ws: Terminated', {
                         code: 4499,
                         reason: 'Terminated',
                         wasClean: false,
-                    });
+                    }));
                 }
             },
         };

This issue body was partially generated by patch-package.

[enisdenjo]

Hey hey, this would make sense! I'd just prefer to have a concrete error class instead of relying on the message. Let's go with class TerminatedError extends Error.

Would you like to open a PR for this? Would be greatly appreciated. :D

[enisdenjo]

🎉 This issue has been resolved in version 5.15.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀