danger process isn't exiting the process with the same error code as the subprocess

[orta]

I saw it "fail" a bunch, but actually pass. It even said "process exited with code '1'" or whatever.

[JamieMason]

I have a dangerfile.js containing:

import { danger, fail, message } from 'danger';

const modified = danger.git.modified_files.join('- ');
const created = danger.git.created_files.join('- ');

message('Created Files in this PR: \n - ' + created);
message('Changed Files in this PR: \n - ' + modified);

fail('force a fail to see what happens');

which seems to have an exit code of 0 when run using:

GITHUB_REPO_SLUG="some-org/some-repo" \
GITHUB_REPO_URL="https://github.com/some-org/some-repo.git" \
DANGER_GITHUB_API_TOKEN="some-token" \
PULL_REQUEST_URL="https://github.com/some-org/some-repo/pull/4" \
yarn danger ci

Is this the same issue as the OP? If so I'd be happy to help try and fix it if I could be pointed at some more information. I noticed in the danger/danger repo some mention of a --fail-on-errors option, but this doesn't seem to be available in the danger/danger-js repo?

Thanks a lot.

[orta]

If danger can successfully set a commit status to be red, it won't fail the build. ( Whereas the --fail-on-error option would make it fail regardless )

Basically this is a guard against people having danger run first on their CI, and because you failed a CHANGELOG check or something it won't pass through to unit tests etc

If this isn't what's happening, then I definitely think it's abug

[JamieMason]

Understood re: not failing the build, I think we'd just misunderstood the difference between the CI build failing and the PR failing via danger.

Regarding --fail-on-error / --fail-on-errors though: danger ci throws if you try to use that, saying it is not recognised?

We're using "danger": "^3.3.2" in package.json.

[orta]

Yeah, looks like that flag hasn't been moved over form the ruby version, I don't use it - so I've not ported it

~/dev/projects/artsy/js/sites/metaphysics validate_env
❯ danger ci --help

  Usage: danger-ci [options]

  Runs a Dangerfile in JavaScript or TypeScript.

  Options:

    -v, --verbose                            Verbose output of files
    -c, --external-ci-provider [modulePath]  Specify custom CI provider
    -t, --text-only                          Provide an STDOUT only interface, Danger will not post to your PR
    -d, --dangerfile [filePath]              Specify a custom dangerfile path
    -i, --id [danger_id]                     Specify a unique Danger ID for the Danger run
    -b, --base [branch_name]                 Base branch
    -h, --help                               output usage information

[orta]

I think the messaging could be improved about the process exit code IMO too

[weitzman]

This was recently added. See --failOnErrors
https://github.com/danger/danger-js/blob/master/source/commands/utils/sharedDangerfileArgs.ts#L27

[adityaridha]

Hi, need help on similar issue...
I'm running danger using NPM with --failOnErrors flag, but the job still a success
Any idea what's wrong with the implementation ?

{
  "dependencies": {
    "danger": "^10.7.1",
    "get-urls": "^10.0.1",
    "node-fetch": "^3.1.0",
    "pg": "^8.6.0",
    "pg-hstore": "^2.3.4",
    "sequelize": "^6.6.5"
  },
  "type": "module",
  "scripts": {
    "danger": "danger"
  }
}

npm run danger ci --failOnErrors --verbose
npm verb cli [
npm verb cli   '/usr/local/bin/node',
npm verb cli   '/usr/local/bin/npm',
npm verb cli   'run',
npm verb cli   'danger',
npm verb cli   'ci',
npm verb cli   '--failOnErrors',
npm verb cli   '--verbose'
npm verb cli ]
npm info using npm@7.15.1
npm info using node@v16.3.0

Failing the build, there is 1 fail.
Feedback: https://gitlab.com/xxxxx-id/xx-xxx/xxx-xxx/merge_requests/1#note_801699689
npm timing command:run-script Completed in 21807ms
npm verb exit 0
npm timing npm Completed in 21966ms
npm info ok 

[binary64]

I too am looking for danger to exit with a non-0 level when there is 1 or more fail()'s emitted. This is because I'm using --no-publish-check because I wanted a single unified build on my PRs.

[dpobel]

@binary64 it looks like danger ci has to be called with --failOnErrors so that it actually exit with a non 0 process exit status.

The Failing the build message is really confusing, without this option there's no way for danger to actually fail a build…

[kmannislands]

I think this is defaulted the wrong way.

I'd certainly expect that fail() fails the build ie: exits the danger process non-zero.

If that is not the desired behavior for some use-case, providing a flag like --exitZeroOnFail might make sense. Or just leaving it to the user and expecting them to run this something like: yarn danger ci || true

[fbartho]

@kmannislands -- this ticket isn't about that case. If you'd like to propose the breaking change of defaulting --failOnErrors to be the standard behavior, feel free to file a separate ticket.

That said, this would be a breaking change, and it just hasn't been a priority since users can get the behavior they want today.

[muuvmuuv]

I would like to test with danger local where it doesn't exit as well.