Skip to content

Commit 06a1879

Browse files
committed
fix(execution): honor retryable-failure suppression for unavailable admission
The new admission-infrastructure log row ignored suppressRetryableFailureLogs, so a webhook with a setup retry still available recorded a terminal failure before the same execution was requeued. That option exists precisely for this shape: statusCode >= 500 and retryable, which RESERVATION_INFRASTRUCTURE (503, retryable) matches. The denial branch nearby is unaffected because its descriptors are 402/429. Builds the failure once and reuses it for both the suppression check and the returned error rather than duplicating the shape.
1 parent c26b83b commit 06a1879

2 files changed

Lines changed: 61 additions & 22 deletions

File tree

apps/sim/lib/execution/preprocessing.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ vi.mock('@/lib/core/rate-limiter/rate-limiter', () => ({
6262
}))
6363
vi.mock('@/lib/logs/execution/logging-session', () => loggingSessionMock)
6464

65+
import { UsageReservationUnavailableError } from '@/lib/billing/calculations/usage-reservation'
6566
import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription'
6667
import { preprocessExecution, WORKFLOW_NOT_DEPLOYED_CODE } from './preprocessing'
6768

@@ -289,6 +290,38 @@ describe('preprocessExecution suppressRetryableFailureLogs option', () => {
289290
expect(loggingSession.safeStart).not.toHaveBeenCalled()
290291
})
291292

293+
it('skips the failure row when admission infrastructure is unavailable and a retry remains', async () => {
294+
mockReserveExecutionSlot.mockRejectedValueOnce(
295+
new UsageReservationUnavailableError('Usage admission is temporarily unavailable.')
296+
)
297+
const loggingSession = makeLoggingSession()
298+
299+
const result = await preprocessExecution({
300+
...baseOptions,
301+
suppressRetryableFailureLogs: true,
302+
loggingSession: loggingSession as any,
303+
})
304+
305+
expect(result).toMatchObject({ success: false, error: { statusCode: 503, retryable: true } })
306+
expect(loggingSession.safeStart).not.toHaveBeenCalled()
307+
})
308+
309+
it('records the failure row for an unavailable admission when no retry remains', async () => {
310+
mockReserveExecutionSlot.mockRejectedValueOnce(
311+
new UsageReservationUnavailableError('Usage admission is temporarily unavailable.')
312+
)
313+
const loggingSession = makeLoggingSession()
314+
315+
const result = await preprocessExecution({
316+
...baseOptions,
317+
suppressRetryableFailureLogs: false,
318+
loggingSession: loggingSession as any,
319+
})
320+
321+
expect(result).toMatchObject({ success: false, error: { statusCode: 503, retryable: true } })
322+
expect(loggingSession.safeStart).toHaveBeenCalled()
323+
})
324+
292325
it('still records non-retryable failures while suppression is on', async () => {
293326
workflowAuthzMockFns.mockGetActiveWorkflowRecord.mockRejectedValueOnce(
294327
new Error('column "unknown" does not exist')

apps/sim/lib/execution/preprocessing.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -822,32 +822,38 @@ export async function preprocessExecution(
822822
* admission infrastructure was unreachable left no execution log at all,
823823
* so it disappeared from the workspace's logs rather than showing as a
824824
* failure — the denial branch twenty lines up always recorded one.
825+
*
826+
* Unlike that branch, this one has to honor the suppression: its denial
827+
* descriptors are 402/429, while this failure is 503 and retryable, which
828+
* is precisely what a requeuing caller defers. Recording here regardless
829+
* would write a terminal failure for an execution about to be retried.
825830
*/
826-
await recordPreprocessingError({
827-
workflowId,
828-
executionId,
829-
triggerType,
830-
requestId,
831-
userId: actorUserId,
832-
workspaceId,
833-
errorMessage: unavailable.message,
834-
loggingSession: providedLoggingSession,
835-
triggerData,
836-
})
837-
838-
return {
839-
success: false,
840-
error: {
841-
message: unavailable.message,
842-
statusCode: unavailable.statusCode,
831+
const unavailableFailure: PreprocessExecutionError = {
832+
message: unavailable.message,
833+
statusCode: unavailable.statusCode,
834+
code: unavailable.code,
835+
retryable: unavailable.retryable,
836+
...retryAfterMsFrom(unavailable.retryAfterSeconds),
837+
cause: {
843838
code: unavailable.code,
844-
retryable: unavailable.retryable,
845-
...retryAfterMsFrom(unavailable.retryAfterSeconds),
846-
cause: {
847-
code: unavailable.code,
848-
},
849839
},
850840
}
841+
842+
if (!isFailureLogSuppressed(unavailableFailure)) {
843+
await recordPreprocessingError({
844+
workflowId,
845+
executionId,
846+
triggerType,
847+
requestId,
848+
userId: actorUserId,
849+
workspaceId,
850+
errorMessage: unavailable.message,
851+
loggingSession: providedLoggingSession,
852+
triggerData,
853+
})
854+
}
855+
856+
return { success: false, error: unavailableFailure }
851857
}
852858
}
853859

0 commit comments

Comments
 (0)