fix: gracefully handle existing PR instead of failing#51
fix: gracefully handle existing PR instead of failing#51mateoviilla1 wants to merge 1 commit intodevelopfrom
Conversation
When a pull request from head into base already exists, the action previously crashed with a 422 Validation Failed error. Now it logs a message, finds the existing open PR, and continues to apply labels, reviewers, and assignees to it.
There was a problem hiding this comment.
Pull request overview
Improves the sync-master-develop GitHub Action to avoid failing the workflow when a sync PR already exists by catching the duplicate-PR error and reusing the existing PR.
Changes:
- Wrap
octokit.rest.pulls.create()in atry/catchto handle duplicate PR creation errors (HTTP 422). - When a duplicate is detected, query for the existing open PR and continue applying labels/reviewers/assignees using that PR number.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (err) { | ||
| const { status, message } = err as OctokitHttpError | ||
| if ( | ||
| String(status) === '422' && | ||
| /A pull request already exists/.test(String(message)) | ||
| ) { |
There was a problem hiding this comment.
The duplicate-PR detection is based on matching err.message against /A pull request already exists/. For pulls.create 422 responses, GitHub commonly returns a top-level message like "Validation Failed" with the specific reason located in err.response.data.errors[]. In that case this check won’t match and the action will still throw, reintroducing the original workflow failure. Consider inspecting err.response?.data?.errors (or similar RequestError fields) for an error entry whose message contains the duplicate-PR text (or a stable code/resource combination) instead of relying on the top-level message string.
| base, | ||
| state: 'open' | ||
| }) | ||
| if (!existingPRs.length) return |
There was a problem hiding this comment.
If a 422 duplicate-PR error is detected but pulls.list returns no open PRs (e.g., due to a filter mismatch or API inconsistency), the action currently returns early without applying labels/reviewers/assignees and without surfacing an error. It would be safer to log and throw in this situation (or at least fail the action) so the workflow doesn’t silently succeed while skipping the remaining steps.
| if (!existingPRs.length) return | |
| if (!existingPRs.length) { | |
| const errorMessage = | |
| `GitHub reported that a pull request from "${head}" into "${base}" already exists, ` + | |
| `but no open pull request was found via pulls.list (head="${context.repo.owner}:${head}", base="${base}").` | |
| core.error(errorMessage) | |
| throw new Error(errorMessage) | |
| } |
| // Create PR | ||
| const prResponse = await octokit.rest.pulls.create({ | ||
| ...context.repo, | ||
| title, | ||
| head, | ||
| base, | ||
| body: | ||
| body || | ||
| Buffer.from( | ||
| pullRequestTemplate | ||
| ? // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (pullRequestTemplate as any).content | ||
| : '', | ||
| 'base64' | ||
| ).toString() | ||
| }) | ||
| const { number: pullNumber } = prResponse.data | ||
| let pullNumber: number | ||
| try { | ||
| const prResponse = await octokit.rest.pulls.create({ | ||
| ...context.repo, | ||
| title, | ||
| head, | ||
| base, | ||
| body: | ||
| body || | ||
| Buffer.from( | ||
| pullRequestTemplate | ||
| ? // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (pullRequestTemplate as any).content | ||
| : '', | ||
| 'base64' | ||
| ).toString() | ||
| }) | ||
| pullNumber = prResponse.data.number | ||
| } catch (err) { | ||
| const { status, message } = err as OctokitHttpError | ||
| if ( | ||
| String(status) === '422' && | ||
| /A pull request already exists/.test(String(message)) | ||
| ) { | ||
| console.log( | ||
| `A pull request from "${head}" into "${base}" already exists, skipping creation.` | ||
| ) | ||
| const { data: existingPRs } = await octokit.rest.pulls.list({ | ||
| ...context.repo, | ||
| head: `${context.repo.owner}:${head}`, | ||
| base, | ||
| state: 'open' | ||
| }) | ||
| if (!existingPRs.length) return | ||
| pullNumber = existingPRs[0].number | ||
| } else { | ||
| throw err | ||
| } | ||
| } |
There was a problem hiding this comment.
This new duplicate-PR recovery path (handling a 422 from pulls.create, then calling pulls.list and continuing with labels/reviewers/assignees) isn’t covered by the existing test suite in test/main.test.ts, which currently only covers the successful create path and template-fetch errors. Please add a test that mocks a 422 duplicate response from POST /pulls, verifies a GET /pulls lookup happens, and asserts subsequent label/reviewer/assignee calls target the existing PR number.
Problem
When the
sync-master-developworkflow triggers on multiple pushes tomaster, the action tries to create a new sync PR each time. If one already exists, GitHub returns a 422 and the action crashes — causing the job to fail and block the sync PR from being merged (circular deadlock).Solution
Wrap
pulls.create()in a try-catch. When a 422 "A pull request already exists" error is detected:pulls.list()Changes
src/main.ts: added error handling aroundpulls.create()for the duplicate PR case