Open peterbe opened 3 years ago
Perhaps it's a bug in npm
because that exec()
shortcut function would crash if the command properly fails.
I sanity checked:
const { execSync } = require('child_process')
const exec = (str, cwd) => process.stdout.write(execSync(str, { cwd }))
exec('ls -l not.exists', '.')
if you run that (e.g. node test.js
) from bash it will fail and echo $?
will return 1
.
Doesn't look like a bug with npm
. I believe it's because the error is thrown inside of an async function context.
e.g. I don't think const run = async () => { throw new Error() }; run();
will properly return a non-zero.
It might be best to put a try/catch inside and return process.exit(1)
in the catch.
We could fix this particular problem by just printing the error and then doing process.exit(1)
here https://github.com/mikeal/merge-release/blob/7f590a5adf6e9def8e399a0a569d22d0de016dc0/src/merge-release-run.js
I think there’s a lack of consistency with this behavior in Node.js depending on whether or not you have anything in the event queue. The proper fix is for Node.js to throw on unhandled rejections, which will eventually be the default behavior.
I have encountered some variation of this problem as well
I see following:
fatal: Invalid symmetric difference expression 0f100775d2eddc660c3e9265f82013e3f2547e0a...22a246627438ee0f62a122c35f637ad10b414b75
current: 1.0.4
/ version: patch
new version: v1.0.5`
And then node runs out of memory and crashes, but action still appears successful.
Seeing this on my build as well
* master ab98de9 chore(deps): bump actions/setup-node from 2 to 3 (#80)
using deploy directory : /github/workspace/
using src directory (package.json) : /github/workspace/
fatal: Invalid symmetric difference expression c88e5f75ad7dc7e[17](https://github.com/meetup/graphql-joda-types/runs/5699144900?check_suite_focus=true#step:10:17)e5493d46333ffc77f65504e...ab98de9c2d[19](https://github.com/meetup/graphql-joda-types/runs/5699144900?check_suite_focus=true#step:10:19)8ae8f98bb14cdc0a5d4c092b45aa
current: 2.2.40
/ version: patch
new version: v2.2.41
Action ref, https://github.com/meetup/graphql-joda-types/runs/5699144900
Unfortunately, I can't find the run now but what happened was:
NPM_AUTH_TOKEN
merge-release
failed the build and I get a red X on the PR.merge-release
succeeded and I got the green checkmark on the PR.So, things like this clearly carries the exit code back out to
entrypoint.sh
. But I think theexec()
call does :(