Skip to content

Commit 8cb201c

Browse files
authoredDec 6, 2021
fix(WebSocketSubject): handle slow WebSocket close (#6708)
The close event on WebSocket does not always occur immediately after running the close function. If the close event is occurs after a new WebSocket is opened, then the reset needs to be skipped. This checks to make sure the socket being reset is the one that matches the event. Closes #4650, #3935 Co-authored-by: Mark Knapp <mknapp@leightronix.com>
1 parent 0a64078 commit 8cb201c

File tree

2 files changed

+54
-14
lines changed

2 files changed

+54
-14
lines changed
 

‎spec/observables/dom/webSocket-spec.ts

+51-13
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ const root: any = (typeof globalThis !== 'undefined' && globalThis)
77
|| (typeof self !== 'undefined' && self)
88
|| global;
99

10+
enum WebSocketState {
11+
CONNECTING = 0,
12+
OPEN = 1,
13+
CLOSING = 2,
14+
CLOSED = 3
15+
}
16+
1017
/** @test {webSocket} */
1118
describe('webSocket', () => {
1219
let __ws: any;
@@ -131,15 +138,18 @@ describe('webSocket', () => {
131138
const socket = MockWebSocket.lastSocket;
132139
socket.open();
133140

134-
expect(socket.readyState).to.equal(1); // open
141+
expect(socket.readyState).to.equal(WebSocketState.OPEN);
135142

136143
sinon.spy(socket, 'close');
137144

138145
expect(socket.close).not.have.been.called;
139146

140147
subject.complete();
141148
expect(socket.close).have.been.called;
142-
expect(socket.readyState).to.equal(3); // closed
149+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
150+
151+
socket.triggerClose({ wasClean: true });
152+
expect(socket.readyState).to.equal(WebSocketState.CLOSED);
143153

144154
subject.unsubscribe();
145155
(<any>socket.close).restore();
@@ -154,7 +164,7 @@ describe('webSocket', () => {
154164
socket.open();
155165

156166
expect(socket.close).have.been.called;
157-
expect(socket.readyState).to.equal(3); // closed
167+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
158168

159169
(<any>socket.close).restore();
160170
});
@@ -168,7 +178,7 @@ describe('webSocket', () => {
168178
socket.open();
169179

170180
expect(socket.close).have.been.called;
171-
expect(socket.readyState).to.equal(3); // closed
181+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
172182

173183
(<any>socket.close).restore();
174184
});
@@ -181,7 +191,7 @@ describe('webSocket', () => {
181191
subject.unsubscribe();
182192

183193
expect(socket.close).have.been.called;
184-
expect(socket.readyState).to.equal(3); // closed
194+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
185195

186196
(<any>socket.close).restore();
187197
});
@@ -194,7 +204,36 @@ describe('webSocket', () => {
194204
subscription.unsubscribe();
195205

196206
expect(socket.close).have.been.called;
197-
expect(socket.readyState).to.equal(3); // closed
207+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
208+
209+
(<any>socket.close).restore();
210+
});
211+
212+
it('should close a socket that opens before the previous socket has closed', () => {
213+
const subject = webSocket<string>('ws://mysocket');
214+
const subscription = subject.subscribe();
215+
const socket = MockWebSocket.lastSocket;
216+
sinon.spy(socket, 'close');
217+
subscription.unsubscribe();
218+
219+
expect(socket.close).have.been.called;
220+
expect(socket.readyState).to.equal(WebSocketState.CLOSING);
221+
222+
const subscription2 = subject.subscribe();
223+
const socket2 = MockWebSocket.lastSocket;
224+
sinon.spy(socket2, 'close');
225+
226+
// Close socket after socket2 has opened
227+
socket2.open();
228+
expect(socket2.readyState).to.equal(WebSocketState.OPEN);
229+
socket.triggerClose({wasClean: true});
230+
231+
expect(socket.readyState).to.equal(WebSocketState.CLOSED);
232+
expect(socket2.close).have.not.been.called;
233+
234+
subscription2.unsubscribe();
235+
expect(socket2.close).have.been.called;
236+
expect(socket2.readyState).to.equal(WebSocketState.CLOSING);
198237

199238
(<any>socket.close).restore();
200239
});
@@ -747,7 +786,7 @@ class MockWebSocket {
747786

748787
sent: string[] = [];
749788
handlers: any = {};
750-
readyState: number = 0;
789+
readyState: WebSocketState = WebSocketState.CONNECTING;
751790
closeCode: any;
752791
closeReason: any;
753792
binaryType?: string;
@@ -766,8 +805,8 @@ class MockWebSocket {
766805
return length > 0 ? sent[length - 1] : undefined!;
767806
}
768807

769-
triggerClose(e: any): void {
770-
this.readyState = 3;
808+
triggerClose(e: Partial<CloseEvent>): void {
809+
this.readyState = WebSocketState.CLOSED;
771810
this.trigger('close', e);
772811
}
773812

@@ -783,16 +822,15 @@ class MockWebSocket {
783822
}
784823

785824
open(): void {
786-
this.readyState = 1;
825+
this.readyState = WebSocketState.OPEN;
787826
this.trigger('open', {});
788827
}
789828

790829
close(code: any, reason: any): void {
791-
if (this.readyState < 2) {
792-
this.readyState = 2;
830+
if (this.readyState < WebSocketState.CLOSING) {
831+
this.readyState = WebSocketState.CLOSING;
793832
this.closeCode = code;
794833
this.closeReason = reason;
795-
this.triggerClose({ wasClean: true });
796834
}
797835
}
798836

‎src/internal/observable/dom/WebSocketSubject.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,9 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
336336
};
337337

338338
socket.onclose = (e: CloseEvent) => {
339-
this._resetState();
339+
if (socket === this._socket) {
340+
this._resetState();
341+
}
340342
const { closeObserver } = this._config;
341343
if (closeObserver) {
342344
closeObserver.next(e);

0 commit comments

Comments
 (0)
Please sign in to comment.