Subscriptions-transport-ws: Cannot set property 'onopen' of null

[alexandre-kairn]

Hi,

We started receiving Cannot set property 'onopen' of null errors in sentry after upgrading subscriptions-transport-ws to v0.9.18

A quick search sent me to this PR with 2 people discussing the issue: https://github.com/apollographql/subscriptions-transport-ws/pull/615#discussion_r484585341

This error occurs only a few times a day so it is hard to create a reproduction repo, I have not looked deeply into it for now so I don't know if the suggested fix by @dko-slapdash would work.

I am not very familiar with the subscriptions-transport-ws, could someone look into it ?

Any help would we greatly appreciated :)

deps:

• subscriptions-transport-ws v0.9.18
• @apollo/client v3.2.0

[atdrago]

I'm seeing this as well. Dev tools pointed me here:

It looks like this.client.onopen is being referenced after this.client.close() is called. I'm new to the code, but it looks like close sets this.client to null, hence the issue.

Downgrading to 0.9.16 seems to have fixed the issue for me

[D34THWINGS]

Same issue here, this started to cause issues in our application E2E tests. When changing page it kills the connection and Cypress catches all uncaught errors thrown on the window it makes our test fail. Downgrading also worked for us. I guess also having client: any makes the type-checking allowing those operations where they shouldn't be. Maybe defining client as a generic type that extends the browser basic implementation would be safer in the code. And also having a cleanup function with all this.client.onXXX = null in it after checking if client is still there.

One thing that is also super weird in the code is that this.client.onclose calls this.close which then calls this.client.close() which in turn this.client.onclose looping up which I wonder how it successfully escapes from this 🤔

But also might be interesting to look inside WsLink if any cleanup function doesn't do redundant WS closing causing this.

[yosiawan]

Same issue here, downgrading to 0.9.16 seemed to fixed it as @atdrago said, so 0.9.18 is probably broken.

[alexandre-kairn]

Any idea on when #829 could be merged?

[ibash]

@alexandre-kairn recommend using https://github.com/ds300/patch-package with a patch like:

diff --git a/node_modules/subscriptions-transport-ws/dist/client.js b/node_modules/subscriptions-transport-ws/dist/client.js
index 3202dcd..9f4b1ed 100644
--- a/node_modules/subscriptions-transport-ws/dist/client.js
+++ b/node_modules/subscriptions-transport-ws/dist/client.js
@@ -122,12 +122,13 @@ var SubscriptionClient = (function () {
                 this.unsubscribeAll();
                 this.sendMessage(undefined, message_types_1.default.GQL_CONNECTION_TERMINATE, null);
             }
-            this.client.close();
-            this.client.onopen = null;
-            this.client.onclose = null;
-            this.client.onerror = null;
-            this.client.onmessage = null;
+            const client = this.client;
             this.client = null;
+            client.close();
+            client.onopen = null;
+            client.onclose = null;
+            client.onerror = null;
+            client.onmessage = null;
             this.eventEmitter.emit('disconnected');
             if (!isForced) {
                 this.tryReconnect();

[alexandre-kairn]

@ibash Thanks, indeed we went with something along those lines 👍

I was hoping to remove the patch when a new version is released, but at least this fixes the issue for now.

[vieira]

We are also experiencing this issue.