[BUG] Bitrise - Could not add a commit status, the GitHub token for Danger does not have access rights.
Describe the bug
The following is printed even when providing the right GitHub token:
Found no issues or messages from Danger. Removing any existing messages on GitHub. Could not add a commit status, the GitHub token for Danger does not have access rights. If the build fails, then danger will use a failing exit code. Danger: ✓ passed review, received no feedback.
I know what you're thinking:
- [x] Is the scope right? Yes, I even created a Personal Access Token with global admin permissions (ticking all scope boxes)
- [x] Did you really provide the token? Yes, in fact the bot is able to post, in certain scenarios - see below
- [x] Were you able to reproduce this locally or just in CI? Both locally and in CI, both in Danger JS and Danger Swift.
To Reproduce
This issue is only reproducible in Bitrise CI environment (and locally, replicating CI's env vars), when Danger's output doesn't have any comments to make.
Steps to reproduce the behavior:
Locally:
- Set env vars:
-
export DANGER_GITHUB_API_TOKEN=132123312312132312 -
export BITRISE_IO=true -
export BITRISE_PULL_REQUEST=9001 -
export GIT_REPOSITORY_URL=https://github.com/artsy/eigen - NOTE that setting e.g.
DANGER_FAKE_CIand similar env vars doesn't work, see below.
- Run
danger ci --id "Danger Static Analysis test" --verbose - Observe the GitHub PR not updating its status check, and danger logging the error message aforementioned.
In Bitrise:
- Set
DANGER_GITHUB_API_TOKENas either a secret or env var (doesn't matter, but use secret to avoid exposing it) - Run
danger ci --id "Danger Static Analysis test" --verbose - Observe the GitHub PR not updating its status check, and danger logging the error message aforementioned.
Similar steps that DO NOT reproduce the issue (instead, they work as expected!):
- If Danger realizes it has comments to post in the PR, it is able to post the comment, and it is able to update the status check correctly, as expected. 🧐
- If instead of setting the 3 bitrise-specific env vars, I set
DANGER_FAKE_CI,DANGER_TEST_REPOandDANGER_TEST_PR, things work as expected, updating the status check correctly. Weird 🤔 - If I clone this repo, set the 3 bitrise env vars as well as github token env var, and run it locally using
yarn build; node --inspect distribution/commands/danger-ci.js --id "Danger Static Analysis test2" --verbose, it also works as expected. Go figure 🤔
Expected behavior
At the risk of sounding obvious, if I provide the github api token I expect the bot to be able to update the status check to green when it finds no issues.
Screenshots
Nothing would help probably, let me know if this is needed.
Your Environment
| software | version |
|---|---|
| danger.js | 10.6.2 |
| node | v15.7.0 |
| npm | 7.4.3 |
| Operating System | macOS 11.2.1 intel-based machine (CI is in 10.15 though) |
Additional context
We use Danger Swift actually. But since Danger Swift uses JS under the hood, I thought of testing the same scenario in Danger JS and it could be reproduced as well, hence why I'm opening this issue in this repo :) So just for completeness sake, here's the command we're using in Danger Swift:
swift run danger-swift ci --id 'Danger Static Analysis test' --dangerfile ./Dangerfile.swift --verbose
Now, I tried debugging this issue as much as I could, so I went on a code review journey:
- I thought this regex could be flaky/wrong:
https://github.com/danger/danger-js/blob/c4fa1308d969af7acc1b2ae052c1a75fbe26a9c3/source/ci_source/providers/Bitrise.ts#L59-L63
But it's not, I verified using real values (from my CI, and local tests).
- I reviewed all the other values in Bitrise.js and they all look right, except:
-
This env var looked wrong: https://github.com/danger/danger-js/pull/1113
-
commitHash()usesBITRISE_GIT_COMMIT, which is documented as:The commit hash of the Git commit that triggered the build, when applicable. There is no commit hash, for example, in the case of a pull request build.
So, naturally, since Danger is required to run in a Pull Request build, then the commit will never be available. This sounded like a problem initially, I still don't know if it is, but then if that was the case, Danger would have to fail at all cases when running locally cuz we never provide any commit hash to it 🤷 PS: I still tested this possibility by explicitly setting BITRISE_GIT_COMMIT to the right commit hash and it still won't work.
-
Interesting, I think running with DEBUG="networking" (focused) or DEBUG="*" (lots of logs) in the ENV should give back the JSON response from GitHub saying why it failed, which might make this much easier t odebug
Will do that today @orta - thanks for the feedback! Is this debugging env var documented anywhere? 😳🙇
Yep: https://danger.systems/js/guides/faq.html#i--m-not-sure-what-danger-is-doing
Using DEBUG="networking" didn't show any extra logs. DEBUG="*" did it:
However, nothing particularly useful. I ran the same command passing in the DANGER_FAKE_CI, DANGER_TEST_REPO and DANGER_TEST_PR env vars (the ones that make the request work just fine), and literally the only difference between the two are that one prints this out:
Could not add a commit status, the GitHub token for Danger does not have access rights.
If the build fails, then danger will use a failing exit code.
And the other doesn't. I compared using a text diff tool.
Is there a way to print even more debugging logs? 😬 Since running yarn build; node --inspect distribution/commands/danger-ci.js locally doesn't trigger the failure, I can't really test by adding extra logs to source/runner/Executor.ts+L345 😥
Is there any way for me to run danger-js from master instead of latest release (server-side, instead of running my local version)? Cuz I think this line would help debugging: https://github.com/danger/danger-js/commit/ade818be29a8497e5cfe879c8885a0af549b328f#diff-29756791c16c9d7d9e6029209f1946b47bb32af13c39177cff12e5e212dd6a7dR394
I wanna spot the difference and understand why running locally doesn't fail the build. Is my command correct?
yarn build; node --inspect distribution/commands/danger-ci.js
Interesting, no, there's no more debugging logs than the debug ones - I deployed a build of danger last night, so there shouldn't be any differences between prod and master right now. Actually, In writing this I just shipped another version with https://github.com/danger/danger-js/commit/2b77252d94910802ea4bd2f01af856b1e6176b02 which should give you better logs here
@orta thanks for publishing this release!
So, the worst happened. Good news and bad news in the same message: it works now. So something changed between the previous version and this one, but after reviewing the code, I have no idea what would've changed. If I remember correctly I tested these scenarios in the old master (before my 3 PRs get merged), and it would already work, so the scope of change was even smaller. I'm so confused 😅
Are there other internal dependencies that danger-js rely on? Do you have any clue on what changed? 😬
@orta nevermind! I figured it out. This fixed it: https://github.com/danger/danger-js/pull/1113
Success! Thank you so much for providing the right tools to get this issue resolved 😊 Just a quick note: https://github.com/danger/danger-js/commit/2b77252d94910802ea4bd2f01af856b1e6176b02 doesn't seem to be working properly - even when my issue is reproducible (i.e. reverting #1113), it doesn't print the "got a non-ok…" message). There's probably something funky with the condition 😅
Are there other internal dependencies that danger-js rely on? Do you have any clue on what changed? 😬
Normally I would say there's a chance that the GitHub API would have changed, but I think all this code was hand-rolled by me in Danger, so aside from your + my changes, I don't think anything else changed. Maybe the access tokens have different rights (fork vs branch)?
Ah nevermind that comment of mine, I was just wondering if there were other forces at play, but apparently not, the solution was really in the danger-js diff 😊
( edited above for clarity )
same @orta 😄
I'm closing this issue, as the problem was in fact resolved 💪 Thank you so much! Let me know if you need help debugging these logs somehow https://github.com/danger/danger-js/commit/2b77252d94910802ea4bd2f01af856b1e6176b02 as they're not showing up 🙈 (at least not in the scenario that we needed) 🙏
@orta I started seeing this issue again last week: when it needs to post a message, the github status is updated correctly, but when it doesn't it can't update the status check. It prints this:
https://github.com/danger/danger-js/blob/fe5f080b4a267012dd80a9d589faee3bd278dc18/source/runner/Executor.ts#L345-L348
Here's what I tried so far:
- Double checked for the same issues that were present last time around;
- Manually checked if Bitrise didn't break their env vars for some reason (I printed all of them on the CI console and they look correct)
- Checked for recent changes in the env var documentation, but the most recent commit just changes a hyperlink reference, not anything functional: https://github.com/bitrise-io/devcenter/blob/master/_articles/en/builds/available-environment-variables.md
- Tried running danger-js latest master locally and I couldn't reproduce the issue
- Tried running the exact same flow as run in CI (danger swift that pulls latest version of danger js under the hood), using the same env vars:
export DANGER_GITHUB_API_TOKEN=ghp_……redacted…
export BITRISE_IO=true
export BITRISE_PULL_REQUEST=2322
export [email protected]:redacted/redacted.git # also tried https://github.com/redacted/redacted but the git@ is the URL style that Bitrise is actually using, and both worked fine locally.
export BITRISEIO_GIT_REPOSITORY_OWNER=redacted
export BITRISEIO_GIT_REPOSITORY_SLUG=redacted
- I added
DEBUG="*"before the command and I got this on my local machine that simulates the CI environment:
…
danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination +576ms
danger:GitHubAPI Sending: https://api.github.com/repos/redacted-org/redacted-repo/statuses/50391411063f61e666cdd5de24f585a246a38f92 {
'Content-Type': 'application/json',
Authorization: 'token ghp_redacted…redacted……'
} +2ms
danger:executor Writing to STDOUT: {
messages: [],
markdowns: [],
meta: {
runtimeHref: 'https://danger.systems/swift',
runtimeName: 'Danger Swift'
},
fails: [],
warnings: []
} +3s
Danger: ✓ passed review, received no feedback.
And on CI (different commit but should be the same thing):
2021-05-11T03:12:20.267Z danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination
2021-05-11T03:12:20.267Z danger:GitHubAPI Sending: https://api.github.com/repos/redacted-org/redacted-repo/statuses/50391411063f61e666cdd5de24f585a246a38f92 {
'Content-Type': 'application/json',
Authorization: 'token [REDACTED]'
}
Could not add a commit status, the GitHub token for Danger does not have access rights.
If the build fails, then danger will use a failing exit code.
2021-05-11T03:12:20.516Z danger:executor Writing to STDOUT: {
messages: [],
markdowns: [],
meta: {
runtimeHref: 'https://danger.systems/swift',
runtimeName: 'Danger Swift'
},
fails: [],
warnings: []
}
Danger: ✓ passed review, received no feedback.
No luck, though.
I ran out of ideas, anything else I could try? 😞
I'm almost giving up on Danger Swift + JS and trying Danger Ruby 😥
Any help will be greatly appreciated 🙏
Latest update:
I tried the exact same setup but replaced danger-swift (that uses danger-js under the hood) with danger-ruby (pure ruby implementation), and it works correctly. This confirms it's not an issue on GitHub's end, it's most likely not an issue on Bitrise's end. But then again lately there have been no relevant changes to danger-js that would explain this issue 😬 really cryptic bug 😭 I'm leaning even more towards migrating all my Dangerfile.swift to Ruby 😅
this error is always occurring for me locally as well. Also, fail is not failing a PR's checks: https://github.com/SgtPooki/awesome-ipfs/pull/2#issuecomment-1246145056
Everything passed locally when the bot the GH_TOKEN I was using was added to the repo as a collaborator. https://github.com/SgtPooki/awesome-ipfs/pull/2#issuecomment-1247181898