Skip to content

Commit 4e66f3b

Browse files
committed
refactor(logs): build failure log metadata lazily and attribute author failures by type
logFailureOnce takes its metadata as a thunk so a skipped boundary does no secret projection, adds the execution id itself, and returns the attribution it logged with. UserFailure attributes author-caused failures by type (missing required fields, boundary-safe custom block refusals, no start block) wherever they surface, replacing a try/catch at one serializer caller. One inheritFailureMarks carries both marks across a boundary that drops cause, the hosted-key check reuses classifyHostedKeyFailure, the Pi backends share one toolResultError, and the sandbox and Function route no longer log author code failures the tool boundary already logs.
1 parent 54949cb commit 4e66f3b

24 files changed

Lines changed: 262 additions & 285 deletions

File tree

‎apps/sim/app/api/workflows/[id]/execute/route.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,13 +1616,11 @@ async function handleExecutePost(
16161616
return payloadTooLargeResponse()
16171617
}
16181618

1619-
logFailureOnce(
1620-
reqLogger,
1621-
'Non-SSE execution failed',
1622-
error,
1623-
loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }),
1624-
executionId
1625-
)
1619+
logFailureOnce(reqLogger, 'Non-SSE execution failed', error, {
1620+
metadata: () =>
1621+
loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }),
1622+
executionId,
1623+
})
16261624

16271625
const executionResult = hasExecutionResult(error) ? error.executionResult : undefined
16281626
const status = executionTimedOut ? 408 : getExecutionErrorStatus(error)
@@ -2424,13 +2422,10 @@ async function handleExecutePost(
24242422
? getTimeoutErrorMessage(timeoutController.timeoutMs)
24252423
: getErrorMessage(error, 'Unknown error')
24262424

2427-
logFailureOnce(
2428-
reqLogger,
2429-
'SSE execution failed',
2430-
error,
2431-
loggingSession.projectDiagnosticError(error, { isTimeout }),
2432-
executionId
2433-
)
2425+
logFailureOnce(reqLogger, 'SSE execution failed', error, {
2426+
metadata: () => loggingSession.projectDiagnosticError(error, { isTimeout }),
2427+
executionId,
2428+
})
24342429

24352430
const executionResult = hasExecutionResult(error) ? error.executionResult : undefined
24362431
let compactErrorLogs: BlockLog[] | undefined

‎apps/sim/background/async-preprocessing-correlation.test.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -488,9 +488,7 @@ describe('async preprocessing correlation threading', () => {
488488
}),
489489
})
490490
)
491-
expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError, {
492-
executionId: 'execution-fault',
493-
})
491+
expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError)
494492
expect(workflowExecutionLogger.error).toHaveBeenCalledWith(
495493
'[request-fault] Workflow execution failed: workflow-1',
496494
{ executionId: 'execution-fault', error: projectedError, failureKind: 'internal' }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,12 @@ describe('executeWebhookJob fault vs error handling', () => {
482482
expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalled()
483483
expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError, {
484484
workflowId: 'workflow-1',
485-
executionId: 'execution-1',
486485
provider: 'gmail',
487486
})
488487
expect(webhookExecutionLogger.error).toHaveBeenCalledWith(
489488
'[request-1] Webhook execution failed',
490489
{
490+
executionId: 'execution-1',
491491
workflowId: 'workflow-1',
492492
provider: 'gmail',
493493
error: projectedError,

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,17 +1268,14 @@ async function executeWebhookJobInternal(
12681268
throw new RetryableSetupError(errorMessage, { cause: retryableSetupCause })
12691269
}
12701270

1271-
logFailureOnce(
1272-
logger,
1273-
`[${requestId}] Webhook execution failed`,
1274-
error,
1275-
loggingSession.projectDiagnosticError(error, {
1276-
workflowId: payload.workflowId,
1277-
executionId,
1278-
provider: payload.provider,
1279-
}),
1280-
executionId
1281-
)
1271+
logFailureOnce(logger, `[${requestId}] Webhook execution failed`, error, {
1272+
metadata: () =>
1273+
loggingSession.projectDiagnosticError(error, {
1274+
workflowId: payload.workflowId,
1275+
provider: payload.provider,
1276+
}),
1277+
executionId,
1278+
})
12821279

12831280
// The finalized flag is set inside a fire-and-forget post-execution promise; await it so the
12841281
// signal is reliable and the failure is fully persisted before we decide fault vs error.

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,10 @@ export async function executeWorkflowJob(
307307
metadata: payload.metadata,
308308
}
309309
} catch (error: unknown) {
310-
logFailureOnce(
311-
logger,
312-
`[${requestId}] Workflow execution failed: ${workflowId}`,
313-
error,
314-
loggingSession.projectDiagnosticError(error, { executionId }),
315-
executionId
316-
)
310+
logFailureOnce(logger, `[${requestId}] Workflow execution failed: ${workflowId}`, error, {
311+
metadata: () => loggingSession.projectDiagnosticError(error),
312+
executionId,
313+
})
317314

318315
if (error instanceof ExecutionTimeoutError) throw error
319316

‎apps/sim/executor/errors/boundary.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { markFailureKind } from '@/lib/core/errors/failure-log'
1+
import { UserFailure } from '@/lib/core/errors/user-failure'
22

33
/**
44
* Machine-readable class of a custom-block failure. Every member describes a
@@ -41,14 +41,13 @@ export interface CustomBlockFailure {
4141
* replaces the older convention of throwing *before* the `try` block to dodge
4242
* the catch's sanitizer, where redaction depended on lexical position.
4343
*/
44-
export class BoundarySafeError extends Error {
44+
export class BoundarySafeError extends UserFailure {
4545
readonly errorType: CustomBlockErrorType
4646

4747
constructor(options: { message: string; errorType: CustomBlockErrorType }) {
4848
super(options.message)
4949
this.name = 'BoundarySafeError'
5050
this.errorType = options.errorType
51-
markFailureKind(this, 'user')
5251
}
5352
}
5453

‎apps/sim/executor/execution/block-executor.ts‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -789,23 +789,31 @@ export class BlockExecutor {
789789
}
790790
}
791791

792-
const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry
793-
? ctx.errorResolvedSecretTraceRegistry
794-
: inputDisplayRegistry?.forkForToolCall()
795-
if (
796-
!ctx.errorResolvedSecretTraceRegistry &&
797-
diagnosticRegistry &&
798-
ctx.resolvedSecretTraceRegistry &&
799-
ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry
800-
) {
801-
diagnosticRegistry.mergeToolCallRegistry(ctx.resolvedSecretTraceRegistry)
792+
/** Projected only when logged: a tool failure arrives already logged and skips it. */
793+
let errorDiagnostic: Record<string, unknown> | undefined
794+
const getErrorDiagnostic = () => {
795+
if (errorDiagnostic) return errorDiagnostic
796+
if (isDatabaseError) {
797+
errorDiagnostic = { cause: describeError(error) }
798+
return errorDiagnostic
799+
}
800+
const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry
801+
? ctx.errorResolvedSecretTraceRegistry
802+
: inputDisplayRegistry?.forkForToolCall()
803+
if (
804+
!ctx.errorResolvedSecretTraceRegistry &&
805+
diagnosticRegistry &&
806+
ctx.resolvedSecretTraceRegistry &&
807+
ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry
808+
) {
809+
diagnosticRegistry.mergeToolCallRegistry(ctx.resolvedSecretTraceRegistry)
810+
}
811+
errorDiagnostic = projectResolvedSecretDiagnosticError(
812+
error,
813+
diagnosticRegistry ?? ctx.resolvedSecretTraceRegistry
814+
)
815+
return errorDiagnostic
802816
}
803-
const errorDiagnostic = isDatabaseError
804-
? { cause: describeError(error) }
805-
: projectResolvedSecretDiagnosticError(
806-
error,
807-
diagnosticRegistry ?? ctx.resolvedSecretTraceRegistry
808-
)
809817

810818
/** A user Stop or the run's own time limit aborted this block; neither is a Sim fault. */
811819
if (isAbort && ctx.abortSignal?.aborted) markFailureKind(error, 'user')
@@ -814,11 +822,13 @@ export class BlockExecutor {
814822
phase === 'input_resolution' ? 'Failed to resolve block inputs' : 'Block execution failed',
815823
error,
816824
{
817-
blockId: node.id,
818-
blockType: block.metadata?.id,
819-
executionId: ctx.executionId,
820-
workflowId: ctx.workflowId,
821-
...errorDiagnostic,
825+
metadata: () => ({
826+
blockId: node.id,
827+
blockType: block.metadata?.id,
828+
executionId: ctx.executionId,
829+
workflowId: ctx.workflowId,
830+
...getErrorDiagnostic(),
831+
}),
822832
}
823833
)
824834

@@ -857,7 +867,7 @@ export class BlockExecutor {
857867
}
858868
this.execLogger.info('Block has error port - returning error output instead of throwing', {
859869
blockId: node.id,
860-
...errorDiagnostic,
870+
...getErrorDiagnostic(),
861871
})
862872
return errorOutput
863873
}

‎apps/sim/executor/execution/engine.ts‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,11 @@ export class ExecutionEngine {
197197
this.finalizeIncompleteLogs()
198198

199199
const errorMessage = normalizeError(error)
200-
logFailureOnce(
201-
this.execLogger,
202-
'Execution failed',
203-
error,
204-
projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry),
205-
this.context.executionId
206-
)
200+
logFailureOnce(this.execLogger, 'Execution failed', error, {
201+
metadata: () =>
202+
projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry),
203+
executionId: this.context.executionId,
204+
})
207205

208206
const executionResult: ExecutionResult = {
209207
success: false,
@@ -485,16 +483,13 @@ export class ExecutionEngine {
485483
* Block failures were logged by the block executor. This catches a completion-handling
486484
* fault, which only this frame sees when a concurrent failure already won `executionError`.
487485
*/
488-
logFailureOnce(
489-
this.execLogger,
490-
'Node execution failed',
491-
error,
492-
{
493-
nodeId,
494-
...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry),
495-
},
496-
this.context.executionId
497-
)
486+
logFailureOnce(this.execLogger, 'Node execution failed', error, {
487+
metadata: () =>
488+
projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry, {
489+
nodeId,
490+
}),
491+
executionId: this.context.executionId,
492+
})
498493
throw error
499494
}
500495
}

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { isPlainRecord, omit } from '@sim/utils/object'
44
import { truncate } from '@sim/utils/string'
5-
import { isProviderKeyRejection, markFailureKind } from '@/lib/core/errors/failure-log'
5+
import { markFailureKind } from '@/lib/core/errors/failure-log'
66
import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records'
77
import {
88
projectModelSchemaAnnotations,
@@ -327,6 +327,12 @@ function describeProviderTransportFailure(error: Error): string | null {
327327
return null
328328
}
329329

330+
/** A provider refusing the key it was given. */
331+
function isProviderKeyRejection(error: unknown): boolean {
332+
const status = (error as { status?: unknown } | null)?.status
333+
return status === 401 || status === 402 || status === 403
334+
}
335+
330336
/**
331337
* Handler for Agent blocks that process LLM requests with optional tools.
332338
*/
@@ -3042,8 +3048,10 @@ export class AgentBlockHandler implements BlockHandler {
30423048

30433049
return this.processProviderResponse(response, block, responseFormat, ctx)
30443050
} catch (error) {
3045-
const errorRegistry = this.createErrorRegistry(providerErrorRegistry, modelRuntimeRegistry)
3046-
ctx.errorResolvedSecretTraceRegistry = errorRegistry
3051+
ctx.errorResolvedSecretTraceRegistry = this.createErrorRegistry(
3052+
providerErrorRegistry,
3053+
modelRuntimeRegistry
3054+
)
30473055
try {
30483056
this.handleExecutionError(error)
30493057
} finally {
@@ -3076,7 +3084,7 @@ export class AgentBlockHandler implements BlockHandler {
30763084
throw markFailureKind(new Error(transportFailure), 'third_party_server')
30773085
}
30783086
/** The handler cannot tell a hosted provider key (Sim's) from the author's own. */
3079-
if (isProviderKeyRejection((error as { status?: unknown } | null)?.status)) {
3087+
if (isProviderKeyRejection(error)) {
30803088
markFailureKind(error, 'internal')
30813089
}
30823090
}

‎apps/sim/executor/handlers/pi/cloud/authoring/backend.ts‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import { createLogger } from '@sim/logger'
2222
import { generateShortId } from '@sim/utils/id'
2323
import { isRecordLike } from '@sim/utils/object'
2424
import { truncate } from '@sim/utils/string'
25-
import { adoptToolFailure } from '@/lib/core/errors/failure-log'
2625
import { getMaxExecutionTimeout, getRemainingExecutionMs } from '@/lib/core/execution-limits'
2726
import { withPiSandbox } from '@/lib/execution/remote-sandbox'
2827
import {
@@ -58,6 +57,7 @@ import {
5857
raceAbort,
5958
resolvePiTimeoutMs,
6059
scrubGitSecrets,
60+
toolResultError,
6161
} from '@/executor/handlers/pi/cloud/shared'
6262
import type {
6363
PiBackendRun,
@@ -186,10 +186,7 @@ async function openPullRequest(
186186
)
187187

188188
if (!result.success) {
189-
throw adoptToolFailure(
190-
new Error(`PR creation failed for branch ${branch}: ${result.error ?? 'unknown error'}`),
191-
result
192-
)
189+
throw toolResultError(`PR creation failed for branch ${branch}`, result)
193190
}
194191

195192
if (!isRecordLike(result.output)) {
@@ -225,12 +222,7 @@ async function repositoryDefaultBranch(
225222
{ signal }
226223
)
227224
if (!result.success) {
228-
throw adoptToolFailure(
229-
new Error(
230-
`Failed to determine the repository default branch: ${result.error ?? 'unknown error'}`
231-
),
232-
result
233-
)
225+
throw toolResultError('Failed to determine the repository default branch', result)
234226
}
235227
if (!isRecordLike(result.output)) {
236228
throw new Error('GitHub repository response must be an object')
@@ -271,12 +263,7 @@ async function updatePullRequest(
271263
{ signal }
272264
)
273265
if (!result.success) {
274-
throw adoptToolFailure(
275-
new Error(
276-
`PR update failed for branch ${params.targetBranch}: ${result.error ?? 'unknown error'}`
277-
),
278-
result
279-
)
266+
throw toolResultError(`PR update failed for branch ${params.targetBranch}`, result)
280267
}
281268
}
282269

0 commit comments

Comments
 (0)