Skip to content

Commit

Permalink
Fix error.timedOut not working with execa.sync() (#249)
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky authored and sindresorhus committed May 14, 2019
1 parent f5172cd commit 21c706a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
2 changes: 1 addition & 1 deletion index.js
Expand Up @@ -431,7 +431,7 @@ module.exports.sync = (command, args, options) => {
code: result.status,
joinedCommand,
parsed,
timedOut: false,
timedOut: result.error && result.error.errno === 'ETIMEDOUT',

This comment has been minimized.

Copy link
@Trott

Trott Oct 2, 2019

Contributor

Currently, this will break in Node.js 13.0.0 that comes out later this month because errno was changed to always be numeric. See nodejs/node@1432065. I believe changing errno to code would fix this. I'll give that a try and submit a PR if it turns out to be true, but pushing back on Node.js to pull that commit out of the 13.0.0 release (or revert it) is also an option if execa needs to support old versions of Node.js prior to the code property being a thing. However, the engines entry in package.json suggests that isn't the case.

This comment has been minimized.

Copy link
@ehmicky

ehmicky Oct 2, 2019

Author Collaborator

Thanks @Trott!

isCanceled: false,
killed: result.signal !== null
});
Expand Down
8 changes: 8 additions & 0 deletions test.js
Expand Up @@ -395,6 +395,14 @@ test('timeout kills the process if it times out', async t => {
t.true(timedOut);
});

test('timeout kills the process if it times out, in sync mode', async t => {
const {killed, timedOut} = await t.throws(() => {
execa.sync('forever', {timeout: 1, message: TIMEOUT_REGEXP});
});
t.false(killed);
t.true(timedOut);
});

test('timeout does not kill the process if it does not time out', async t => {
const {timedOut} = await execa('delay', ['500'], {timeout: 1e8});
t.false(timedOut);
Expand Down

0 comments on commit 21c706a

Please sign in to comment.