Skip to content

Commit 84d6f66

Browse files
bigmontzbigmontz
authored
Clear receive timeout when close browser and Deno channels (#1016)
The receive timeout was not being clearer when channel get closed. With the timeout running, receive timeouts events can still be notified to the existing observers. This was causing some tests failing and possible issues in production code. This bug was found in the test `package/neo4j-driver/test/result.test.js (should handle missing onCompleted)`. This test was calling `done` twice because of a late failing coming to the observer. Node channels are not affected by this error since its timeout implementation is controlled to the socket.
1 parent 1fe1471 commit 84d6f66

File tree

6 files changed

+63
-45
lines changed

6 files changed

+63
-45
lines changed

‎packages/bolt-connection/src/channel/browser/browser-channel.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ export default class WebSocketChannel {
168168
return new Promise((resolve, reject) => {
169169
if (this._ws && this._ws.readyState !== WS_CLOSED) {
170170
this._open = false
171+
this.stopReceiveTimeout()
171172
this._clearConnectionTimeout()
172173
this._ws.onclose = () => resolve()
173174
this._ws.close()
@@ -206,7 +207,7 @@ export default class WebSocketChannel {
206207
* Start the receive timeout for the channel.
207208
*/
208209
startReceiveTimeout () {
209-
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
210+
if (this._open && this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
210211
this._receiveTimeoutStarted = true
211212
this._resetTimeout()
212213
}

‎packages/bolt-connection/src/channel/deno/deno-channel.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ export default class DenoChannel {
150150
async close () {
151151
if (this._open) {
152152
this._open = false
153+
this.stopReceiveTimeout()
153154
if (this._conn != null) {
154155
await this._conn.close()
155156
}
@@ -185,7 +186,7 @@ export default class DenoChannel {
185186
* Start the receive timeout for the channel.
186187
*/
187188
startReceiveTimeout () {
188-
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
189+
if (this._open && this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
189190
this._receiveTimeoutStarted = true
190191
this._resetTimeout()
191192
}

‎packages/bolt-connection/test/channel/browser/browser-channel.test.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,31 @@ describe('WebSocketChannel', () => {
412412
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
413413
})
414414

415+
it('should be cleared when connection closes', async () => {
416+
webSocketChannel.startReceiveTimeout()
417+
418+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
419+
expect(fakeSetTimeout.calls.length).toEqual(1)
420+
expect(fakeSetTimeout.calls[0][1]).toEqual(receiveTimeout)
421+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
422+
423+
await webSocketChannel.close()
424+
425+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(1)
426+
expect(fakeSetTimeout.calls.length).toEqual(1)
427+
expect(fakeSetTimeout.clearedTimeouts).toEqual([0])
428+
})
429+
430+
it('should call not setTimeout(receiveTimeout) when connection is closed', async () => {
431+
await webSocketChannel.close()
432+
433+
webSocketChannel.startReceiveTimeout()
434+
435+
expect(fakeSetTimeout._timeoutIdCounter).toEqual(0)
436+
expect(fakeSetTimeout.calls.length).toEqual(0)
437+
expect(fakeSetTimeout.clearedTimeouts).toEqual([])
438+
})
439+
415440
it('should call setTimeout(receiveTimeout) after stopped', () => {
416441
webSocketChannel.startReceiveTimeout()
417442

‎packages/neo4j-driver-deno/lib/bolt-connection/channel/browser/browser-channel.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ export default class WebSocketChannel {
168168
return new Promise((resolve, reject) => {
169169
if (this._ws && this._ws.readyState !== WS_CLOSED) {
170170
this._open = false
171+
this.stopReceiveTimeout()
171172
this._clearConnectionTimeout()
172173
this._ws.onclose = () => resolve()
173174
this._ws.close()
@@ -206,7 +207,7 @@ export default class WebSocketChannel {
206207
* Start the receive timeout for the channel.
207208
*/
208209
startReceiveTimeout () {
209-
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
210+
if (this._open && this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
210211
this._receiveTimeoutStarted = true
211212
this._resetTimeout()
212213
}

‎packages/neo4j-driver-deno/lib/bolt-connection/channel/deno/deno-channel.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ export default class DenoChannel {
150150
async close () {
151151
if (this._open) {
152152
this._open = false
153+
this.stopReceiveTimeout()
153154
if (this._conn != null) {
154155
await this._conn.close()
155156
}
@@ -185,7 +186,7 @@ export default class DenoChannel {
185186
* Start the receive timeout for the channel.
186187
*/
187188
startReceiveTimeout () {
188-
if (this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
189+
if (this._open && this._receiveTimeout !== null && !this._receiveTimeoutStarted) {
189190
this._receiveTimeoutStarted = true
190191
this._resetTimeout()
191192
}

‎packages/neo4j-driver/test/temporal-types.test.js‎

Lines changed: 30 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import neo4j from '../src'
2121
import sharedNeo4j from './internal/shared-neo4j'
2222
import { toNumber, internal } from 'neo4j-driver-core'
23-
import timesSeries from 'async/timesSeries'
2423
import testUtils from './internal/test-utils'
2524

2625
const {
@@ -55,11 +54,11 @@ describe('#integration temporal-types', () => {
5554

5655
beforeAll(() => {
5756
driver = neo4j.driver(
58-
`bolt://${sharedNeo4j.hostname}`,
57+
`${sharedNeo4j.scheme}://${sharedNeo4j.hostname}`,
5958
sharedNeo4j.authToken
6059
)
6160
driverWithNativeNumbers = neo4j.driver(
62-
`bolt://${sharedNeo4j.hostname}`,
61+
`${sharedNeo4j.scheme}://${sharedNeo4j.hostname}`,
6362
sharedNeo4j.authToken,
6463
{ disableLosslessIntegers: true }
6564
)
@@ -101,13 +100,13 @@ describe('#integration temporal-types', () => {
101100
)
102101
}, 90000)
103102

104-
it('should send and receive random Duration', async () => {
103+
describe('Duration', () => {
105104
if (neo4jDoesNotSupportTemporalTypes()) {
106105
return
107106
}
108107

109-
await testSendAndReceiveRandomTemporalValues(() => randomDuration())
110-
}, 90000)
108+
testSendAndReceiveRandomTemporalValues('Duration', () => randomDuration())
109+
})
111110

112111
it('should send and receive Duration when disableLosslessIntegers=true', async () => {
113112
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -169,13 +168,13 @@ describe('#integration temporal-types', () => {
169168
)
170169
}, 90000)
171170

172-
it('should send and receive random LocalTime', async () => {
171+
describe('LocalTime', () => {
173172
if (neo4jDoesNotSupportTemporalTypes()) {
174173
return
175174
}
176175

177-
await testSendAndReceiveRandomTemporalValues(() => randomLocalTime())
178-
}, 90000)
176+
testSendAndReceiveRandomTemporalValues('LocalTime', () => randomLocalTime())
177+
})
179178

180179
it('should send and receive array of LocalTime', async () => {
181180
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -226,13 +225,13 @@ describe('#integration temporal-types', () => {
226225
)
227226
}, 90000)
228227

229-
it('should send and receive random Time', async () => {
228+
describe('Time', async () => {
230229
if (neo4jDoesNotSupportTemporalTypes()) {
231230
return
232231
}
233232

234-
await testSendAndReceiveRandomTemporalValues(() => randomTime())
235-
}, 90000)
233+
testSendAndReceiveRandomTemporalValues('Time', () => randomTime())
234+
})
236235

237236
it('should send and receive array of Time', async () => {
238237
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -281,13 +280,13 @@ describe('#integration temporal-types', () => {
281280
await testSendReceiveTemporalValue(new neo4j.types.Date(1923, 8, 14))
282281
}, 90000)
283282

284-
it('should send and receive random Date', async () => {
283+
describe('Date', () => {
285284
if (neo4jDoesNotSupportTemporalTypes()) {
286285
return
287286
}
288287

289-
await testSendAndReceiveRandomTemporalValues(() => randomDate())
290-
}, 90000)
288+
testSendAndReceiveRandomTemporalValues('Date', () => randomDate())
289+
})
291290

292291
it('should send and receive array of Date', async () => {
293292
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -346,13 +345,13 @@ describe('#integration temporal-types', () => {
346345
)
347346
}, 90000)
348347

349-
it('should send and receive random LocalDateTime', async () => {
348+
describe('LocalDateTime', async () => {
350349
if (neo4jDoesNotSupportTemporalTypes()) {
351350
return
352351
}
353352

354-
await testSendAndReceiveRandomTemporalValues(() => randomLocalDateTime())
355-
}, 90000)
353+
testSendAndReceiveRandomTemporalValues('LocalDateTime', () => randomLocalDateTime())
354+
})
356355

357356
it('should send and receive array of random LocalDateTime', async () => {
358357
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -442,15 +441,15 @@ describe('#integration temporal-types', () => {
442441
)
443442
}, 90000)
444443

445-
it('should send and receive random DateTime with zone offset', async () => {
444+
describe('DateTime with zone offset', () => {
446445
if (neo4jDoesNotSupportTemporalTypes()) {
447446
return
448447
}
449448

450-
await testSendAndReceiveRandomTemporalValues(() =>
449+
testSendAndReceiveRandomTemporalValues('DateTime with zone offset', () =>
451450
randomDateTimeWithZoneOffset()
452451
)
453-
}, 90000)
452+
})
454453

455454
it('should send and receive array of DateTime with zone offset', async () => {
456455
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -540,15 +539,15 @@ describe('#integration temporal-types', () => {
540539
)
541540
}, 90000)
542541

543-
it('should send and receive random DateTime with zone id', async () => {
542+
describe('DateTime with zone id', async () => {
544543
if (neo4jDoesNotSupportTemporalTypes()) {
545544
return
546545
}
547546

548-
await testSendAndReceiveRandomTemporalValues(() =>
547+
testSendAndReceiveRandomTemporalValues('DateTime with zone id', () =>
549548
randomDateTimeWithZoneId()
550549
)
551-
}, 90000)
550+
})
552551

553552
it('should send and receive array of DateTime with zone id', async () => {
554553
if (neo4jDoesNotSupportTemporalTypes()) {
@@ -1403,22 +1402,12 @@ describe('#integration temporal-types', () => {
14031402
)
14041403
})
14051404

1406-
function testSendAndReceiveRandomTemporalValues (valueGenerator) {
1407-
const asyncFunction = (index, callback) => {
1408-
testSendReceiveTemporalValue(valueGenerator())
1409-
.then(() => callback())
1410-
.catch(error => callback(error))
1411-
}
1412-
1413-
return new Promise((resolve, reject) => {
1414-
timesSeries(RANDOM_VALUES_TO_TEST, asyncFunction, (error, result) => {
1415-
if (error) {
1416-
reject(error)
1417-
} else {
1418-
resolve(result)
1419-
}
1405+
function testSendAndReceiveRandomTemporalValues (temporalType, valueGenerator) {
1406+
for (let i = 0; i < RANDOM_VALUES_TO_TEST; i++) {
1407+
it(`should send and receive random ${temporalType} [index=${i}]`, async () => {
1408+
await testSendReceiveTemporalValue(valueGenerator())
14201409
})
1421-
})
1410+
}
14221411
}
14231412

14241413
async function testSendAndReceiveArrayOfRandomTemporalValues (valueGenerator) {
@@ -1463,10 +1452,10 @@ describe('#integration temporal-types', () => {
14631452
}
14641453

14651454
async function testSendReceiveTemporalValue (value) {
1466-
const result = await session.run(
1455+
const result = await session.executeWrite(tx => tx.run(
14671456
'CREATE (n:Node {value: $value}) RETURN n.value',
14681457
{ value: value }
1469-
)
1458+
))
14701459

14711460
const records = result.records
14721461
expect(records.length).toEqual(1)

0 commit comments

Comments
 (0)