Skip to content

Commit 198eb9c

Browse files
apapirovskiBridgeAR
authored andcommitted
timers: reschedule interval even if it threw
To match browser behaviour, intervals should continue being scheduled even if the user callback threw during execution. PR-URL: #20002 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent a3bd06a commit 198eb9c

File tree

3 files changed

+37
-12
lines changed

3 files changed

+37
-12
lines changed

lib/timers.js

+7-8
Original file line numberDiff line numberDiff line change
@@ -285,15 +285,18 @@ function tryOnTimeout(timer, start) {
285285
var threw = true;
286286
if (timerAsyncId !== null)
287287
emitBefore(timerAsyncId, timer[trigger_async_id_symbol]);
288+
if (start === undefined && timer._repeat)
289+
start = TimerWrap.now();
288290
try {
289-
ontimeout(timer, start);
291+
ontimeout(timer);
290292
threw = false;
291293
} finally {
292294
if (timerAsyncId !== null) {
293295
if (!threw)
294296
emitAfter(timerAsyncId);
295-
if ((threw || !timer._repeat) && destroyHooksExist() &&
296-
!timer._destroyed) {
297+
if (timer._repeat) {
298+
rearm(timer, start);
299+
} else if (destroyHooksExist() && !timer._destroyed) {
297300
emitDestroy(timerAsyncId);
298301
timer._destroyed = true;
299302
}
@@ -417,18 +420,14 @@ setTimeout[internalUtil.promisify.custom] = function(after, value) {
417420
exports.setTimeout = setTimeout;
418421

419422

420-
function ontimeout(timer, start) {
423+
function ontimeout(timer) {
421424
const args = timer._timerArgs;
422425
if (typeof timer._onTimeout !== 'function')
423426
return promiseResolve(timer._onTimeout, args[0]);
424-
if (start === undefined && timer._repeat)
425-
start = TimerWrap.now();
426427
if (!args)
427428
timer._onTimeout();
428429
else
429430
Reflect.apply(timer._onTimeout, timer, args);
430-
if (timer._repeat)
431-
rearm(timer, start);
432431
}
433432

434433
function rearm(timer, start = TimerWrap.now()) {

test/async-hooks/test-timers.setInterval.js

+13-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const TIMEOUT = common.platformTimeout(100);
99
const hooks = initHooks();
1010
hooks.enable();
1111

12-
setInterval(common.mustCall(ontimeout), TIMEOUT);
12+
const interval = setInterval(common.mustCall(ontimeout, 2), TIMEOUT);
1313
const as = hooks.activitiesOfTypes('Timeout');
1414
assert.strictEqual(as.length, 1);
1515
const t1 = as[0];
@@ -18,9 +18,18 @@ assert.strictEqual(typeof t1.uid, 'number');
1818
assert.strictEqual(typeof t1.triggerAsyncId, 'number');
1919
checkInvocations(t1, { init: 1 }, 't1: when timer installed');
2020

21+
let iter = 0;
2122
function ontimeout() {
22-
checkInvocations(t1, { init: 1, before: 1 }, 't1: when first timer fired');
23-
23+
if (iter === 0) {
24+
checkInvocations(t1, { init: 1, before: 1 }, 't1: when first timer fired');
25+
} else {
26+
checkInvocations(t1, { init: 1, before: 2, after: 1 },
27+
't1: when first interval fired again');
28+
clearInterval(interval);
29+
return;
30+
}
31+
32+
iter++;
2433
throw new Error('setInterval Error');
2534
}
2635

@@ -32,6 +41,6 @@ process.on('exit', () => {
3241
hooks.disable();
3342
hooks.sanityCheck('Timeout');
3443

35-
checkInvocations(t1, { init: 1, before: 1, after: 1, destroy: 1 },
44+
checkInvocations(t1, { init: 1, before: 2, after: 2, destroy: 1 },
3645
't1: when process exits');
3746
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
// To match browser behaviour, interval should continue
7+
// being rescheduled even if it throws.
8+
9+
let count = 2;
10+
const interval = setInterval(() => { throw new Error('IntervalError'); }, 1);
11+
12+
process.on('uncaughtException', common.mustCall((err) => {
13+
assert.strictEqual(err.message, 'IntervalError');
14+
if (--count === 0) {
15+
clearInterval(interval);
16+
}
17+
}, 2));

0 commit comments

Comments
 (0)