Skip to content

Commit 96ee093

Browse files
committed
fix(logs): keep trusted ids beside projected diagnostics and dedupe resumes on the parent run
The resume and child failure lines keep their execution, block, and resume ids outside the secret projection, which drops everything when it fails closed. Resumed runs execute under the parent's execution id, so the resume boundaries dedupe on it; a 4xx resume admission refusal is the caller's. The tool failure line names the credential selected after normalization.
1 parent 3d4a54d commit 96ee093

6 files changed

Lines changed: 37 additions & 26 deletions

File tree

‎apps/sim/background/resume-execution.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ describe('executeResumeJob terminal errors', () => {
9494

9595
await expect(executeResumeJob(payload)).rejects.toBe(rawError)
9696

97-
expect(resumeExecutionLogger.error).toHaveBeenCalledWith('Background resume execution failed', {
98-
errorType: 'error',
99-
hasStack: true,
100-
})
97+
expect(resumeExecutionLogger.error).toHaveBeenCalledWith(
98+
'Background resume execution failed',
99+
expect.objectContaining({ errorType: 'error', hasStack: true })
100+
)
101101
const loggerPayload = JSON.stringify(resumeExecutionLogger.error.mock.calls)
102102
expect(loggerPayload).not.toContain(secret)
103103
expect(loggerPayload).not.toContain('__var_')

‎apps/sim/background/resume-execution.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,14 @@ export async function executeResumeJob(payload: ResumeExecutionPayload, signal?:
249249
executedAt: new Date().toISOString(),
250250
}
251251
} catch (error) {
252+
/** The resumed run executes under its parent's execution id, so dedupe on that one. */
252253
logFailureOnce(logger, 'Background resume execution failed', error, {
253-
metadata: () => projectResolvedSecretDiagnosticError(error, undefined, { workflowId }),
254-
executionId: resumeExecutionId,
254+
metadata: () => ({
255+
resumeExecutionId,
256+
workflowId,
257+
...projectResolvedSecretDiagnosticError(error, undefined),
258+
}),
259+
executionId: parentExecutionId,
255260
})
256261
throw error
257262
} finally {

‎apps/sim/executor/handlers/workflow/workflow-handler.ts‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,14 +1575,12 @@ export class WorkflowBlockHandler implements BlockHandler {
15751575

15761576
if (!success) {
15771577
const rootErrorMessage = childResult.error || 'Child workflow execution failed'
1578-
logger.warn(
1579-
`Child workflow ${childWorkflowName} failed`,
1580-
projectResolvedSecretDiagnosticError(rootErrorMessage, parent?.registry, {
1581-
executionId: parent?.executionId,
1582-
blockId: parent?.blockId,
1583-
childExecutionId: parent?.childExecutionId,
1584-
})
1585-
)
1578+
logger.warn(`Child workflow ${childWorkflowName} failed`, {
1579+
executionId: parent?.executionId,
1580+
blockId: parent?.blockId,
1581+
childExecutionId: parent?.childExecutionId,
1582+
...projectResolvedSecretDiagnosticError(rootErrorMessage, parent?.registry),
1583+
})
15861584
const chain = [childWorkflowName]
15871585
const childFailure = new ChildWorkflowError({
15881586
message: formatWorkflowChainMessage(chain, rootErrorMessage),

‎apps/sim/lib/workflows/executor/human-in-the-loop-manager.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -486,10 +486,10 @@ describe('resume failure diagnostic projection', () => {
486486
})
487487
).rejects.toBe(rawError)
488488

489-
expect(humanInTheLoopLogger.error).toHaveBeenCalledWith('Resume execution failed', {
490-
errorType: 'error',
491-
hasStack: true,
492-
})
489+
expect(humanInTheLoopLogger.error).toHaveBeenCalledWith(
490+
'Resume execution failed',
491+
expect.objectContaining({ errorType: 'error', hasStack: true })
492+
)
493493
const loggerPayload = JSON.stringify(humanInTheLoopLogger.error.mock.calls)
494494
expect(loggerPayload).not.toContain(secret)
495495
expect(loggerPayload).not.toContain('__var_')

‎apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type { Edge } from '@xyflow/react'
88
import { and, asc, desc, eq, inArray, lt, type SQL, sql } from 'drizzle-orm'
99
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
1010
import { assertBillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution'
11-
import { logFailureOnce } from '@/lib/core/errors/failure-log'
11+
import { logFailureOnce, markFailureKind } from '@/lib/core/errors/failure-log'
1212
import {
1313
createTimeoutAbortController,
1414
getAsyncExecutionTimeoutForBillingAttribution,
@@ -1055,13 +1055,18 @@ export class PauseResumeManager {
10551055
)
10561056
})
10571057
}
1058+
/** A refusal to admit the resume (already resumed, not paused) is the caller's, not Sim's. */
1059+
if (error instanceof ResumeAdmissionError && error.statusCode < 500) {
1060+
markFailureKind(error, 'user')
1061+
}
1062+
/** The resumed run executes under its parent's execution id, so dedupe on that one. */
10581063
logFailureOnce(logger, 'Resume execution failed', error, {
1059-
metadata: () =>
1060-
projectResolvedSecretDiagnosticError(error, undefined, {
1061-
parentExecutionId: pausedExecution.executionId,
1062-
contextId,
1063-
}),
1064-
executionId: resumeExecutionId,
1064+
metadata: () => ({
1065+
resumeExecutionId,
1066+
contextId,
1067+
...projectResolvedSecretDiagnosticError(error, undefined),
1068+
}),
1069+
executionId: pausedExecution.executionId,
10651070
})
10661071
if (!(error instanceof ResumeAdmissionError)) {
10671072
await PauseResumeManager.processQueuedResumes(

‎apps/sim/tools/index.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1710,6 +1710,8 @@ async function executeToolImplementation(
17101710

17111711
// Hoisted so the outer catch can attribute a thrown failure to the chosen key.
17121712
let hostedKeyForMetrics: { provider: string; tool: string; key: string } | undefined
1713+
/** Hoisted so the outer catch names the account selected after normalization. */
1714+
let selectedCredentialId: string | undefined
17131715
let completePendingSecretActivation: (() => void) | undefined
17141716

17151717
try {
@@ -1890,6 +1892,7 @@ async function executeToolImplementation(
18901892
const workflowId = scope.workflowId
18911893
const userId = scope.userId
18921894
const credentialId = contextParams.credential as string
1895+
selectedCredentialId = credentialId
18931896
const toolLabel = tool?.name || toolId
18941897
const impersonateEmail = contextParams.impersonateUserEmail as string | undefined
18951898

@@ -2321,7 +2324,7 @@ async function executeToolImplementation(
23212324
workflowId: executionContext?.workflowId ?? undefined,
23222325
executionId: executionContext?.executionId,
23232326
blockId: typeof toolContext.blockId === 'string' ? toolContext.blockId : undefined,
2324-
credentialId: typeof params.credential === 'string' ? params.credential : undefined,
2327+
credentialId: selectedCredentialId,
23252328
...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}),
23262329
...projectToolLogMetadata(
23272330
{

0 commit comments

Comments
 (0)