Reconnect on 1000

[felipeochoa]

I realize that 1000 signals a clean exit, but I'm behind a proxy that (incorrectly, I think) closes connections with a 1000 after an idle timeout. When receiving exit code 1000, robust-websocket won't call shouldReconnect, so responding to these events requires separate reconnecting logic. (E.g., adding an event listener to call .open again).

It would be nice to be able to handle these from shouldReconnect as well. Perhaps to keep back-compat there could be a flag to disable the default 1000 handler?

[nathanboktae]

That's why you can provide your own logic to shouldReconnect. Per standard 1000 is basically a HTTP 200 - "everything went as expected" and the connection is closing as desired.

A mess of flags is a bad OOP design that was intentionally avoided, favoring responsibility delegating functional programming composition instead.

[felipeochoa]

Right, except shouldReconnect isn't called if the response is 1000 (line 106, which I linked to).

I don't much care for the flag. I was just mentioning the idea in case you agreed that shouldReconnect should respond to 1000 but wanted keep back-compat (where shouldReconnect does not handle 1000).

[nathanboktae]

No I don't agree at all. I highly recommend to all users of the library to provide their own logic for shouldReconnect

[felipeochoa]

I don't think I'm explaining myself properly. I agree. shouldReconnect should handle all reconnect logic. Currently, it does not (hence this bug). If you look at line 106, you can see that as robust-websocket is currently written, users are not able to handle 1000 from shouldReconnect. The library itself handles these, never calling the user's function

function reconnect(event) {
    if (event.code === 1000 || explicitlyClosed) {  // <----- Line 106
        attempts = 0
        return
    }
    if (navigator.onLine === false) {
        reconnectWhenOnlineAgain = true
        return
    }

    var delay = opts.shouldReconnect(event, self)
    if (typeof delay === 'number') {
        pendingReconnect = setTimeout(newWebSocket, delay)
    }
}

[nathanboktae]

Ah ok sorry misunderstood. Hmm this is a decent breaking change to start sending disconnect events on 1000... Yeah a flag would prevent that but I'd rather not do that and do a breaking change...

[felipeochoa]

What about allowing shouldReconnect to opt into handling 1000 events? Something like the following:

function reconnect(event) {
    if ((!opts.shouldReconnect.handle1000 && event.code === 1000) || explicitlyClosed) {
        attempts = 0
        return
    }
    if (navigator.onLine === false) {
        reconnectWhenOnlineAgain = true
        return
    }

    var delay = opts.shouldReconnect(event, self)
    if (typeof delay === 'number') {
        pendingReconnect = setTimeout(newWebSocket, delay)
    }
}

This way it's not a top-level flag and there's no breaking change

[nathanboktae]

Yeah I like that proposal. Send a PR please :)

[Tobyheiam]

Please tell me how can I set shouldReconnect.handle1000 to true.