Skip to content

Fix non-executable execution with input option#212

Merged
sindresorhus merged 5 commits intosindresorhus:masterfrom
stroncium:fix-non-executable
May 7, 2019
Merged

Fix non-executable execution with input option#212
sindresorhus merged 5 commits intosindresorhus:masterfrom
stroncium:fix-non-executable

Conversation

@stroncium
Copy link
Contributor

@stroncium stroncium commented May 6, 2019

execa throws internal error instead of rejecting with proper error when running non-executable with input.

fixes #166

Failing test: https://travis-ci.org/sindresorhus/execa/builds/528756511?utm_source=github_status&utm_medium=notification

Added test, fixed the problem.

Copy link
Collaborator

@ehmicky ehmicky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except for comment about styling consistency with other tests.

@ehmicky
Copy link
Collaborator

ehmicky commented May 6, 2019

Thanks for this PR @stroncium!

This issue was reported with #166. There is an ongoing fix within Node.js here: nodejs/node#26852

However for current and older Node.js versions, we should definitely use that check, i.e. looks good to me.

Just need @sindresorhus review as well before merging.

Yanis Benson and others added 2 commits May 7, 2019 09:40
@sindresorhus sindresorhus changed the title Fix non-executable execution with input, fixes #166 Fix non-executable execution with input option May 7, 2019
@sindresorhus sindresorhus merged commit a428249 into sindresorhus:master May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash with input option and absolute path without execute permissions

3 participants