Skip to content

Commit 65aa100

Browse files
committed
Clear WebSocket connection timeout when closing channel
`WebSocketChannel` is built on top of a `WebSocket` and contains property that references it. `WebSocket` connection timeout is enforced by a separate setTimeout timer that closes the socket after configured amount of time. Previously this timeout has only been cleared when connection is established. It has not been cleared when channel is closed, resulting in potential existence of stray timers. This commit makes `WebSocketChannel` clear the connection timeout timer when it's closed. So connect timeout timer is removed even if channel is closed before connection is established.
1 parent 9351009 commit 65aa100

File tree

2 files changed

+70
-5
lines changed

2 files changed

+70
-5
lines changed

src/v1/internal/ch-websocket.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ class WebSocketChannel {
6565
}
6666
};
6767
this._ws.onopen = function() {
68-
// Connected! Cancel connection timeout
69-
clearTimeout(self._connectionTimeoutId);
68+
// Connected! Cancel the connection timeout
69+
self._clearConnectionTimeout();
7070

7171
// Drain all pending messages
7272
let pending = self._pending;
@@ -85,7 +85,7 @@ class WebSocketChannel {
8585
this._ws.onerror = this._handleConnectionError;
8686

8787
this._connectionTimeoutFired = false;
88-
this._connectionTimeoutId = this._setupConnectionTimeout(config);
88+
this._connectionTimeoutId = this._setupConnectionTimeout();
8989
}
9090

9191
_handleConnectionError() {
@@ -141,6 +141,7 @@ class WebSocketChannel {
141141
*/
142142
close ( cb = ( () => null )) {
143143
this._open = false;
144+
this._clearConnectionTimeout();
144145
this._ws.close();
145146
this._ws.onclose = cb;
146147
}
@@ -164,6 +165,19 @@ class WebSocketChannel {
164165
}
165166
return null;
166167
}
168+
169+
/**
170+
* Remove active connection timeout, if any.
171+
* @private
172+
*/
173+
_clearConnectionTimeout() {
174+
const timeoutId = this._connectionTimeoutId;
175+
if (timeoutId || timeoutId === 0) {
176+
this._connectionTimeoutFired = false;
177+
this._connectionTimeoutId = null;
178+
clearTimeout(timeoutId);
179+
}
180+
}
167181
}
168182

169183
let available = typeof WebSocket !== 'undefined';

test/internal/ch-websocket.test.js

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@ import wsChannel from '../../src/v1/internal/ch-websocket';
2121
import ChannelConfig from '../../src/v1/internal/ch-config';
2222
import urlUtil from '../../src/v1/internal/url-util';
2323
import {SERVICE_UNAVAILABLE} from '../../src/v1/error';
24+
import {setTimeoutMock} from './timers-util';
2425

2526
describe('WebSocketChannel', () => {
2627

2728
const WebSocketChannel = wsChannel.channel;
2829
const webSocketChannelAvailable = wsChannel.available;
2930

3031
let OriginalWebSocket;
32+
let webSocketChannel;
3133

3234
beforeEach(() => {
3335
if (webSocketChannelAvailable) {
@@ -39,6 +41,9 @@ describe('WebSocketChannel', () => {
3941
if (webSocketChannelAvailable) {
4042
WebSocket = OriginalWebSocket;
4143
}
44+
if (webSocketChannel) {
45+
webSocketChannel.close();
46+
}
4247
});
4348

4449
it('should fallback to literal IPv6 when SyntaxError is thrown', () => {
@@ -49,6 +54,47 @@ describe('WebSocketChannel', () => {
4954
testFallbackToLiteralIPv6('bolt://[fe80::1%lo0]:8888', 'ws://fe80--1slo0.ipv6-literal.net:8888');
5055
});
5156

57+
it('should clear connection timeout when closed', () => {
58+
if (!webSocketChannelAvailable) {
59+
return;
60+
}
61+
62+
const fakeSetTimeout = setTimeoutMock.install();
63+
try {
64+
// do not execute setTimeout callbacks
65+
fakeSetTimeout.pause();
66+
67+
let fakeWebSocketClosed = false;
68+
69+
// replace real WebSocket with a function that does nothing
70+
WebSocket = () => {
71+
return {
72+
close: () => {
73+
fakeWebSocketClosed = true;
74+
}
75+
};
76+
};
77+
78+
const url = urlUtil.parseBoltUrl('bolt://localhost:7687');
79+
const driverConfig = {connectionTimeout: 4242};
80+
const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE);
81+
82+
webSocketChannel = new WebSocketChannel(channelConfig);
83+
84+
expect(fakeWebSocketClosed).toBeFalsy();
85+
expect(fakeSetTimeout.invocationDelays).toEqual([]);
86+
expect(fakeSetTimeout.clearedTimeouts).toEqual([]);
87+
88+
webSocketChannel.close();
89+
90+
expect(fakeWebSocketClosed).toBeTruthy();
91+
expect(fakeSetTimeout.invocationDelays).toEqual([]);
92+
expect(fakeSetTimeout.clearedTimeouts).toEqual([0]); // cleared one timeout with id 0
93+
} finally {
94+
fakeSetTimeout.uninstall();
95+
}
96+
});
97+
5298
function testFallbackToLiteralIPv6(boltAddress, expectedWsAddress) {
5399
if (!webSocketChannelAvailable) {
54100
return;
@@ -59,14 +105,19 @@ describe('WebSocketChannel', () => {
59105
if (url.indexOf('[') !== -1) {
60106
throw new SyntaxError();
61107
}
62-
return {url: url};
108+
return {
109+
url: url,
110+
close: () => {
111+
}
112+
};
63113
};
64114

65115
const url = urlUtil.parseBoltUrl(boltAddress);
66116
// disable connection timeout, so that WebSocketChannel does not set any timeouts
67117
const driverConfig = {connectionTimeout: 0};
68118
const channelConfig = new ChannelConfig(url, driverConfig, SERVICE_UNAVAILABLE);
69-
const webSocketChannel = new WebSocketChannel(channelConfig);
119+
120+
webSocketChannel = new WebSocketChannel(channelConfig);
70121

71122
expect(webSocketChannel._ws.url).toEqual(expectedWsAddress);
72123
}

0 commit comments

Comments
 (0)