Description
checkAndRetryJob (the job retry lambda) has no error handling around its call to isJobQueued. If that call throws for any reason (a transient GitHub API error, a rate limit, an unsupported event type), the error propagates out of the whole function, skipping both the actual retry (publishMessage) and the RetryJob metric.
At the call site, that error is caught and only logged as a warning:
await checkAndRetryJob(payload).catch((e) => {
logger.warn(`Error processing job retry: ${e.message}`, { error: e });
});so the SQS message is still marked as processed and never redelivered.
Impact
On any transient GitHub API error during a retry check, the job is dropped for good instead of being retried, and there's no metric recording that it happened. This repo has already closed two issues for the same failure shape in the main scale-up path (#5024, #5105) — the retry lambda has the same class of bug but was never fixed.
Proposed fix
Wrap the isJobQueued call in a try/catch that mirrors the equivalent check already in scale-up.ts: skip the retry only for an UnsupportedEventError (that error can never resolve itself), and for any other error, assume the job is still queued and publish the retry anyway — a transient error is not evidence the job stopped needing a runner.
See PR (to follow) for the implementation.
Description
checkAndRetryJob(the job retry lambda) has no error handling around its call toisJobQueued. If that call throws for any reason (a transient GitHub API error, a rate limit, an unsupported event type), the error propagates out of the whole function, skipping both the actual retry (publishMessage) and theRetryJobmetric.At the call site, that error is caught and only logged as a warning:
so the SQS message is still marked as processed and never redelivered.
Impact
On any transient GitHub API error during a retry check, the job is dropped for good instead of being retried, and there's no metric recording that it happened. This repo has already closed two issues for the same failure shape in the main scale-up path (#5024, #5105) — the retry lambda has the same class of bug but was never fixed.
Proposed fix
Wrap the
isJobQueuedcall in a try/catch that mirrors the equivalent check already inscale-up.ts: skip the retry only for anUnsupportedEventError(that error can never resolve itself), and for any other error, assume the job is still queued and publish the retry anyway — a transient error is not evidence the job stopped needing a runner.See PR (to follow) for the implementation.