Fix multiple sockets on reconnection

[talhazubairbutt]

Currently websocket listeners are not cleaned up on disconnection and createWebSocketConnection is called from multiple sources which results in multiple sockets to be created every time a user comes back online from sleep or there is a disruption in connectivity.

Steps to reproduce:

1. Open playground (connect to a remote websocket server as it wont occur if you are connecting to 127.0.0.1)
2. Switch wifi from network a to b
3. Initial socket will timeout depending on your messageReconnectTimeout value (hearbeat failure).
4. New socket is created
5. Browser calls onClose after about a minute or so, (network tab shows socket duration instead of pending state once close is received). This event triggers a new connection setup.
6. Two connections are now open both have listeners attached and are sending/received data. However, hocuspocus only has reference to one. Keep repeating the process and the number of sockets will keep on increasing and connected sockets go into retry loop and a new socket is created for each close event sent by the browser.

The online event listener also calls createWebSocketConnection

After this change only 1 socket is active at any given time. This PR is just a reference to fix the underlying issue. It can be fixed in multiple ways. I think the event emitter should be bound with the lifecycle of the websocket. Currently even stale websockets emit events that are handled with the latest context.

Ran lint:fix so there are some extra lint changes.
attachWebSocketListeners stores the reference of attached listeners.
cleanupWebSocket removes the listeners before resetting reference of the websocket.
online event listener has been removed as it leads to a duplicate socket. Once connection has been established, sockets are retried anyway infinite times. If initial connection fails, the retry wrapper takes over and attempts only the number of times that have been set.
Added an identifier to the socket instance.

Before:

Multiple.Connections.720.mov

After:

[TalhaASiddiqi]

This fails to run in Next JS SSR.
We probably need to add check that switches the crypto import based on environment (browser vs node).

[EugeneZ]

Thanks for the PR, this is an interesting bug! Why do you need crypto? Why not just use an incrementing number?

[talhazubairbutt]

Thanks for the PR, this is an interesting bug! Why do you need crypto? Why not just use an incrementing number?

Yup, fixed

[janthurau]

Thanks a lot for the fix & detailed reproduction steps, and even more thanks for the video :)

Merging this 💪

[xymfs]

I customized a WebSocketPolyfill for encryption and decryption of websocket data：

class WS extends WebSocket{
    constructor(url) {
        super(url)
    }
    set onmessage(f) {
        super.onmessage = (e) => {
            f({
                data: decrypt(e.data)
            })
        }
    }
    send(message) {
        super.send(encrypt(message))
    }
}

new HocuspocusProvider({
    websocketProvider: new HocuspocusProviderWebsocket({
        WebSocketPolyfill: WS,
    }),
})

But I noticed that this change replaced onmessage with addEventListener. How will I hook the returned information?

[TalhaASiddiqi]

I customized a WebSocketPolyfill for encryption and decryption of websocket data：

class WS extends WebSocket{
    constructor(url) {
        super(url)
    }
    set onmessage(f) {
        super.onmessage = (e) => {
            f({
                data: decrypt(e.data)
            })
        }
    }
    send(message) {
        super.send(encrypt(message))
    }
}

new HocuspocusProvider({
    websocketProvider: new HocuspocusProviderWebsocket({
        WebSocketPolyfill: WS,
    }),
})

But I noticed that this change replaced onmessage with addEventListener. How will I hook the returned information?

@xymfs you can override addEventListener over here and capture the listener for message handler inside it

[xymfs]

I customized a WebSocketPolyfill for encryption and decryption of websocket data：

class WS extends WebSocket{
    constructor(url) {
        super(url)
    }
    set onmessage(f) {
        super.onmessage = (e) => {
            f({
                data: decrypt(e.data)
            })
        }
    }
    send(message) {
        super.send(encrypt(message))
    }
}

new HocuspocusProvider({
    websocketProvider: new HocuspocusProviderWebsocket({
        WebSocketPolyfill: WS,
    }),
})

But I noticed that this change replaced onmessage with addEventListener. How will I hook the returned information?

@xymfs you can override addEventListener over here and capture the listener for message handler inside it

Thanks for the tip, it works fine after modification

class WS extends WebSocket{
    constructor(url) {
        super(url)
        const originalAddEventListener = this.addEventListener
        this.addEventListener = (type, listener, options) => {
            if (type === 'message') {
                const wrappedListener = (event) => {
                    listener({ ...event, data: decrypt(event.data)})
                }
                originalAddEventListener.call(this, type, wrappedListener, options)
            }else {
                originalAddEventListener.call(this, type, listener, options)
            }
        }
    }
    send(message) {
        super.send(encrypt(message))
    }
}