pusher / pusher-js

Pusher Javascript library
http://pusher.com
MIT License
2.11k stars 374 forks source link

Bug: Websocket is already in CLOSING or CLOSED state. #689

Closed Prince-Mendiratta closed 1 year ago

Prince-Mendiratta commented 1 year ago

Do you want to request a feature or report a bug? This issue is regarding a bug report.

What is the current behavior? If a channel is unsubscribed and the pusher is immediately disconnected, the unsubscribe request fails with the error as mentioned in the issue title.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar. The occurrence of this issue was found in this issue in Expensify/App. More details can be found in this comment. The use of Util.defer in send function delays the execution enough such that the Pusher gets disconnected before the event is completed. https://github.com/pusher/pusher-js/blob/807b778463851f3a9b02114abf06f00f65e94d8f/src/core/transports/transport_connection.ts#L129-L136

What is the expected behavior? If the Pusher has been disconnected, any pending send_event requests should be aborted instead of trying to complete and throwing an error.

Which versions of Pusher, and which browsers / OS are affected by this issue? Did this work in previous versions of Pusher? If so, which? The use of this function has been since the first commit, so most likely in all versions. Just very unlikely to occur.

Proposed Fix I'm unsure of the typescript implementation for this but checking the readyState of the channel before sending the message will ensure that this error does not occur. I'd appreciate some tips on how this can be implemented in typescript.

diff --git a/dist/web/pusher-with-encryption.js b/dist/web/pusher-with-encryption.js
index 32eb8fb3..3f31747f 100644
--- a/dist/web/pusher-with-encryption.js
+++ b/dist/web/pusher-with-encryption.js
@@ -4075,7 +4075,7 @@ var transport_connection_TransportConnection = (function (_super) {
         var _this = this;
         if (this.state === 'open') {
             util.defer(function () {
-                if (_this.socket) {
+                if (_this.socket && _this.socket.readyState == 1) {
                     _this.socket.send(data);
                 }
             });
benjamin-tang-pusher commented 1 year ago

@Prince-Mendiratta Reading the Expensify thread, are you able to reproduce this in a simpler test app, and not in Expensify's? If you can provide an example, I will try to reproduce it and see if your change fixes the issue.