Skip to content

Commit

Permalink
fix(WebSocketSubject): fix subject failing to close socket (#4446)
Browse files Browse the repository at this point in the history
Fix WebSocketSubject failing to close the underlying socket connection when it is unsubscribed
before the socket is in the open state. Currently, when unsubscribe is called on the subject or
when a subscription is unsubscribed there are checks on whether the socket is open (readyState = 1)
before it closes the connection. If unsubscribe is called before the connection is open, it will not
close the socket but will reset the state of the subject (_socket set to null), and it becomes
impossible to close the socket.
  • Loading branch information
rsikdar authored and benlesh committed Jan 29, 2019
1 parent e691379 commit dcfa52b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
28 changes: 28 additions & 0 deletions spec/observables/dom/webSocket-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,34 @@ describe('webSocket', () => {
(<any>socket.close).restore();
});

it('should close the socket when unsubscribed before socket open', () => {
const subject = webSocket<string>('ws://mysocket');
subject.subscribe();
subject.unsubscribe();
const socket = MockWebSocket.lastSocket;
sinon.spy(socket, 'close');
socket.open();

expect(socket.close).have.been.called;
expect(socket.readyState).to.equal(3); // closed

(<any>socket.close).restore();
});

it('should close the socket when subscription is cancelled before socket open', () => {
const subject = webSocket<string>('ws://mysocket');
const subscription = subject.subscribe();
subscription.unsubscribe();
const socket = MockWebSocket.lastSocket;
sinon.spy(socket, 'close');
socket.open();

expect(socket.close).have.been.called;
expect(socket.readyState).to.equal(3); // closed

(<any>socket.close).restore();
});

it('should close the socket with a code and a reason when errored', () => {
const subject = webSocket<string>('ws://mysocket');
subject.subscribe();
Expand Down
13 changes: 8 additions & 5 deletions src/internal/observable/dom/WebSocketSubject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
});

socket.onopen = (e: Event) => {
const { _socket } = this;
if (!_socket) {
socket.close();
this._resetState();
return;
}
const { openObserver } = this._config;
if (openObserver) {
openObserver.next(e);
Expand Down Expand Up @@ -280,14 +286,11 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
}

unsubscribe() {
const { source, _socket } = this;
const { _socket } = this;
if (_socket && _socket.readyState === 1) {
_socket.close();
this._resetState();
}
this._resetState();
super.unsubscribe();
if (!source) {
this.destination = new ReplaySubject();
}
}
}

0 comments on commit dcfa52b

Please sign in to comment.