From 30450d89f886c743e7acfa8f1bff156f473f893f Mon Sep 17 00:00:00 2001 From: Oscar Date: Fri, 26 Jul 2019 16:51:42 -0700 Subject: [PATCH] Fix timeout regression that caused Node.js to not terminate (#145) Co-authored-by: Sindre Sorhus --- index.js | 39 ++++++++++++++++++++++++--------------- test/main.js | 20 +++++++++++++++++++- 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/index.js b/index.js index 1be5d501..ff1235ea 100644 --- a/index.js +++ b/index.js @@ -115,27 +115,36 @@ class TimeoutError extends Error { } } -const delay = ms => new Promise((resolve, reject) => { +const safeTimeout = (resolve, reject, ms) => { if (ms > 2147483647) { // The maximum value of a 32bit int (see #117) reject(new RangeError('The `timeout` option cannot be greater than 2147483647')); - } else { - setTimeout(resolve, ms); } -}); + + return setTimeout(resolve, ms); +}; + +const delay = ms => new Promise((resolve, reject) => safeTimeout(resolve, reject, ms)); // `Promise.race()` workaround (#91) -const timeout = (promise, ms, abortController) => new Promise((resolve, reject) => { - /* eslint-disable promise/prefer-await-to-then */ - promise.then(resolve).catch(reject); - delay(ms).then(() => { - if (supportsAbortController) { - abortController.abort(); - } +const timeout = (promise, ms, abortController) => + new Promise((resolve, reject) => { + const timeoutID = safeTimeout(() => { + if (supportsAbortController) { + abortController.abort(); + } - reject(new TimeoutError()); - }).catch(reject); - /* eslint-enable promise/prefer-await-to-then */ -}); + reject(new TimeoutError()); + }, reject, ms); + + /* eslint-disable promise/prefer-await-to-then */ + promise + .then(resolve) + .catch(reject) + .then(() => { + clearTimeout(timeoutID); + }); + /* eslint-enable promise/prefer-await-to-then */ + }); const normalizeRequestMethod = input => requestMethods.includes(input) ? input.toUpperCase() : input; diff --git a/test/main.js b/test/main.js index 22c532c0..845694d9 100644 --- a/test/main.js +++ b/test/main.js @@ -196,7 +196,25 @@ test('invalid timeout option', async t => { // #117 }); await t.throwsAsync(ky(server.url, {timeout: 21474836470}).text(), RangeError, 'The `timeout` option cannot be greater than 2147483647'); - t.is(requestCount, 1); + t.is(requestCount, 0); + + await server.close(); +}); + +test('timeout option is cancelled when the promise is resolved', async t => { + const server = await createTestServer(); + + server.get('/', (request, response) => { + response.end(request.method); + }); + + const start = new Date().getTime(); + + await ky(server.url, {timeout: 2000}); + + const duration = start - new Date().getTime(); + + t.true(duration < 10); await server.close(); });