Skip to content

Commit

Permalink
fix(WebSocketSubject): close websocket connection attempt on unsubscribe
Browse files Browse the repository at this point in the history
Fix closing the underlying WebSocket when a WebSocketSubject is
unsubscribed while the WebSocket is in the `CONNECTING` state.
Initial support for closing a WebSocket before it was opened was added in #4446.
However, that pull-request still waits for the connection to eventually
become open before attempting to close it.
This pull-request takes it a step further by closing the websocket as soon as the
subject is unsubscribed.
  • Loading branch information
vially committed Jun 17, 2019
1 parent b24d3ea commit e1a671c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
26 changes: 26 additions & 0 deletions spec/observables/dom/webSocket-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,32 @@ describe('webSocket', () => {
(<any>socket.close).restore();
});

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

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 while connecting', () => {
const subject = webSocket<string>('ws://mysocket');
const subscription = subject.subscribe();
const socket = MockWebSocket.lastSocket;
sinon.spy(socket, 'close');
subscription.unsubscribe();

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
4 changes: 2 additions & 2 deletions src/internal/observable/dom/WebSocketSubject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
subscriber.add(() => {
const { _socket } = this;
if (this._output.observers.length === 0) {
if (_socket && _socket.readyState === 1) {
if (_socket && (_socket.readyState === 1 || _socket.readyState === 0)) {
_socket.close();
}
this._resetState();
Expand All @@ -378,7 +378,7 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {

unsubscribe() {
const { _socket } = this;
if (_socket && _socket.readyState === 1) {
if (_socket && (_socket.readyState === 1 || _socket.readyState === 0)) {
_socket.close();
}
this._resetState();
Expand Down

0 comments on commit e1a671c

Please sign in to comment.