Skip to content

Commit

Permalink
Fix timeout regression that caused Node.js to not terminate (#145)
Browse files Browse the repository at this point in the history
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
obartra and sindresorhus committed Jul 26, 2019
1 parent 2ea165a commit 30450d8
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 16 deletions.
39 changes: 24 additions & 15 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
20 changes: 19 additions & 1 deletion test/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down

0 comments on commit 30450d8

Please sign in to comment.