WebSocketSubject does not reconnect with beta10 (change behaviour from beta7)

[sebald]

RxJS version: 5.0.0-beta10

Code to reproduce:

    const RETRY_DELAY = 200;
    const openObserver:NextObserver<Event> = { next: () => console.log('connected') };
    const closeObserver:NextObserver<CloseEvent> = { next: () => console.log('disconnected') };

       const ws$ = new WebSocketSubject<APIMessage>({
            url,
            openObserver,
            closeObserver
        });

        const actions$ = ws$
            .retryWhen(errors => errors.mergeMap(error => {
                if (window.navigator.onLine) {
                    console.warn(`Retrying in ${RETRY_DELAY}ms.`);
                    return Observable.timer(RETRY_DELAY);
                } else {
                    return Observable.fromEvent(window, 'online').take(1);
                }
            }))
            .map(msg => msg /** usually maps APIMessage to ReduxMessages */ );

Expected behavior:
When the connected WS server is shut down, constantly push errors to the retryWhen until no errors are thrown anymore (=> server is back online again).

Actual behavior:
retryWhen is called and the timer observable is constantly repeated, but the WebSocketSubject seems not to recognise that the server is back online.

Additional information:
I wanted to update the dependencies of our app we wrote a few months back. We used beta7 before. Updating rxjs to beta8-10 will "break" the reconnecting feature we implemented with the above code .

It seems that with the rewrite of Subject in beta8 the behaviour completely changed. Bevor the console did show the following:

With beta10 the same code will behave like this:

The beta10 is (from my point a view) a lot nicer because it doesn't trigger the closeObserver multiple times and there is no error anymore :) But it also does not recognise when the WebSocket connection is availble again.

I tried to debug it myself but couldn't find a the issue.

EDIT: I added the fix applied in 848a527 but didn't solve my problem.

[benlesh]

Thanks for reporting this. We'll have a look

[deontologician]

I have run into this as well, and I think this is due to _output not being reset. Since subjects keep their errored state and can't be restarted, it's causing any subscribers to the websocket to immediately get the error from last time.

My guess would be that inside this branch when the socket is closed:

https://github.com/ReactiveX/rxjs/blob/master/src/observable/dom/WebSocketSubject.ts#L213

It should also do something like this._output = new Subject() so the next subscriber after the disconnect doesn't just keep getting the old error.

[deontologician]

And probably also here:

https://github.com/ReactiveX/rxjs/blob/master/src/observable/dom/WebSocketSubject.ts#L213

Basically, any time the socket is set to null, it should reset _output as well

[sebald]

I tried it today with beta11 and now an error pops up again! :)

WebSocket is already in CLOSING or CLOSED state.

Maybe this helps to find the problem? (Even though this is from the WebSocket object)
I have some time to debug this again. Hopefully I can come up with something helpful...

I hacked together a little project to debug the issue: https://github.com/sebald/WS-Reconnect

[deontologician]

So, I tried out your reconnect thing with the ._output fix I suggested earlier. Still doesn't work quite right. It seems to retry once and then give up. I am thinking of just hacking this by using Observable.defer and treating WebSocketSubject as not reuseable

[sebald]

Could you post your ._output fix here so I can try it too?

I noticed that when ._subscribe() is called and the socket is in CLOSED state, it still is set as this.socket. My guess is that there is (also) the removal of the closed socket missing/not called.

[deontologician]

I don't have a patch atm, but basically everywhere it has this.socket =null, you add this._output = new Subject() right after it

On Thu, Sep 1, 2016, 23:18 Sebastian Sebald notifications@github.com
wrote:

Could you post your ._output fix here so I can try it too?

I noticed that when ._subscribe() is called and the socket is in CLOSED
state, it still is set as this.socket. My guess is that there is (also)
the removal of the closed socket missing/not called.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#1863 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAFVixDPEzRJP-dkjgkEABV0Xsz4zNpks5ql7-bgaJpZM4JbWsu
.

[sebald]

I found a solution, but I am not convinced that it is the correct one, because I am not 100% familiar with the internal of RxJS 😢 Anyway ...

I added the following line to the _subscribe() method:

if (socket && socket.readyState === 3) {
  this.socket = null;
  this._output = new Subject<T>();
}

This will clear the current WebSocket and force the WebSocketSubject to call _connectSocket again, after the teardown of a Subscription happens and the socket was closed.

Here is the commit in my fork: sebald@85f4f87
As you can see, this will also make the test for WebSocketSubject.multiplex fail. Which is also the reason I believe that this is not really the right place to clear the WebSocket.

I updated the repo I mentioned before accordingly: https://github.com/sebald/WS-Reconnect

[benlesh]

So looking into this, @sebald was on the right track... but it's a little broader than that.

There are three things that need to happen everytime we want to "reset" and null out the socket:

1. We check the source and reset the destination to a ReplaySubject
2. We reset the _output Subject with a new Subject
3. We null the socket.

That seems to fix all reconnection issues, even with multiplexed sockets.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.