centrifugal/centrifuge-js

WebsocketTransport: Cannot read properties of null (reading 'close')

HJK181 opened this issue · 4 comments

HJK181 commented

Describe the bug
If no connection could be established, for example, due to some network errors and the client does call this.centrifuge.disconnect() an Uncaught TypeError is thrown in WebsocketTransport#close:

Uncaught TypeError: Cannot read properties of null (reading 'close')
    at WebsocketTransport.close
    at Centrifuge._disconnect 
    at Centrifuge.disconnect

To Reproduce
Steps to reproduce the behavior:

  1. Create a new connection where you have mixed content, e.g. client uses HTTPS and you request ws://
  2. Call disconnect()on the Centrifugo instance

Expected behavior
The disconnect method should not throw an error in case the connection is not established. I know that the client could check the connection state before calling disconnect but I think this should be handled by the lib.

Versions

  • Centrifuge client version [^5.0.0-beta.0]
  • Server version [e.g. 5.0.3]
FZambia commented

Hey!

In general centrifuge-js should handle disconnect upon non-available transport out of the box. Though I suppose this concrete case is exceptional, most probably it's because of an exception during transport object creation - which may happen only due to app misconfiguration I believe. In this case it seems reasonable to not handle missing transport and behave like you mentioned. At least the situation is not normal - it's a developer mistake, not a general network issue.

Could you please make an experiment - instead of mixed content problem pass the wrong WebSocket connection address (I mean valid wss:// websocket URL, but to non-websocket server, like wss://example.com/connection/websocket) - to emulate unavailable server endpoint. In this case I suppose you should not have Uncaught TypeError: Cannot read properties of null (reading 'close').

HJK181 commented

You are right, when using wss://example.com/connection/websocket the lib logs WebSocket connection to 'wss://example.com/connection/websocket' failed: . While it tries to re-connect in a loop, I can safely call disconnectw/o getting any error.

So it looks like that with the mixed error that is caused by the Browser something behaves differently.

FZambia commented

Yes, it prevents creating transport by throwing exception when new WebSocket(...) is called. In general, I suppose we could handle such things somehow - but not obvious why we really want it - because it's a signal of developer mistake and crashing loud seems a good thing.

HJK181 commented

Fine with it. Feel free to close the issue.