Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Multiple inline comments don't work #1134

Closed
hlynurstef opened this issue Apr 9, 2021 · 4 comments
Closed

[BUG] Multiple inline comments don't work #1134

hlynurstef opened this issue Apr 9, 2021 · 4 comments
Labels
bug You Can Do This This idea is well spec'd and ready for a PR

Comments

@hlynurstef
Copy link

hlynurstef commented Apr 9, 2021

Describe the bug
When you try to post multiple inline comments, only the first comment is posted while the others fail (but still appear in the overview comment).

To Reproduce
Steps to reproduce the behavior:

  1. Create a danger rule that warn/fail/message on a specific file and line number on several files that are included in the diff on GitHub
  2. Run yarn danger ci from your CI
  3. Notice that only the first inline comment is posted while the others fail

Expected behavior
Danger should post all of the inline comments requested.

** Your Environment**

software version
danger.js 10.6.3
node
npm
Operating System macOS 10.15.7 (Catalina)

Additional context
When running danger with the following command: DEBUG={*} yarn danger ci I see that there seem to be two requests made to the same endpoint at almost the same time (maybe related to the error?) and then we can see that the request failed with an error from the GitHub API:

2021-04-09T01:14:10.956Z danger:GitHubAPI Sending:  https://api.github.com/repos/driftline-analytics/runmaker/pulls/43/comments {
  'Content-Type': 'application/json',
  Authorization: 'token [REDACTED]'
}
2021-04-09T01:14:10.959Z danger:GitHubAPI Sending:  https://api.github.com/repos/driftline-analytics/runmaker/pulls/43/comments {
  'Content-Type': 'application/json',
  Authorization: 'token [REDACTED]'
}
Request failed [422]: https://api.github.com/repos/driftline-analytics/runmaker/pulls/43/comments
Response: {
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "PullRequestReviewComment",
      "code": "custom",
      "field": "pull_request_review_id",
      "message": "pull_request_review_id must be pending"
    }
  ],
  "documentation_url": "https://docs.github.com/rest/reference/pulls#create-a-review-comment-for-a-pull-request"
}

This seems related to danger/swift#301

@hlynurstef hlynurstef added the bug label Apr 9, 2021
@orta
Copy link
Member

orta commented Apr 9, 2021

Cool, yeah, maybe the API usage rules changed this since this was originally built out. I don't use inline messages, so I'm unlikely to fix it - but I'd accept PRs 👍🏻

@orta orta added the You Can Do This This idea is well spec'd and ready for a PR label Apr 9, 2021
@hlynurstef
Copy link
Author

Seems like the multiple inline comments have started working correctly all of a sudden, I haven't updated any dependencies so I'm guessing something might have changed serverside from GitHub?

Since this has started working correctly I'm going to close this issue.

@hlynurstef
Copy link
Author

Actually I jumped the gun on closing this. I thought this was fixed because my CI ran a few times and every time it re-ran the PR it posted one new inline comment at a time. Then when I took a look at the PR it looked like danger had posted all of the inline comments at once but in reality it only added one at a time per each CI run.

So I'm re-opening this issue as it's not fixed. I'm still seeing the same error messages in my CI logs when danger tries to post multiple inline comments.

@fbartho
Copy link
Member

fbartho commented Mar 14, 2022

Issue was resolved in #1176 ! Closing.

@fbartho fbartho closed this as completed Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug You Can Do This This idea is well spec'd and ready for a PR
Projects
None yet
Development

No branches or pull requests

3 participants