Open rogerluan opened 3 years ago
orta
commented
3 years ago 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
rogerluan
commented
3 years ago Will do that today @orta - thanks for the feedback! Is this debugging env var documented anywhere? 😳🙇
orta
commented
3 years ago rogerluan
commented
3 years ago 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 😥
rogerluan
commented
3 years ago 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.jsInteresting, 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
rogerluan
commented
3 years ago @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? 😬
rogerluan
commented
3 years ago @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 😅
orta
commented
3 years ago 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)?
rogerluan
commented
3 years ago 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 😊
orta
commented
3 years ago ( edited above for clarity )
rogerluan
commented
3 years ago same @orta 😄
rogerluan
commented
3 years ago 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) 🙏
rogerluan
commented
3 years ago @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:
Here's what I tried so far:
export DANGER_GITHUB_API_TOKEN=ghp_……redacted…
export BITRISE_IO=true
export BITRISE_PULL_REQUEST=2322
export GIT_REPOSITORY_URL=git@github.com: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=redactedDEBUG="*" 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 🙏
rogerluan
commented
3 years ago 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 😅
SgtPooki
commented
2 years ago 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
SgtPooki
commented
2 years ago 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
Describe the bug
The following is printed even when providing the right GitHub token:
I know what you're thinking:
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:
export DANGER_GITHUB_API_TOKEN=132123312312132312export BITRISE_IO=trueexport BITRISE_PULL_REQUEST=9001export GIT_REPOSITORY_URL=https://github.com/artsy/eigenDANGER_FAKE_CIand similar env vars doesn't work, see below.danger ci --id "Danger Static Analysis test" --verboseIn Bitrise:
DANGER_GITHUB_API_TOKENas either a secret or env var (doesn't matter, but use secret to avoid exposing it)danger ci --id "Danger Static Analysis test" --verboseSimilar steps that DO NOT reproduce the issue (instead, they work as expected!):
DANGER_FAKE_CI,DANGER_TEST_REPOandDANGER_TEST_PR, things work as expected, updating the status check correctly. Weird 🤔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
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 --verboseNow, I tried debugging this issue as much as I could, so I went on a code review journey:
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:
commitHash()usesBITRISE_GIT_COMMIT, which is documented as: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.