Didn't exit non-zero when it failed

[peterbe]

Unfortunately, I can't find the run now but what happened was:

1. I set up this workflow and run it
2. Oops I hadn't set up the NPM_AUTH_TOKEN
3. As expected, the merge-release failed the build and I get a red X on the PR.
4. Fix it and run again.
5. It failed because I hadn't published the npm package initially from my machine. I.e. it had never ever been published before.
6. 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 the exec() call does :(

[peterbe]

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.

[dougrday]

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.

[mikeal]

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.

[Gozala]

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.

[chenrui333]

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