Skip to content

Commit c252d90

Browse files
committed
fix(queue): remove the unreachable catch masking a disallowed reopen
The outer .catch() around maybeRecloseDisallowedReopen could never actually fire — the lock claim/release fail open and every step in recloseDisallowedReopenIfNeeded already catches its own errors — so codecov/patch flagged it as an uncoverable line. Swallowing an unexpected error there into a silent 'allowed' would have re-permitted the exact disallowed reopen this guard exists to stop, so removing it is also the safer behavior: let it propagate and retry.
1 parent 8ab9ec6 commit c252d90

1 file changed

Lines changed: 6 additions & 1 deletion

File tree

src/queue/processors.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3785,6 +3785,11 @@ async function processGitHubWebhook(
37853785
// closed their own PR) stay reopenable; the bot's own nightly-re-review reopens are exempt. A contended
37863786
// actuation lock ALSO skips the re-review (#2135, review round 3) — the winning delivery already owns
37873787
// this PR, so this pass must not evaluate/mutate it concurrently under a false "not blocked" reading.
3788+
// Deliberately UNCAUGHT here: every step inside maybeRecloseDisallowedReopen already fails safe on its own
3789+
// (the lock claim/release fail open; recloseDisallowedReopenIfNeeded's own operations all .catch()), so a
3790+
// swallowing catch at this call site could only ever mask a genuinely unexpected error into a silent
3791+
// "allowed" — which would re-permit exactly the disallowed reopen this guard exists to stop. Let it
3792+
// propagate and retry instead, same reasoning as the draft-dodge sibling's uncaught getInstallation read.
37883793
const reopenOutcome: ReopenRecloseOutcome =
37893794
payload.action === "reopened" && installationId
37903795
? await maybeRecloseDisallowedReopen(
@@ -3794,7 +3799,7 @@ async function processGitHubWebhook(
37943799
repoFullName,
37953800
pr,
37963801
payload,
3797-
).catch(() => "allowed" as const)
3802+
)
37983803
: "allowed";
37993804
if (reopenOutcome === "reclosed" || reopenOutcome === "lock_contended") {
38003805
// Stamp the delivery processed like every other owning path — the early return otherwise leaves the

0 commit comments

Comments
 (0)