Send more info on disconnected event #10
Conversation
| @@ -108,7 +108,7 @@ class LongPollTransport extends Transport | |||
|
|
|||
| unless @_needToNotifyOfReconnect or @_stopRequestLoop | |||
| @_needToNotifyOfReconnect = true | |||
| @config.disconnected() | |||
| @config.disconnected(jqXhr, status, error) | |||
There was a problem hiding this comment.
What type is the event that's passed to this from the websocket versions? As I'd at least like the first parameter to be a similar type across them both, my guess that in this case error may match better as a first argument.
There was a problem hiding this comment.
Also, make sure we can determine if it was long poll or web socket in the handler. If the event doesn't include something we can use (somewhat clearly) then I'd suggest another parameter of just long-poll or similar
There was a problem hiding this comment.
Yeah, I was on the fence about this. I think it's a bit different across transports, since this is the result from a rejected jQuery promise whereas the WebSockets one is a CloseEvent specific to WebSockets.
There was a problem hiding this comment.
I'm fine switching to just the error object if you think that's better. Should we pass the transport type in all the events (connected, disconnected, failed, error)?
There was a problem hiding this comment.
Ideally, yes. Although as the other events already have some data passed in, it would be hard to define a standard way. Perhaps that change would be better waiting for a rewrite/refactor/v2 to standardize them.
We don't currently send any data to the
disconnectedhandler. In order to debug, I'd like to send more information along to the help identify disconnects. Specifically, I'm looking forCloseEvent.closeandCloseEvent.reasonfrom http://stackoverflow.com/questions/18803971/websocket-onerror-how-to-read-error-description