Skip to content

github: Make addComment idempotent to prevent duplicate PR comments#225

Open
matias-christensen-skydio wants to merge 1 commit intomainfrom
fix-duplicate-comments
Open

github: Make addComment idempotent to prevent duplicate PR comments#225
matias-christensen-skydio wants to merge 1 commit intomainfrom
fix-duplicate-comments

Conversation

@matias-christensen-skydio
Copy link
Copy Markdown
Contributor

Summary

  • Fixes a bug where retrying a GraphQL mutation after a transient error (5xx or RESOURCE_LIMITS_EXCEEDED) would re-post addComment mutations that the server had already applied, producing duplicate stack outline and diff table comments on PRs.
  • Moves the retry loop from the generic graphql() method into update_pull_requests() where it has context to handle idempotency. Between retry attempts, re-queries each PR's comments and converts any already-posted new comments to updateIssueComment (edit) operations.
  • Retry coverage for all transient GitHub API failures is preserved.

Test plan

  • Run revup upload on a stack of PRs and verify comments are not duplicated
  • Simulate a transient failure (e.g. by temporarily injecting a raised RevupRequestException(502, {}) after the first attempt) and verify the retry re-queries comments and converts adds to edits

When retrying a mutation after a transient error (5xx or
RESOURCE_LIMITS_EXCEEDED), addComment would re-post comments that the
server had already created, producing duplicate stack outlines and diff
tables on PRs.

Move the retry loop from the generic graphql() method into
update_pull_requests() where it has context to handle this. Between
retry attempts, re-query each PR's comments and convert any
already-posted new comments to updateIssueComment (edit) operations.
This makes the retry idempotent while preserving retry coverage for
all transient GitHub API failures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants