From 7fd6fea20525b8135cc6798af943321ccd6e6c9e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 16:45:28 -0700 Subject: [PATCH 01/10] improvement(logs): log each execution failure once at its owning boundary A single external tool failure was logged at ERROR up to eight times as it propagated from the tool layer through the block executor, the engine, execution-core, and the trigger surfaces. Each boundary now calls logFailureOnce, which skips a failure an inner boundary already logged and sets severity by attribution: the author's input, configuration, or code at info, a third-party 4xx or 5xx at warn, and Sim's own faults (database, retryable setup, hosted-key rejection, anything unattributed) at error. The logged mark and attribution live in WeakMap/WeakSet side tables keyed by the thrown value, walked through the cause chain, and carried on the flattened tool failure's output so they survive a handler rebuilding a failed tool result as a new error. Run logs and trace spans are unchanged. --- .../app/api/workflows/[id]/execute/route.ts | 9 +- apps/sim/background/webhook-execution.ts | 6 +- apps/sim/background/workflow-execution.ts | 5 +- apps/sim/executor/errors/boundary.ts | 3 + apps/sim/executor/execution/block-executor.ts | 14 +- apps/sim/executor/execution/engine.ts | 11 +- .../executor/execution/failure-trace.test.ts | 20 ++ .../executor/handlers/agent/agent-handler.ts | 59 +++-- .../handlers/function/function-handler.ts | 2 + .../handlers/workflow/workflow-handler.ts | 89 +++++--- apps/sim/lib/core/errors/failure-log.test.ts | 81 +++++++ apps/sim/lib/core/errors/failure-log.ts | 159 ++++++++++++++ .../sim/lib/execution/remote-sandbox/index.ts | 10 +- .../lib/function-execution/execute-request.ts | 23 +- .../lib/workflows/executor/execute-service.ts | 3 +- .../lib/workflows/executor/execution-core.ts | 47 ++-- apps/sim/tools/index.test.ts | 103 ++++++++- apps/sim/tools/index.ts | 203 ++++++++---------- 18 files changed, 638 insertions(+), 209 deletions(-) create mode 100644 apps/sim/lib/core/errors/failure-log.test.ts create mode 100644 apps/sim/lib/core/errors/failure-log.ts diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index f81c4c43ade..6e0b2b0c04f 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -26,6 +26,7 @@ import { requireBillingAttributionHeader, } from '@/lib/billing/core/billing-attribution' import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { createTimeoutAbortController, getTimeoutErrorMessage, @@ -1615,8 +1616,10 @@ async function handleExecutePost( return payloadTooLargeResponse() } - reqLogger.error( + logFailureOnce( + reqLogger, 'Non-SSE execution failed', + error, loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }) ) @@ -2420,8 +2423,10 @@ async function handleExecutePost( ? getTimeoutErrorMessage(timeoutController.timeoutMs) : getErrorMessage(error, 'Unknown error') - reqLogger.error( + logFailureOnce( + reqLogger, 'SSE execution failed', + error, loggingSession.projectDiagnosticError(error, { isTimeout }) ) diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index 5e4f5a6476a..dbae0a098a5 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -25,6 +25,7 @@ import { import { getJobQueue } from '@/lib/core/async-jobs' import type { AsyncExecutionCorrelation } from '@/lib/core/async-jobs/types' import { env, envNumber } from '@/lib/core/config/env' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { describeRetryableInfrastructureError, isRetryableInfrastructureError, @@ -1267,10 +1268,13 @@ async function executeWebhookJobInternal( throw new RetryableSetupError(errorMessage, { cause: retryableSetupCause }) } - logger.error( + logFailureOnce( + logger, `[${requestId}] Webhook execution failed`, + error, loggingSession.projectDiagnosticError(error, { workflowId: payload.workflowId, + executionId, provider: payload.provider, }) ) diff --git a/apps/sim/background/workflow-execution.ts b/apps/sim/background/workflow-execution.ts index 33acc5ed146..04ee9ee3f07 100644 --- a/apps/sim/background/workflow-execution.ts +++ b/apps/sim/background/workflow-execution.ts @@ -16,6 +16,7 @@ import { type BillingAttributionSnapshot, } from '@/lib/billing/core/billing-attribution' import type { AsyncExecutionCorrelation } from '@/lib/core/async-jobs/types' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { capExecutionTimeoutMs, createTimeoutAbortController, @@ -306,8 +307,10 @@ export async function executeWorkflowJob( metadata: payload.metadata, } } catch (error: unknown) { - logger.error( + logFailureOnce( + logger, `[${requestId}] Workflow execution failed: ${workflowId}`, + error, loggingSession.projectDiagnosticError(error, { executionId }) ) diff --git a/apps/sim/executor/errors/boundary.ts b/apps/sim/executor/errors/boundary.ts index c25d45c1648..4371c174bce 100644 --- a/apps/sim/executor/errors/boundary.ts +++ b/apps/sim/executor/errors/boundary.ts @@ -1,3 +1,5 @@ +import { markFailureKind } from '@/lib/core/errors/failure-log' + /** * Machine-readable class of a custom-block failure. Every member describes a * fact the CONSUMER already knows or can act on — never the source workflow's @@ -46,6 +48,7 @@ export class BoundarySafeError extends Error { super(options.message) this.name = 'BoundarySafeError' this.errorType = options.errorType + markFailureKind(this, 'user') } } diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 4b1a84cd33b..1bdc2c61dd6 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -3,6 +3,7 @@ import { describeError, toError } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' import { isRecordLike, toRecord } from '@sim/utils/object' import { DrizzleQueryError } from 'drizzle-orm/errors' +import { logFailureOnce, markFailureKind, markFailureLogged } from '@/lib/core/errors/failure-log' import { isTimeoutAbortReason } from '@/lib/core/execution-limits/types' import { redactApiKeys } from '@/lib/core/security/redaction' import { normalizeStringArray } from '@/lib/core/utils/arrays' @@ -806,11 +807,17 @@ export class BlockExecutor { diagnosticRegistry ?? ctx.resolvedSecretTraceRegistry ) - this.execLogger.error( + /** A user Stop or the run's own time limit aborted this block; neither is a Sim fault. */ + if (isAbort && ctx.abortSignal?.aborted) markFailureKind(error, 'user') + logFailureOnce( + this.execLogger, phase === 'input_resolution' ? 'Failed to resolve block inputs' : 'Block execution failed', + error, { blockId: node.id, blockType: block.metadata?.id, + executionId: ctx.executionId, + workflowId: ctx.workflowId, ...errorDiagnostic, } ) @@ -861,7 +868,7 @@ export class BlockExecutor { ? error : new Error(errorMessage) - throw buildBlockExecutionError({ + const blockError = buildBlockExecutionError({ block, error: errorToThrow, context: ctx, @@ -870,6 +877,9 @@ export class BlockExecutor { executionTime: duration, }, }) + /** A thrown primitive has no `cause` link back to the value logged above. */ + markFailureLogged(blockError) + throw blockError } private hasErrorPortEdge(node: DAGNode): boolean { diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index 1dd151bdc2f..03d52001a22 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -1,5 +1,6 @@ import { createLogger, type Logger } from '@sim/logger' import { toError } from '@sim/utils/errors' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { combineExecutionAbortSignals } from '@/lib/core/execution-limits' import { subscribeToExecutionCancellation } from '@/lib/execution/cancellation' import { BlockType, EDGE } from '@/executor/constants' @@ -196,8 +197,10 @@ export class ExecutionEngine { this.finalizeIncompleteLogs() const errorMessage = normalizeError(error) - this.execLogger.error( + logFailureOnce( + this.execLogger, 'Execution failed', + error, projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry) ) @@ -477,7 +480,11 @@ export class ExecutionEngine { }) } } catch (error) { - this.execLogger.error('Node execution failed', { + /** + * Block failures were logged by the block executor. This catches a completion-handling + * fault, which only this frame sees when a concurrent failure already won `executionError`. + */ + logFailureOnce(this.execLogger, 'Node execution failed', error, { nodeId, ...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), }) diff --git a/apps/sim/executor/execution/failure-trace.test.ts b/apps/sim/executor/execution/failure-trace.test.ts index 929cb6707c4..d76ed6884a9 100644 --- a/apps/sim/executor/execution/failure-trace.test.ts +++ b/apps/sim/executor/execution/failure-trace.test.ts @@ -1,5 +1,6 @@ import { permissionCheckMock } from '@sim/testing/mocks/permission-check.mock' import { describe, expect, it, vi } from 'vitest' +import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' import { DAGExecutor } from '@/executor/execution/executor' import { hasExecutionResult } from '@/executor/utils/errors' @@ -70,4 +71,23 @@ describe('failed run trace', () => { expect(failing?.output?.error).toMatch(/"nope" doesn't exist on block "start"/) expect(thrown.executionResult.error).toBe(thrown.message) }) + + it('reaches the run boundary attributed to the author and already logged', async () => { + const executor = new DAGExecutor({ + workflow, + contextExtensions: { workspaceId: 'ws', executionId: 'exec', userId: 'u' }, + }) + + const thrown = await executor.execute('wf').then( + () => undefined, + (error: unknown) => error + ) + + /** + * The block executor logged it; the block wrap and the engine's rethrow must keep that + * visible so execution-core and the trigger surfaces do not log it again. + */ + expect(wasFailureLogged(thrown)).toBe(true) + expect(classifyFailure(thrown)).toBe('user') + }) }) diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 2fad9b54b0e..7a8ddfcc3ac 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { isPlainRecord, omit } from '@sim/utils/object' import { truncate } from '@sim/utils/string' +import { logFailureOnce, markFailureKind, markFailureLogged } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { projectModelSchemaAnnotations, @@ -310,6 +311,30 @@ function isTransportTimeout(error: unknown): boolean { /** * Handler for Agent blocks that process LLM requests with optional tools. */ +/** + * The user-facing message for a provider request that never got an answer, or null when the + * provider did answer. The original message is appended for timeouts rather than replaced: + * providers annotate it with the request phase they died in, which is the only thing + * separating a request that was never answered from one whose body stalled. + */ +function describeProviderTransportFailure(error: Error): string | null { + if (isTransportTimeout(error)) { + return `Provider request timed out - the API took too long to respond (${error.message})` + } + if (error.name === 'TypeError' && error.message.includes('fetch')) { + return 'Network error - unable to connect to provider API. Please check your internet connection.' + } + if (error.message.includes('ENOTFOUND') || error.message.includes('ECONNREFUSED')) { + return 'Unable to connect to server - DNS or connection issue' + } + return null +} + +function isAmbiguousProviderKeyRejection(error: unknown): boolean { + const status = (error as { status?: unknown } | null)?.status + return status === 401 || status === 402 || status === 403 +} + export class AgentBlockHandler implements BlockHandler { canHandle(block: SerializedBlock): boolean { return block.metadata?.id === BlockType.AGENT @@ -3059,9 +3084,18 @@ export class AgentBlockHandler implements BlockHandler { block: SerializedBlock ) { const executionTime = Date.now() - startTime + const transportFailure = error instanceof Error ? describeProviderTransportFailure(error) : null + if (transportFailure) { + markFailureKind(error, 'third_party_server') + } else if (isAmbiguousProviderKeyRejection(error)) { + /** The handler cannot tell a hosted provider key (ours) from the author's own. */ + markFailureKind(error, 'internal') + } - logger.error( + logFailureOnce( + logger, 'Error executing provider request', + error, projectAgentDiagnosticMetadata( ctx, { @@ -3081,25 +3115,10 @@ export class AgentBlockHandler implements BlockHandler { ) ) - if (!(error instanceof Error)) return - - /** - * The original message is appended rather than replaced: providers annotate it with - * the request phase they died in, which is the only thing separating a request that - * was never answered from one whose body stalled. - */ - if (isTransportTimeout(error)) { - throw new Error( - `Provider request timed out - the API took too long to respond (${error.message})` - ) - } - if (error.name === 'TypeError' && error.message.includes('fetch')) { - throw new Error( - 'Network error - unable to connect to provider API. Please check your internet connection.' - ) - } - if (error.message.includes('ENOTFOUND') || error.message.includes('ECONNREFUSED')) { - throw new Error('Unable to connect to server - DNS or connection issue') + if (transportFailure) { + const replacement = new Error(transportFailure) + markFailureLogged(replacement) + throw replacement } } diff --git a/apps/sim/executor/handlers/function/function-handler.ts b/apps/sim/executor/handlers/function/function-handler.ts index 2d66a019e1f..f9aab10c322 100644 --- a/apps/sim/executor/handlers/function/function-handler.ts +++ b/apps/sim/executor/handlers/function/function-handler.ts @@ -1,3 +1,4 @@ +import { markFailureLogged, wasFailureLogged } from '@/lib/core/errors/failure-log' import { getRemainingExecutionMs } from '@/lib/core/execution-limits' import { normalizeRecord, @@ -117,6 +118,7 @@ export class FunctionBlockHandler implements BlockHandler { ? new NonRetryableExecutionError(result.error || 'Function execution is indeterminate') : new Error(result.error || 'Function execution failed') attachTrustedExecutionCost(error, result.output?.cost) + if (wasFailureLogged(result.output)) markFailureLogged(error) throw error } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 321ecab58b7..fd3eec57f3a 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -4,6 +4,12 @@ import { generateId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import type { Variable, WorkflowState } from '@sim/workflow-types/workflow' import { resolveBillingAttribution } from '@/lib/billing/core/billing-attribution' +import { + classifyFailure, + logFailureOnce, + markFailureKind, + markFailureLogged, +} from '@/lib/core/errors/failure-log' import { getExecutionDeadlineAt } from '@/lib/core/execution-limits' import { withResourceOutboundScope } from '@/lib/core/network/resource-scope.server' import { asOrchestrationError } from '@/lib/core/orchestration/types' @@ -315,16 +321,19 @@ export class WorkflowBlockHandler implements BlockHandler { // for a custom block too — but `childWorkflowName` is still the source // workflow id at this point, so a custom block must carry its own block // name instead of leaking that id across the invocation boundary. - throw new ChildWorkflowError({ - message: depthError, - childWorkflowName: isCustomBlock - ? block.metadata?.name || 'Custom block' - : childWorkflowName, - childWorkflowInstanceId: instanceId, - ...(isCustomBlock - ? { consumerFacing: { errorType: 'depth_limit' as const, message: depthError } } - : {}), - }) + throw markFailureKind( + new ChildWorkflowError({ + message: depthError, + childWorkflowName: isCustomBlock + ? block.metadata?.name || 'Custom block' + : childWorkflowName, + childWorkflowInstanceId: instanceId, + ...(isCustomBlock + ? { consumerFacing: { errorType: 'depth_limit' as const, message: depthError } } + : {}), + }), + 'user' + ) } let childWorkflowSnapshotId: string | undefined @@ -1007,7 +1016,7 @@ export class WorkflowBlockHandler implements BlockHandler { return mappedResult } catch (error: unknown) { - logger.error('Error executing child workflow', { + logFailureOnce(logger, 'Error executing child workflow', error, { errorName: toError(error).name, hasWorkflowId: workflowId.length > 0, }) @@ -1025,7 +1034,16 @@ export class WorkflowBlockHandler implements BlockHandler { // `buildBoundaryFailure` preserves an already-attached `consumerFacing`, so the // depth guard keeps its own classification. if (isCustomBlock) { - throw this.buildBoundaryFailure(error, block, instanceId, childExecutionId, traceChildRuns) + const boundaryFailure = this.buildBoundaryFailure( + error, + block, + instanceId, + childExecutionId, + traceChildRuns + ) + /** The boundary severs `cause`, so the logged mark has to cross it explicitly. */ + markFailureLogged(boundaryFailure) + throw boundaryFailure } // An error this same invocation already attributed (e.g. the depth guard, or @@ -1172,14 +1190,19 @@ export class WorkflowBlockHandler implements BlockHandler { ChildWorkflowError.isChildWorkflowError(error) && error.consumerFacing ? error.consumerFacing : undefined + /** The boundary severs `cause`, so the failure's attribution has to cross it explicitly. */ + const failureKind = classifyFailure(error) if (alreadyClassified) { - return new ChildWorkflowError({ - message: alreadyClassified.message, - childWorkflowName: blockName, - childWorkflowInstanceId: instanceId, - consumerFacing: alreadyClassified, - ...traceHandle, - }) + return markFailureKind( + new ChildWorkflowError({ + message: alreadyClassified.message, + childWorkflowName: blockName, + childWorkflowInstanceId: instanceId, + consumerFacing: alreadyClassified, + ...traceHandle, + }), + failureKind + ) } const safe = isBoundarySafeError(error) ? error : undefined @@ -1193,16 +1216,19 @@ export class WorkflowBlockHandler implements BlockHandler { ? `Custom block execution failed (ref: ${ref})` : 'Custom block execution failed' - return new ChildWorkflowError({ - message, - childWorkflowName: blockName, - childWorkflowInstanceId: instanceId, - consumerFacing: { errorType, ...(ref ? { ref } : {}), message }, - // Carried even when `ref` is withheld (boundary-safe failures such as - // `cancelled` set no ref), so the parent's log always keeps the handle - // needed to join the child's own run at read time. - ...traceHandle, - }) + return markFailureKind( + new ChildWorkflowError({ + message, + childWorkflowName: blockName, + childWorkflowInstanceId: instanceId, + consumerFacing: { errorType, ...(ref ? { ref } : {}), message }, + // Carried even when `ref` is withheld (boundary-safe failures such as + // `cancelled` set no ref), so the parent's log always keeps the handle + // needed to join the child's own run at read time. + ...traceHandle, + }), + failureKind + ) } /** @@ -1552,7 +1578,7 @@ export class WorkflowBlockHandler implements BlockHandler { logger.warn(`Child workflow ${childWorkflowName} failed`) const rootErrorMessage = childResult.error || 'Child workflow execution failed' const chain = [childWorkflowName] - throw new ChildWorkflowError({ + const childFailure = new ChildWorkflowError({ message: formatWorkflowChainMessage(chain, rootErrorMessage), childWorkflowName, workflowChain: chain, @@ -1561,6 +1587,9 @@ export class WorkflowBlockHandler implements BlockHandler { childWorkflowSnapshotId, childWorkflowInstanceId: instanceId, }) + /** The child run logged its own failure; the warning above names it for this run. */ + markFailureLogged(childFailure) + throw childFailure } const output: BlockOutput = { diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts new file mode 100644 index 00000000000..f60ffd801bf --- /dev/null +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -0,0 +1,81 @@ +import { DrizzleQueryError } from 'drizzle-orm/errors' +import { describe, expect, it } from 'vitest' +import { + classifyFailure, + markDeliberateFailure, + markFailureKind, + markFailureLogged, + wasFailureLogged, +} from '@/lib/core/errors/failure-log' +import { RetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' +import { HostedKeyRateLimitedError, HostedKeyUnavailableError } from '@/tools/errors' + +describe('classifyFailure', () => { + it('keeps a database failure internal even beneath a user mark', () => { + const queryError = new DrizzleQueryError('select 1', [], new Error('connection reset')) + const wrapped = markFailureKind(new Error('Block failed', { cause: queryError }), 'user') + expect(classifyFailure(wrapped)).toBe('internal') + }) + + it('keeps a retryable setup failure internal even when its cause was the author’s', () => { + const cause = markFailureKind(new Error('missing field'), 'user') + expect(classifyFailure(new RetryableSetupError('setup', { cause }))).toBe('internal') + }) + + it.each([ + [ + 'Sim refusing for the workspace’s own rate bucket', + new HostedKeyRateLimitedError('slow'), + 'user', + ], + ['Sim having no hosted key to serve', new HostedKeyUnavailableError('none'), 'internal'], + ] as const)('reads a Sim HttpError status: %s', (_name, error, kind) => { + expect(classifyFailure(error)).toBe(kind) + }) + + it('lets an explicit mark on an outer link override an upstream status beneath it', () => { + const upstream = Object.assign(new Error('Unauthorized'), { status: 401 }) + const ours = markFailureKind(new Error('hosted key rejected', { cause: upstream }), 'internal') + expect(classifyFailure(ours)).toBe('internal') + expect(classifyFailure(upstream)).toBe('third_party_client') + }) + + it('leaves an unattributed failure internal', () => { + expect(classifyFailure(new Error('something broke'))).toBe('internal') + expect(classifyFailure('a thrown string')).toBe('internal') + }) + + it('terminates on a cyclic cause chain', () => { + const first = new Error('first') + const second = new Error('second', { cause: first }) + Object.assign(first, { cause: second }) + expect(classifyFailure(first)).toBe('internal') + expect(wasFailureLogged(first)).toBe(false) + }) +}) + +describe('markDeliberateFailure', () => { + it('attributes a plain Error but not a TypeError raised by a bug in the same code', () => { + expect(classifyFailure(markDeliberateFailure(new Error('channel_not_found'), 'user'))).toBe( + 'user' + ) + expect(classifyFailure(markDeliberateFailure(new TypeError('x is undefined'), 'user'))).toBe( + 'internal' + ) + }) +}) + +describe('wasFailureLogged', () => { + it('marks a frozen error, which a property write would throw on', () => { + const frozen = Object.freeze(new Error('frozen')) + markFailureLogged(frozen) + markFailureKind(frozen, 'user') + expect(wasFailureLogged(frozen)).toBe(true) + expect(classifyFailure(frozen)).toBe('user') + }) + + it('does not treat an unrelated error as logged', () => { + markFailureLogged(new Error('logged elsewhere')) + expect(wasFailureLogged(new Error('fresh'))).toBe(false) + }) +}) diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts new file mode 100644 index 00000000000..c55f68d1fc4 --- /dev/null +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -0,0 +1,159 @@ +import type { Logger } from '@sim/logger' +import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' +import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' +import { HttpError } from '@/lib/core/utils/http-error' + +/** + * Who a failure is attributable to, which decides how loudly the server logs it. The user + * sees every failure in full in the run log and trace spans regardless; this only governs + * server log severity. + * + * - `user`: the workflow author's input, configuration, or code. Expected, logged at info. + * - `third_party_client`: an external service rejected the request (4xx), usually because of + * the author's input or credentials. Logged at warn. + * - `third_party_server`: an external service failed (5xx). Not ours to fix, and the server that + * produced the status owns the fault (Sim's own internal routes log their own 5xx), so warn. + * - `internal`: Sim's own fault, or a cause nothing attributed. Always logged at error. + */ +export type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal' + +/** Mirrors the cause depth `describeError` and `readStatusCode` follow. */ +const MAX_CAUSE_DEPTH = 8 + +/** + * Side tables keyed by the thrown value, not properties on it: a thrown value may be frozen or + * sealed, and identity keying adds nothing to a serialized error. + */ +const failureKinds = new WeakMap() +const loggedFailures = new WeakSet() + +function isKeyable(value: unknown): value is object { + return (typeof value === 'object' || typeof value === 'function') && value !== null +} + +/** The thrown value and each `cause` beneath it, outermost first. */ +function causeChain(error: unknown): object[] { + const chain: object[] = [] + let current = error + while (isKeyable(current) && chain.length < MAX_CAUSE_DEPTH && !chain.includes(current)) { + chain.push(current) + current = (current as { cause?: unknown }).cause + } + return chain +} + +/** + * The flattened tool failure a block handler copies onto the error it throws. `executeTool` + * turns a thrown failure into a `{ success: false, output }` result, so the handler's new error + * shares only `output` with the failure the tool layer saw. + */ +function readToolFailureOutput(link: object): object | undefined { + const output = (link as { output?: unknown }).output + return isKeyable(output) ? output : undefined +} + +/** + * Upstream HTTP status of an external call: `status` on a transformed tool error, or on the + * tool failure output a block handler copied. Never Sim's own `statusCode`. + */ +function readUpstreamStatus(link: object): number | undefined { + const status = (link as { status?: unknown }).status + if (typeof status === 'number' && status >= 400 && status < 600) return status + const output = readToolFailureOutput(link) as { status?: unknown } | undefined + const outputStatus = output?.status + if (typeof outputStatus === 'number' && outputStatus >= 400 && outputStatus < 600) { + return outputStatus + } + return undefined +} + +/** Records who `error` is attributable to, overriding what its shape would imply. */ +export function markFailureKind(error: T, kind: FailureKind): T { + if (isKeyable(error)) failureKinds.set(error, kind) + return error +} + +/** + * Records `kind` only when `error` is a plain `Error` deliberately thrown with a message. A + * `TypeError`, `RangeError`, or other subclass raised by a bug in the same code stays + * unattributed, so it keeps logging at error. + */ +export function markDeliberateFailure(error: T, kind: FailureKind): T { + if (error instanceof Error && Object.getPrototypeOf(error) === Error.prototype) { + failureKinds.set(error, kind) + } + return error +} + +/** + * Attributes `error` from its cause chain. A database failure anywhere is always internal, + * then the outermost link with an explicit mark, a Sim `HttpError` status, or an upstream status + * decides. Anything unattributed is internal. + */ +export function classifyFailure(error: unknown): FailureKind { + if (findDatabaseQueryError(error)) return 'internal' + + for (const link of causeChain(error)) { + if (isRetryableSetupError(link)) return 'internal' + + const output = readToolFailureOutput(link) + const marked = failureKinds.get(link) ?? (output ? failureKinds.get(output) : undefined) + if (marked) return marked + + if (link instanceof HttpError) { + return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal' + } + + const upstreamStatus = readUpstreamStatus(link) + if (upstreamStatus !== undefined) { + return upstreamStatus >= 500 ? 'third_party_server' : 'third_party_client' + } + } + + return 'internal' +} + +/** + * Records that the boundary owning this failure has logged it. `subject` is the thrown value, + * or the output of a flattened tool failure. + */ +export function markFailureLogged(subject: unknown): void { + if (isKeyable(subject)) loggedFailures.add(subject) +} + +/** + * Whether any link of the cause chain, or the tool failure output a link carries, was already + * logged. Survives `buildBlockExecutionError` and `ChildWorkflowError` (both keep `cause`), the + * engine's identity-preserving rethrow, and a handler rebuilding a failed tool result as a new + * error that carries the result's `output`. + */ +export function wasFailureLogged(error: unknown): boolean { + return causeChain(error).some((link) => { + if (loggedFailures.has(link)) return true + const output = readToolFailureOutput(link) + return output !== undefined && loggedFailures.has(output) + }) +} + +const LOG_LEVEL_BY_KIND = { + user: 'info', + third_party_client: 'warn', + third_party_server: 'warn', + internal: 'error', +} as const satisfies Record + +/** + * Logs a failure at the severity its cause earns, unless a boundary closer to the cause already + * logged it, then marks it logged so every boundary it propagates through stays quiet. + */ +export function logFailureOnce( + logger: Logger, + message: string, + error: unknown, + metadata: Record = {} +): void { + if (wasFailureLogged(error)) return + const failureKind = classifyFailure(error) + logger[LOG_LEVEL_BY_KIND[failureKind]](message, { ...metadata, failureKind }) + markFailureLogged(error) +} diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 16b8fa7547b..30d3de73d96 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -958,9 +958,12 @@ async function executeInSandboxWithinBudget( if (execution.error) { const errorMessage = `${execution.error.name}: ${execution.error.value}` - logger.error('Sandbox execution failed', { + /** The author's code raised; only a provider-side failure is ours to look at. */ + const logFailure = execution.providerFailure ? logger.error : logger.info + logFailure.call(logger, 'Sandbox execution failed', { sandboxId, hasTraceback: Boolean(execution.error.traceback), + providerFailure: execution.providerFailure, }) const executionResult = { result: null, @@ -1153,9 +1156,12 @@ async function executeShellInSandboxWithinBudget( // back to stdout for the real command output before the generic message. const errorMessage = result.stderr || result.stdout || `Process exited with code ${result.exitCode}` - logger.error('Sandbox shell execution error', { + /** The author's command exited non-zero; only a provider-side failure is ours to look at. */ + const logFailure = result.providerFailure ? logger.error : logger.info + logFailure.call(logger, 'Sandbox shell execution error', { sandboxId, exitCode: result.exitCode, + providerFailure: result.providerFailure, }) const executionResult = { result: null, diff --git a/apps/sim/lib/function-execution/execute-request.ts b/apps/sim/lib/function-execution/execute-request.ts index 89348be909d..e26dbd6c91d 100644 --- a/apps/sim/lib/function-execution/execute-request.ts +++ b/apps/sim/lib/function-execution/execute-request.ts @@ -3272,12 +3272,6 @@ export async function executeFunctionRequest( } const isSystemError = isolatedResult.error.isSystemError === true - const logFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger) - logFn(`[${requestId}] Function execution failed in isolated-vm`, { - executionTime, - isSystemError, - hasStack: Boolean(isolatedResult.error.stack), - }) const ivmError = isolatedResult.error let adjustedLine = ivmError.line @@ -3307,8 +3301,12 @@ export async function executeFunctionRequest( errorDisplayCode ) - const detailLogFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger) - detailLogFn(`[${requestId}] Enhanced error details`, { + /** The author's code threw unless the isolate itself failed. */ + const logFailure = isSystemError ? logger.error : logger.info + logFailure.call(logger, `[${requestId}] Function execution failed in isolated-vm`, { + executionTime, + isSystemError, + hasStack: Boolean(ivmError.stack), line: enhancedError.line, column: enhancedError.column, }) @@ -3498,11 +3496,6 @@ export async function executeFunctionRequest( ) } - logger.error(`[${requestId}] Function execution failed`, { - executionTime, - hasStack: Boolean(error.stack), - }) - const errorDisplayCode = getErrorDisplayCode(sourceCodeForErrors, resolvedCode) const enhancedError = extractEnhancedError(error, userCodeStartLine, errorDisplayCode) const userFriendlyErrorMessage = scrubInternalIdentifiers( @@ -3510,7 +3503,9 @@ export async function executeFunctionRequest( compilerInternalIdentifiers ) - logger.error(`[${requestId}] Enhanced error details`, { + logger.error(`[${requestId}] Function execution failed`, { + executionTime, + hasStack: Boolean(error.stack), line: enhancedError.line, column: enhancedError.column, userCodeStartLine, diff --git a/apps/sim/lib/workflows/executor/execute-service.ts b/apps/sim/lib/workflows/executor/execute-service.ts index 7cb9707f635..9fa265b2990 100644 --- a/apps/sim/lib/workflows/executor/execute-service.ts +++ b/apps/sim/lib/workflows/executor/execute-service.ts @@ -6,6 +6,7 @@ import { generateId, isValidUuid } from '@sim/utils/id' import type { BlockState } from '@sim/workflow-types/workflow' import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { createTimeoutAbortController, getTimeoutErrorMessage } from '@/lib/core/execution-limits' import { SSE_HEADERS } from '@/lib/core/utils/sse' import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits' @@ -847,7 +848,7 @@ export async function executeWorkflowService( }) } - reqLogger.error(`Execution failed: ${errorMessage}`) + logFailureOnce(reqLogger, `Execution failed: ${errorMessage}`, error) let compactErrorOutput: NormalizedBlockOutput | undefined let compactErrorBlockOutputs: Record | null = null diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index 1fafc7cb337..d8333f66681 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -14,6 +14,11 @@ import type { Edge } from '@xyflow/react' import { eq } from 'drizzle-orm' import { z } from 'zod' import { type EffectivePiiRedaction, resolveEffectivePiiRedaction } from '@/lib/billing/retention' +import { + logFailureOnce, + markDeliberateFailure, + markFailureKind, +} from '@/lib/core/errors/failure-log' import { getExecutionDeadlineAt, getTimeoutErrorMessage, @@ -74,6 +79,7 @@ import { buildParallelSentinelEndId, } from '@/executor/utils/subflow-node-id-codec' import { Serializer } from '@/serializer' +import type { SerializedWorkflow } from '@/serializer/types' const logger = createLogger('ExecutionCore') @@ -874,9 +880,10 @@ async function executeWorkflowCoreImpl( const startBlock = TriggerUtils.findStartBlock(mergedStates, executionKind, false) if (!startBlock) { - const errorMsg = 'No start block found. Add a start block to this workflow.' - logger.error(`[${requestId}] ${errorMsg}`) - throw new Error(errorMsg) + throw markFailureKind( + new Error('No start block found. Add a start block to this workflow.'), + 'user' + ) } resolvedTriggerBlockId = startBlock.blockId @@ -888,13 +895,19 @@ async function executeWorkflowCoreImpl( } // Serialize workflow - const serializedWorkflow = new Serializer().serializeWorkflow( - mergedStates, - filteredEdges, - loops, - parallels, - true - ) + let serializedWorkflow: SerializedWorkflow + try { + serializedWorkflow = new Serializer().serializeWorkflow( + mergedStates, + filteredEdges, + loops, + parallels, + true + ) + } catch (serializeError) { + /** The serializer throws a plain `Error` for the author's configuration (missing required fields, unknown block type). */ + throw markDeliberateFailure(serializeError, 'user') + } const inputFileKeys = new Set() processedInput = resumeFromSnapshot || runFromBlock @@ -1347,13 +1360,15 @@ async function executeWorkflowCoreImpl( return result } catch (error: unknown) { const errorCause = describeErrorCause(error) - logger.error( + logFailureOnce( + logger, `[${requestId}] Execution failed:`, - projectResolvedSecretDiagnosticError( - error, - resolvedSecretTraceRegistry, - errorCause ? { cause: errorCause } : undefined - ) + error, + projectResolvedSecretDiagnosticError(error, resolvedSecretTraceRegistry, { + workflowId, + executionId, + ...(errorCause ? { cause: errorCause } : {}), + }) ) await waitForLifecycleCallbacks() diff --git a/apps/sim/tools/index.test.ts b/apps/sim/tools/index.test.ts index 3ef04348c3f..44c9c866b1f 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -419,7 +419,11 @@ vi.mock('@/tools/utils.server', async (importOriginal) => { }) import type { QueryClient } from '@tanstack/react-query' +import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' import * as getQueryClientModule from '@/app/_shell/providers/get-query-client' +import { ApiBlockHandler } from '@/executor/handlers/api/api-handler' +import { buildBlockExecutionError } from '@/executor/utils/errors' +import type { SerializedBlock } from '@/serializer/types' import { executeTool, postProcessToolOutput } from '@/tools' import { tools } from '@/tools/registry' import { createToolConfig, getTool } from '@/tools/utils' @@ -440,6 +444,15 @@ const mockAssertPermissionsAllowed = permissionCheckMockFns.mockAssertPermission const mockToolsLogger = getMockLogger('Tools') +/** Every level, so a secret-leak assertion holds wherever a failure's severity lands. */ +function allToolLogCalls() { + return [ + mockToolsLogger.error.mock.calls, + mockToolsLogger.warn.mock.calls, + mockToolsLogger.info.mock.calls, + ] +} + /** * Overlay the mock tools onto the REAL registry object instead of vi.mock: * under `isolate: false` shared consumers (`@/tools/utils`, `@/tools`) may be @@ -1610,8 +1623,8 @@ describe('executeTool Function', () => { error: untrustedDetail, }) expect(JSON.stringify(result)).not.toContain(untrustedHeader) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(untrustedDetail) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(untrustedHeader) + expect(JSON.stringify(allToolLogCalls())).not.toContain(untrustedDetail) + expect(JSON.stringify(allToolLogCalls())).not.toContain(untrustedHeader) expect(registry.isComplete()).toBe(true) } ) @@ -1793,8 +1806,8 @@ describe('executeTool Function', () => { expect(result.success).toBe(false) expect(result.error).toContain(secret) expect(result.output?.cost).toEqual(cost) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(secret) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(runtimeAlias) + expect(JSON.stringify(allToolLogCalls())).not.toContain(secret) + expect(JSON.stringify(allToolLogCalls())).not.toContain(runtimeAlias) expect(JSON.stringify(projectToolResultForCopilot(result, registry))).not.toContain(secret) expect(JSON.stringify(projectToolResultForCopilot(result, registry))).not.toContain( runtimeAlias @@ -1831,8 +1844,8 @@ describe('executeTool Function', () => { expect(result.success).toBe(false) expect(result.error).toContain(secret) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(secret) - expect(JSON.stringify(mockToolsLogger.error.mock.calls)).not.toContain(runtimeAlias) + expect(JSON.stringify(allToolLogCalls())).not.toContain(secret) + expect(JSON.stringify(allToolLogCalls())).not.toContain(runtimeAlias) }) it('does not lift an invalid sandbox cost from a Function error response', async () => { @@ -2625,6 +2638,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -2668,6 +2684,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -2707,6 +2726,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -5781,6 +5803,75 @@ describe('Centralized Error Handling', () => { // Should fall back to HTTP status text when both parsing methods fail expect(result.error).toBe('Internal Server Error') }) + + describe('failure attribution across the block boundary', () => { + const apiBlock: SerializedBlock = { + id: 'api-1', + position: { x: 0, y: 0 }, + config: { tool: 'http_request', params: {} }, + inputs: {}, + outputs: {}, + metadata: { id: 'api', name: 'Call' }, + enabled: true, + } + + /** Runs the real API block handler over the real `executeTool`, wrapped as the block executor wraps it. */ + async function failApiBlock(response: Response): Promise { + mockValidateUrlWithDNS.mockResolvedValue({ isValid: true, resolvedIP: '93.184.216.34' }) + mockSecureFetchWithPinnedIP.mockResolvedValue(toSecureFetchResponse(response)) + const thrown = await new ApiBlockHandler() + .execute(createToolExecutionContext(), apiBlock, { + url: 'https://example.com/test', + method: 'GET', + }) + .then( + () => undefined, + (error: unknown) => error + ) + expect(thrown).toBeInstanceOf(Error) + return buildBlockExecutionError({ block: apiBlock, error: thrown as Error }) + } + + function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }) + } + + it.each([ + [404, 'third_party_client'], + [503, 'third_party_server'], + ] as const)( + 'attributes an upstream %i to the third party, logged once by the tool layer', + async (status, kind) => { + const blockError = await failApiBlock(jsonResponse(status, { error: 'rejected' })) + expect(classifyFailure(blockError)).toBe(kind) + expect(wasFailureLogged(blockError)).toBe(true) + } + ) + + it.each([ + [ + 'a provider rejection a transform reports as a plain Error', + new Error('channel_not_found'), + 'third_party_client', + ], + ['a bug in the transform itself', new TypeError('data.channel is undefined'), 'internal'], + ] as const)('attributes %s', async (_name, transformError, kind) => { + const originalTransform = tools.http_request.transformResponse + tools.http_request.transformResponse = async () => { + throw transformError + } + try { + const blockError = await failApiBlock(jsonResponse(200, { ok: false })) + expect(classifyFailure(blockError)).toBe(kind) + expect(wasFailureLogged(blockError)).toBe(true) + } finally { + tools.http_request.transformResponse = originalTransform + } + }) + }) }) describe('MCP Tool Execution', () => { diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index a4db4d7895a..778cc614eec 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -17,6 +17,14 @@ import { } from '@/lib/billing/core/billing-attribution' import { isHosted } from '@/lib/core/config/env-flags' import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' +import { + classifyFailure, + logFailureOnce, + markDeliberateFailure, + markFailureKind, + markFailureLogged, +} from '@/lib/core/errors/failure-log' +import { isRetryableNetworkError } from '@/lib/core/errors/retryable-infrastructure' import { createTimeoutAbortController, DEFAULT_EXECUTION_TIMEOUT_MS, @@ -1196,6 +1204,11 @@ async function readToolResponseBody( } } +/** A provider refusing Sim's own hosted key is Sim's fault, not the workflow author's. */ +function isOwnHostedKeyRejection(status: unknown): boolean { + return status === 401 || status === 402 || status === 403 +} + /** * Create an Error instance from errorInfo and attach useful context * Uses the error extractor registry to find the best error message @@ -1838,12 +1851,16 @@ async function executeToolImplementation( } } - validateRequiredParametersAfterMerge(toolId, tool, contextParams) - if (!tool) { throw new Error(`Tool not found: ${toolId}`) } + try { + validateRequiredParametersAfterMerge(toolId, tool, contextParams) + } catch (validationError) { + throw markFailureKind(validationError, 'user') + } + await normalizeFileParams(tool, contextParams, scope, executionContext) normalizeCopilotCredentialParams(contextParams) enforceCopilotCredentialSelection(toolId, tool, contextParams, scope) @@ -2323,15 +2340,25 @@ async function executeToolImplementation( const normalizedError = toError(error) const databaseQueryError = findDatabaseQueryError(error) const databaseErrorCause = databaseQueryError ? describeError(error) : undefined - logger.error( - `[${requestId}] Error executing tool ${toolId}:`, - projectToolLogMetadata( + const upstreamStatus = (error as { status?: unknown } | null)?.status + if (hostedKeyForMetrics && isOwnHostedKeyRejection(upstreamStatus)) { + markFailureKind(error, 'internal') + } + const toolContext = params._context as Record | undefined + logFailureOnce(logger, `[${requestId}] Error executing tool ${toolId}:`, error, { + toolId, + workflowId: executionContext?.workflowId ?? undefined, + executionId: executionContext?.executionId, + blockId: typeof toolContext?.blockId === 'string' ? toolContext.blockId : undefined, + ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), + ...projectToolLogMetadata( { ...(databaseErrorCause ? { cause: databaseErrorCause } : { error: normalizedError.message, stack: error instanceof Error ? error.stack : undefined, + errorData: (error as { data?: unknown } | null)?.data, }), }, resolvedSecretTraceRegistry, @@ -2341,8 +2368,8 @@ async function executeToolImplementation( ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), }, structuralOnlyToolLogs - ) - ) + ), + }) if (hostedKeyForMetrics) { hostedKeyMetrics.recordFailed({ @@ -2420,12 +2447,16 @@ async function executeToolImplementation( const responseData = isRecordLike(rawResponseData) ? rawResponseData : undefined const functionSandboxCost = normalizedToolId === 'function_execute' ? readFunctionSandboxCost(responseData) : undefined + const failureOutput = { + ...errorDetails, + ...(functionSandboxCost ? { cost: functionSandboxCost } : {}), + } + /** A handler rebuilding this result as a thrown error carries `output`, and with it both marks. */ + markFailureLogged(failureOutput) + markFailureKind(failureOutput, classifyFailure(error)) return { success: false, - output: { - ...errorDetails, - ...(functionSandboxCost ? { cost: functionSandboxCost } : {}), - }, + output: failureOutput, error: errorMessage, ...(responseData?.retryable === false ? { retryable: false } : {}), // Sim's own status (hosted-key 429/503) survives the flattening from a @@ -3048,22 +3079,6 @@ async function executeToolRequest( throw new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE) } - logger.error( - `[${requestId}] External tool error for ${toolId}:`, - projectToolLogMetadata( - { - status: errorInfo.status, - errorData: errorInfo.data, - }, - resolvedSecretTraceRegistry, - { - status: errorInfo.status, - hasErrorData: errorInfo.data !== null, - }, - structuralOnlyToolLogs - ) - ) - throw errorToTransform } @@ -3096,86 +3111,61 @@ async function executeToolRequest( const { isError, errorInfo } = isErrorResponse(response, responseData) if (isError) { - const errorToTransform = createTransformedErrorFromErrorInfo(errorInfo, tool.errorExtractor) - - logger.error( - `[${requestId}] External tool error for ${toolId}:`, - projectToolLogMetadata( - { - status: errorInfo?.status, - errorData: errorInfo?.data, - }, - resolvedSecretTraceRegistry, - { - status: errorInfo?.status, - hasErrorData: errorInfo?.data !== null && errorInfo?.data !== undefined, - }, - structuralOnlyToolLogs - ) - ) - - throw errorToTransform + throw createTransformedErrorFromErrorInfo(errorInfo, tool.errorExtractor) } if (tool.transformResponse) { + // Forward the real body stream. Some transformResponse helpers (e.g. TikTok) + // read via readResponseTextWithLimit, which requires `.body` (or Content-Length) + // and otherwise mis-reports a false "response exceeded maximum size" error. + const mockResponse = { + ok: response.ok, + status: response.status, + statusText: response.statusText, + headers: response.headers, + url: fullUrl, + body: response.body, + json: () => response.json(), + text: () => response.text(), + arrayBuffer: () => response.arrayBuffer(), + blob: () => response.blob(), + } as Response + + /** + * A transform throws a plain `Error` to report what the provider rejected (Slack's + * `{ ok: false }` arrives as a 200). A `TypeError` from the transform itself is ours. + */ + let data: ToolResponse try { - // Forward the real body stream. Some transformResponse helpers (e.g. TikTok) - // read via readResponseTextWithLimit, which requires `.body` (or Content-Length) - // and otherwise mis-reports a false "response exceeded maximum size" error. - const mockResponse = { - ok: response.ok, - status: response.status, - statusText: response.statusText, - headers: response.headers, - url: fullUrl, - body: response.body, - json: () => response.json(), - text: () => response.text(), - arrayBuffer: () => response.arrayBuffer(), - blob: () => response.blob(), - } as Response - - const data = await tool.transformResponse(mockResponse, params, { signal }) - if (tool.request.responseType === 'binary' && data.success) { - if (!context) throw new Error('Binary file output requires trusted execution context') - const file = data.output?.file - if ( - !isRecordLike(file) || - !Buffer.isBuffer(file.data) || - typeof file.name !== 'string' || - typeof file.mimeType !== 'string' - ) { - throw new Error('Binary download tools must return a buffered file output') - } - return await storeInternalToolFileResult( - createInternalToolFileResult( - { buffer: file.data, name: file.name, mimeType: file.mimeType }, - (stored) => ({ ...data, output: { ...data.output, file: stored } }) - ), - context, - (body) => { - if (!isToolResponse(body)) throw new Error('Invalid binary tool response') - return body - }, - signal - ) - } - return data + data = await tool.transformResponse(mockResponse, params, { signal }) } catch (transformError) { - const normalizedError = toError(transformError) - logger.error( - `[${requestId}] Transform response error for ${toolId}:`, - projectToolLogMetadata( - { error: normalizedError.message }, - resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - }, - structuralOnlyToolLogs - ) + throw markDeliberateFailure(transformError, 'third_party_client') + } + if (tool.request.responseType === 'binary' && data.success) { + if (!context) throw new Error('Binary file output requires trusted execution context') + const file = data.output?.file + if ( + !isRecordLike(file) || + !Buffer.isBuffer(file.data) || + typeof file.name !== 'string' || + typeof file.mimeType !== 'string' + ) { + throw new Error('Binary download tools must return a buffered file output') + } + return await storeInternalToolFileResult( + createInternalToolFileResult( + { buffer: file.data, name: file.name, mimeType: file.mimeType }, + (stored) => ({ ...data, output: { ...data.output, file: stored } }) + ), + context, + (body) => { + if (!isToolResponse(body)) throw new Error('Invalid binary tool response') + return body + }, + signal ) - throw transformError } + return data } return { @@ -3194,18 +3184,7 @@ async function executeToolRequest( structuralOnlyToolLogs ) - const normalizedError = toError(error) - logger.error( - `[${requestId}] External request error for ${toolId}:`, - projectToolLogMetadata( - { error: normalizedError.message }, - resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - }, - structuralOnlyToolLogs - ) - ) + if (isRetryableNetworkError(error)) markFailureKind(error, 'third_party_server') throw error } From 5fba52cff4dcda12c05f474fea217fa501602dc1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 17:20:44 -0700 Subject: [PATCH 02/10] test(logs): pin the logged mark and attribution at the block executor Asserts the block executor's thrown error is already logged and attributed (database failure internal, user Stop user), switches the tool-boundary test to the central jsonResponse helper, and updates exact-argument log assertions for the added failureKind and executionId fields. --- .../async-preprocessing-correlation.test.ts | 2 +- apps/sim/background/webhook-execution.test.ts | 8 +++++++- apps/sim/executor/execution/block-executor.test.ts | 10 +++++++++- apps/sim/tools/index.test.ts | 12 +++--------- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/sim/background/async-preprocessing-correlation.test.ts b/apps/sim/background/async-preprocessing-correlation.test.ts index bdc774bc027..b4a26a04147 100644 --- a/apps/sim/background/async-preprocessing-correlation.test.ts +++ b/apps/sim/background/async-preprocessing-correlation.test.ts @@ -493,7 +493,7 @@ describe('async preprocessing correlation threading', () => { }) expect(workflowExecutionLogger.error).toHaveBeenCalledWith( '[request-fault] Workflow execution failed: workflow-1', - { executionId: 'execution-fault', error: projectedError } + { executionId: 'execution-fault', error: projectedError, failureKind: 'internal' } ) const loggerPayload = JSON.stringify(workflowExecutionLogger.error.mock.calls) expect(loggerPayload).not.toContain(secret) diff --git a/apps/sim/background/webhook-execution.test.ts b/apps/sim/background/webhook-execution.test.ts index f9f32ca541d..ba58994a3ca 100644 --- a/apps/sim/background/webhook-execution.test.ts +++ b/apps/sim/background/webhook-execution.test.ts @@ -482,11 +482,17 @@ describe('executeWebhookJob fault vs error handling', () => { expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalled() expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError, { workflowId: 'workflow-1', + executionId: 'execution-1', provider: 'gmail', }) expect(webhookExecutionLogger.error).toHaveBeenCalledWith( '[request-1] Webhook execution failed', - { workflowId: 'workflow-1', provider: 'gmail', error: projectedError } + { + workflowId: 'workflow-1', + provider: 'gmail', + error: projectedError, + failureKind: 'internal', + } ) const loggerPayload = JSON.stringify(webhookExecutionLogger.error.mock.calls) expect(loggerPayload).not.toContain(secret) diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index e9f7f9cf8fe..b0e0d850af0 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -5,6 +5,7 @@ import { storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' import { uploadsMock } from '@sim/testing/mocks/uploads.mock' import { DrizzleQueryError } from 'drizzle-orm/errors' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache' import { createLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest' import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref' @@ -496,6 +497,9 @@ describe('BlockExecutor', () => { expect.objectContaining({ cause: expect.objectContaining({ code: 'ECONNRESET' }) }) ) expect(JSON.stringify(logged)).not.toContain('owner-secret-id') + /** Logged here at error, so the engine and the run surfaces must see it as already logged. */ + expect(classifyFailure(thrown)).toBe('internal') + expect(wasFailureLogged(thrown)).toBe(true) }) it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => { @@ -614,11 +618,15 @@ describe('BlockExecutor', () => { const ctx = createContext(state) ctx.abortSignal = abortController.signal - await expect(executor.execute(ctx, createNode(block), block)).rejects.toThrow(/abort/i) + const thrown = await executor.execute(ctx, createNode(block), block).catch((error) => error) + expect(thrown).toBeInstanceOf(Error) + expect(thrown.message).toMatch(/abort/i) const output = state.getBlockOutput(block.id) expect(output?.error).toBeTruthy() expect(output).not.toEqual({ content: '' }) + /** A user Stop is not a Sim fault, so it must not page at error. */ + expect(classifyFailure(thrown)).toBe('user') }) it('keeps Sim Chat secret policy in runtime inputs and out of trace inputs', async () => { diff --git a/apps/sim/tools/index.test.ts b/apps/sim/tools/index.test.ts index 44c9c866b1f..636fba47f62 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -1,4 +1,5 @@ import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' +import { jsonResponse } from '@sim/testing/helpers/http' import { apiKeyByokMock, apiKeyByokMockFns } from '@sim/testing/mocks/api-key-byok.mock' import { authInternalMock, authInternalMockFns } from '@sim/testing/mocks/auth-internal.mock' import { billingUsageLogMock } from '@sim/testing/mocks/billing-usage-log.mock' @@ -5832,20 +5833,13 @@ describe('Centralized Error Handling', () => { return buildBlockExecutionError({ block: apiBlock, error: thrown as Error }) } - function jsonResponse(status: number, body: unknown): Response { - return new Response(JSON.stringify(body), { - status, - headers: { 'content-type': 'application/json' }, - }) - } - it.each([ [404, 'third_party_client'], [503, 'third_party_server'], ] as const)( 'attributes an upstream %i to the third party, logged once by the tool layer', async (status, kind) => { - const blockError = await failApiBlock(jsonResponse(status, { error: 'rejected' })) + const blockError = await failApiBlock(jsonResponse({ error: 'rejected' }, status)) expect(classifyFailure(blockError)).toBe(kind) expect(wasFailureLogged(blockError)).toBe(true) } @@ -5864,7 +5858,7 @@ describe('Centralized Error Handling', () => { throw transformError } try { - const blockError = await failApiBlock(jsonResponse(200, { ok: false })) + const blockError = await failApiBlock(jsonResponse({ ok: false })) expect(classifyFailure(blockError)).toBe(kind) expect(wasFailureLogged(blockError)).toBe(true) } finally { From 82120ab2e01d46af2d11e5ecfde372e9aad18c21 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:11:23 -0700 Subject: [PATCH 03/10] fix(logs): scope the logged mark to one propagation and attribute in-process failures Never mark a raw thrown value as logged: a persistent fault that rethrows one object (a rejected dynamic import, a memoized rejected promise) was logged once per process and then silenced everywhere. Only carriers created during the propagation (the flattened tool failure output, the block error, a handler's rebuilt error) carry the mark, and execution-level boundaries scope a raw value's mark to their execution id. Handlers that rebuild a failed tool result now call adoptToolFailure instead of relying on the result output being copied by reference. The child workflow and agent handlers no longer log failures the block executor logs with the block's and run's identity. In-process operations are attributed to Sim (4xx user, 5xx internal) rather than to a third party, only external HTTP failures log their response body, a missing required field is the only serializer refusal at info, and the size-limit and JSON-parse paths no longer log twice. --- .../app/api/workflows/[id]/execute/route.ts | 6 +- apps/sim/background/webhook-execution.ts | 3 +- apps/sim/background/workflow-execution.ts | 3 +- .../executor/execution/block-executor.test.ts | 69 ++++++++ apps/sim/executor/execution/engine.ts | 17 +- .../handlers/agent/agent-handler.test.ts | 70 -------- .../executor/handlers/agent/agent-handler.ts | 69 ++------ apps/sim/executor/handlers/api/api-handler.ts | 3 +- .../handlers/condition/condition-handler.ts | 8 +- .../handlers/function/function-handler.ts | 5 +- .../handlers/generic/generic-handler.ts | 2 + .../human-in-the-loop-handler.ts | 5 +- .../handlers/pi/cloud/authoring/backend.ts | 20 ++- .../handlers/pi/cloud/babysit/github.ts | 7 +- .../executor/handlers/pi/cloud/github-pr.ts | 13 +- .../handlers/pi/cloud/review/backend.ts | 8 +- .../workflow/workflow-handler.test.ts | 16 ++ .../handlers/workflow/workflow-handler.ts | 22 +-- apps/sim/lib/core/errors/failure-log.test.ts | 17 ++ apps/sim/lib/core/errors/failure-log.ts | 111 ++++++------- .../sim/lib/execution/remote-sandbox/index.ts | 6 +- .../lib/function-execution/execute-request.ts | 18 ++- .../tool-operations/execute-json-operation.ts | 8 +- .../lib/workflows/executor/execute-service.ts | 2 +- .../workflows/executor/execution-core.test.ts | 28 ++++ .../lib/workflows/executor/execution-core.ts | 16 +- apps/sim/serializer/errors.ts | 10 ++ apps/sim/serializer/index.ts | 5 +- apps/sim/tools/index.test.ts | 86 +++++++++- apps/sim/tools/index.ts | 152 ++++++------------ 30 files changed, 451 insertions(+), 354 deletions(-) create mode 100644 apps/sim/serializer/errors.ts diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 6e0b2b0c04f..59914e7d276 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -1620,7 +1620,8 @@ async function handleExecutePost( reqLogger, 'Non-SSE execution failed', error, - loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }) + loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }), + executionId ) const executionResult = hasExecutionResult(error) ? error.executionResult : undefined @@ -2427,7 +2428,8 @@ async function handleExecutePost( reqLogger, 'SSE execution failed', error, - loggingSession.projectDiagnosticError(error, { isTimeout }) + loggingSession.projectDiagnosticError(error, { isTimeout }), + executionId ) const executionResult = hasExecutionResult(error) ? error.executionResult : undefined diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index dbae0a098a5..5737307bf7b 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -1276,7 +1276,8 @@ async function executeWebhookJobInternal( workflowId: payload.workflowId, executionId, provider: payload.provider, - }) + }), + executionId ) // The finalized flag is set inside a fire-and-forget post-execution promise; await it so the diff --git a/apps/sim/background/workflow-execution.ts b/apps/sim/background/workflow-execution.ts index 04ee9ee3f07..259035d0bac 100644 --- a/apps/sim/background/workflow-execution.ts +++ b/apps/sim/background/workflow-execution.ts @@ -311,7 +311,8 @@ export async function executeWorkflowJob( logger, `[${requestId}] Workflow execution failed: ${workflowId}`, error, - loggingSession.projectDiagnosticError(error, { executionId }) + loggingSession.projectDiagnosticError(error, { executionId }), + executionId ) if (error instanceof ExecutionTimeoutError) throw error diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index b0e0d850af0..6c0f5d59bed 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -13,6 +13,7 @@ import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' import { validateBlockType } from '@/ee/access-control/utils/permission-check' import { BlockType, EDGE } from '@/executor/constants' import type { DAGNode } from '@/executor/dag/builder' +import { ChildWorkflowError } from '@/executor/errors/child-workflow-error' import { BlockExecutor } from '@/executor/execution/block-executor' import { ExecutionState } from '@/executor/execution/state' import type { BlockHandler, ExecutionContext } from '@/executor/types' @@ -576,6 +577,74 @@ describe('BlockExecutor', () => { expect(state.getBlockOutput(block.id)).toEqual(output) }) + function failBlockWith(thrown: unknown) { + const block = createBlock() + const workflow: SerializedWorkflow = { + version: '1', + blocks: [block], + connections: [], + loops: {}, + parallels: {}, + } + const state = new ExecutionState() + const resolver = new VariableResolver(workflow, {}, state) + const handler: BlockHandler = { + canHandle: () => true, + execute: async () => { + throw thrown + }, + } + const executor = new BlockExecutor([handler], resolver, {}, state) + const ctx = createContext(state) + ctx.resolvedSecretTraceRegistry = new ResolvedSecretTraceRegistry([]) + return executor.execute(ctx, createNode(block), block).catch((error) => error) + } + + function blockFailureLogsSince(loggerIndex: number) { + const loggers = new Set<{ error: { mock: { calls: unknown[][] } } }>( + blockExecutorBaseLogger.withMetadata.mock.results + .slice(loggerIndex) + .map((result: { value: { error: { mock: { calls: unknown[][] } } } }) => result.value) + ) + return [...loggers].flatMap((logger) => + logger.error.mock.calls.filter(([message]) => message === 'Block execution failed') + ) + } + + it('logs every block failure that rethrows one persistent object, not just the first', async () => { + /** A rejected dynamic `import()` or memoized rejected promise rethrows the same object. */ + const persistentFault = new Error('handler module failed to load') + const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length + + await failBlockWith(persistentFault) + await failBlockWith(persistentFault) + + expect(blockFailureLogsSince(loggerIndex)).toHaveLength(2) + }) + + it('logs an internal child workflow fault with the block and run identity', async () => { + const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length + + await failBlockWith( + new ChildWorkflowError({ + message: '"Child" failed: child load blew up', + childWorkflowName: 'Child', + cause: new TypeError('child load blew up'), + }) + ) + + const [[, logged]] = blockFailureLogsSince(loggerIndex) + expect(logged).toEqual( + expect.objectContaining({ + blockId: 'function-block-1', + executionId: 'execution-1', + workflowId: 'workflow-1', + error: '"Child" failed: child load blew up', + failureKind: 'internal', + }) + ) + }) + it('does not soft-succeed non-agent blocks on user AbortError', async () => { const block = createBlock() const workflow: SerializedWorkflow = { diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index 03d52001a22..2bc5f448a5a 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -201,7 +201,8 @@ export class ExecutionEngine { this.execLogger, 'Execution failed', error, - projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry) + projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), + this.context.executionId ) const executionResult: ExecutionResult = { @@ -484,10 +485,16 @@ export class ExecutionEngine { * Block failures were logged by the block executor. This catches a completion-handling * fault, which only this frame sees when a concurrent failure already won `executionError`. */ - logFailureOnce(this.execLogger, 'Node execution failed', error, { - nodeId, - ...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), - }) + logFailureOnce( + this.execLogger, + 'Node execution failed', + error, + { + nodeId, + ...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), + }, + this.context.executionId + ) throw error } } diff --git a/apps/sim/executor/handlers/agent/agent-handler.test.ts b/apps/sim/executor/handlers/agent/agent-handler.test.ts index 9a9781e7a26..1ec5a5d29c8 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.test.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.test.ts @@ -3224,14 +3224,6 @@ describe('AgentBlockHandler', () => { ctx: ExecutionContext, tools: Array> ) => Promise - handleExecutionError: ( - error: unknown, - startTime: number, - provider: string, - model: string, - ctx: ExecutionContext, - block: SerializedBlock - ) => void processStructuredResponse: ( result: Record, responseFormat: unknown, @@ -3239,68 +3231,6 @@ describe('AgentBlockHandler', () => { ) => Record } - it('projects provider errors and internal runtime identifiers before logging', () => { - const registry = new ResolvedSecretTraceRegistry([ - { - name: 'TOKEN', - plaintext: 'diagnostic-secret', - encryptedValue: 'encrypted-diagnostic-secret', - }, - ]) - registry.recordResolved('TOKEN', 'diagnostic-secret') - const ctx = { ...mockContext, resolvedSecretTraceRegistry: registry } - - privateHandler().handleExecutionError( - new Error('failed with diagnostic-secret __var_TOKEN __sim_runtime_test_1'), - Date.now(), - 'diagnostic-secret', - '__var_TOKEN', - ctx, - mockBlock - ) - - const serializedCalls = JSON.stringify(mockAgentLogger.error.mock.calls) - expect(serializedCalls).not.toContain('diagnostic-secret') - expect(serializedCalls).not.toContain('__var_') - expect(serializedCalls).not.toContain('__sim_') - expect(mockAgentLogger.error).toHaveBeenCalledWith( - 'Error executing provider request', - expect.objectContaining({ - provider: '{{TOKEN}}', - model: '{{TOKEN}}', - errorMessage: 'failed with {{TOKEN}} {{TOKEN}} [RUNTIME_BINDING]', - }) - ) - }) - - it('fails closed to structural provider diagnostics without a complete registry', () => { - const ctx = { ...mockContext, resolvedSecretTraceRegistry: undefined } - - privateHandler().handleExecutionError( - new Error('untracked-secret __var_TOKEN __sim_runtime_test_1'), - Date.now(), - 'untracked-secret', - '__var_TOKEN', - ctx, - mockBlock - ) - - const metadata = mockAgentLogger.error.mock.calls.at(-1)?.[1] - expect(metadata).toEqual( - expect.objectContaining({ - workflowId: mockContext.workflowId, - blockId: mockBlock.id, - errorType: 'error', - }) - ) - expect(metadata).not.toHaveProperty('provider') - expect(metadata).not.toHaveProperty('model') - expect(metadata).not.toHaveProperty('errorMessage') - expect(JSON.stringify(mockAgentLogger.error.mock.calls)).not.toContain('untracked-secret') - expect(JSON.stringify(mockAgentLogger.error.mock.calls)).not.toContain('__var_') - expect(JSON.stringify(mockAgentLogger.error.mock.calls)).not.toContain('__sim_') - }) - it('projects tool diagnostics without logging code or raw params', async () => { const registry = new ResolvedSecretTraceRegistry([ { diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 7a8ddfcc3ac..0ad3fbfdaad 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { isPlainRecord, omit } from '@sim/utils/object' import { truncate } from '@sim/utils/string' -import { logFailureOnce, markFailureKind, markFailureLogged } from '@/lib/core/errors/failure-log' +import { isProviderKeyRejection, markFailureKind } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { projectModelSchemaAnnotations, @@ -308,9 +308,6 @@ function isTransportTimeout(error: unknown): boolean { return false } -/** - * Handler for Agent blocks that process LLM requests with optional tools. - */ /** * The user-facing message for a provider request that never got an answer, or null when the * provider did answer. The original message is appended for timeouts rather than replaced: @@ -330,11 +327,9 @@ function describeProviderTransportFailure(error: Error): string | null { return null } -function isAmbiguousProviderKeyRejection(error: unknown): boolean { - const status = (error as { status?: unknown } | null)?.status - return status === 401 || status === 402 || status === 403 -} - +/** + * Handler for Agent blocks that process LLM requests with optional tools. + */ export class AgentBlockHandler implements BlockHandler { canHandle(block: SerializedBlock): boolean { return block.metadata?.id === BlockType.AGENT @@ -2964,7 +2959,6 @@ export class AgentBlockHandler implements BlockHandler { ): Promise { const providerId = providerRequest.provider const model = providerRequest.model - const providerStartTime = Date.now() try { let finalApiKey: string | undefined = providerRequest.apiKey @@ -3050,11 +3044,8 @@ export class AgentBlockHandler implements BlockHandler { } catch (error) { const errorRegistry = this.createErrorRegistry(providerErrorRegistry, modelRuntimeRegistry) ctx.errorResolvedSecretTraceRegistry = errorRegistry - const diagnosticCtx = errorRegistry - ? { ...ctx, resolvedSecretTraceRegistry: errorRegistry } - : ctx try { - this.handleExecutionError(error, providerStartTime, providerId, model, diagnosticCtx, block) + this.handleExecutionError(error) } finally { if (modelRuntimeRegistry) { ctx.resolvedSecretTraceRegistry = modelRuntimeRegistry.forkForPropagatedEntries() @@ -3075,50 +3066,18 @@ export class AgentBlockHandler implements BlockHandler { return errorRegistry } - private handleExecutionError( - error: any, - startTime: number, - provider: string, - model: string, - ctx: ExecutionContext, - block: SerializedBlock - ) { - const executionTime = Date.now() - startTime + /** + * Attributes a provider failure and rewrites a transport failure into a message the author can + * act on. The block executor owns the log line, with the block's and run's identity. + */ + private handleExecutionError(error: unknown) { const transportFailure = error instanceof Error ? describeProviderTransportFailure(error) : null if (transportFailure) { - markFailureKind(error, 'third_party_server') - } else if (isAmbiguousProviderKeyRejection(error)) { - /** The handler cannot tell a hosted provider key (ours) from the author's own. */ - markFailureKind(error, 'internal') + throw markFailureKind(new Error(transportFailure), 'third_party_server') } - - logFailureOnce( - logger, - 'Error executing provider request', - error, - projectAgentDiagnosticMetadata( - ctx, - { - executionTime, - provider, - model, - workflowId: ctx.workflowId, - blockId: block.id, - ...getErrorDiagnosticMetadata(error), - }, - { - executionTime, - workflowId: ctx.workflowId, - blockId: block.id, - ...getErrorDiagnosticFallback(error), - } - ) - ) - - if (transportFailure) { - const replacement = new Error(transportFailure) - markFailureLogged(replacement) - throw replacement + /** The handler cannot tell a hosted provider key (Sim's) from the author's own. */ + if (isProviderKeyRejection((error as { status?: unknown } | null)?.status)) { + markFailureKind(error, 'internal') } } diff --git a/apps/sim/executor/handlers/api/api-handler.ts b/apps/sim/executor/handlers/api/api-handler.ts index 348e29ccb93..87bf254930a 100644 --- a/apps/sim/executor/handlers/api/api-handler.ts +++ b/apps/sim/executor/handlers/api/api-handler.ts @@ -1,3 +1,4 @@ +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { BlockType, HTTP } from '@/executor/constants' import type { BlockHandler, ExecutionContext } from '@/executor/types' import type { SerializedBlock } from '@/serializer/types' @@ -124,7 +125,7 @@ export class ApiBlockHandler implements BlockHandler { timestamp: new Date().toISOString(), }) - throw error + throw adoptToolFailure(error, result) } return result.output diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index 44824f6c894..c841bc0208f 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { isNonRetryableExecutionError, @@ -293,9 +294,12 @@ async function evaluateSingleCondition( if (!result.success) { if (result.retryable === false) { - throw new NonRetryableExecutionError(result.error ?? 'Condition evaluation is indeterminate') + throw adoptToolFailure( + new NonRetryableExecutionError(result.error ?? 'Condition evaluation is indeterminate'), + result + ) } - throw new Error(result.error ?? 'Condition evaluation failed') + throw adoptToolFailure(new Error(result.error ?? 'Condition evaluation failed'), result) } return Boolean(result.output?.result) diff --git a/apps/sim/executor/handlers/function/function-handler.ts b/apps/sim/executor/handlers/function/function-handler.ts index f9aab10c322..94f7bab2ced 100644 --- a/apps/sim/executor/handlers/function/function-handler.ts +++ b/apps/sim/executor/handlers/function/function-handler.ts @@ -1,4 +1,4 @@ -import { markFailureLogged, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getRemainingExecutionMs } from '@/lib/core/execution-limits' import { normalizeRecord, @@ -118,8 +118,7 @@ export class FunctionBlockHandler implements BlockHandler { ? new NonRetryableExecutionError(result.error || 'Function execution is indeterminate') : new Error(result.error || 'Function execution failed') attachTrustedExecutionCost(error, result.output?.cost) - if (wasFailureLogged(result.output)) markFailureLogged(error) - throw error + throw adoptToolFailure(error, result) } mergeLargeValueKeys(ctx, result.largeValueKeys ?? []) diff --git a/apps/sim/executor/handlers/generic/generic-handler.ts b/apps/sim/executor/handlers/generic/generic-handler.ts index 112d9c2d42e..1cc72f004d9 100644 --- a/apps/sim/executor/handlers/generic/generic-handler.ts +++ b/apps/sim/executor/handlers/generic/generic-handler.ts @@ -2,6 +2,7 @@ import { isDeepStrictEqual } from 'node:util' import { createLogger } from '@sim/logger' import { toError } from '@sim/utils/errors' import { isPlainRecord } from '@sim/utils/object' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getBlock } from '@/blocks/index' import { isMcpTool } from '@/executor/constants' import type { BlockHandler, BlockNodeMetadata, ExecutionContext } from '@/executor/types' @@ -362,6 +363,7 @@ export class GenericBlockHandler implements BlockHandler { // error so `getExecutionErrorStatus` can still reach the API caller. ...(typeof result.statusCode === 'number' ? { statusCode: result.statusCode } : {}), }) + adoptToolFailure(error, result) throw error } diff --git a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts index 1ac7e5d5a8f..c929f82c1d8 100644 --- a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts +++ b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts @@ -560,11 +560,8 @@ export class HumanInTheLoopBlockHandler implements BlockHandler { const result = await executeTool(toolId, toolParams, { executionContext: ctx }) const durationMs = Date.now() - startTime + /** `executeTool` already logged the failure with its tool id and attribution. */ if (!result.success) { - logger.warn('Notification tool execution failed', { - toolId, - error: result.error, - }) return { toolId, title: toolConfig.title, diff --git a/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts index 8c0e16252d5..9f913a62ad9 100644 --- a/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts @@ -22,6 +22,7 @@ import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getMaxExecutionTimeout, getRemainingExecutionMs } from '@/lib/core/execution-limits' import { withPiSandbox } from '@/lib/execution/remote-sandbox' import { @@ -185,7 +186,10 @@ async function openPullRequest( ) if (!result.success) { - throw new Error(`PR creation failed for branch ${branch}: ${result.error ?? 'unknown error'}`) + throw adoptToolFailure( + new Error(`PR creation failed for branch ${branch}: ${result.error ?? 'unknown error'}`), + result + ) } if (!isRecordLike(result.output)) { @@ -221,8 +225,11 @@ async function repositoryDefaultBranch( { signal } ) if (!result.success) { - throw new Error( - `Failed to determine the repository default branch: ${result.error ?? 'unknown error'}` + throw adoptToolFailure( + new Error( + `Failed to determine the repository default branch: ${result.error ?? 'unknown error'}` + ), + result ) } if (!isRecordLike(result.output)) { @@ -264,8 +271,11 @@ async function updatePullRequest( { signal } ) if (!result.success) { - throw new Error( - `PR update failed for branch ${params.targetBranch}: ${result.error ?? 'unknown error'}` + throw adoptToolFailure( + new Error( + `PR update failed for branch ${params.targetBranch}: ${result.error ?? 'unknown error'}` + ), + result ) } } diff --git a/apps/sim/executor/handlers/pi/cloud/babysit/github.ts b/apps/sim/executor/handlers/pi/cloud/babysit/github.ts index 30556c8acaf..e44b17ec287 100644 --- a/apps/sim/executor/handlers/pi/cloud/babysit/github.ts +++ b/apps/sim/executor/handlers/pi/cloud/babysit/github.ts @@ -1,6 +1,7 @@ import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import type { BabysitRoundDecision } from '@/executor/handlers/pi/cloud/babysit/round' import { fetchPrSnapshot, @@ -288,7 +289,8 @@ export async function fetchBabysitThreads( }, { signal } ) - if (!result.success) throw toolFailure('Failed to fetch review threads', result.error) + if (!result.success) + throw adoptToolFailure(toolFailure('Failed to fetch review threads', result.error), result) const output = result.output if (!isRecordLike(output) || !Array.isArray(output.threads)) { throw new Error('Review thread response is incomplete') @@ -423,7 +425,8 @@ export async function fetchBabysitCheckState( }, { signal } ) - if (!result.success) throw toolFailure('Failed to fetch checks', result.error) + if (!result.success) + throw adoptToolFailure(toolFailure('Failed to fetch checks', result.error), result) const output = result.output if (!isRecordLike(output) || !Array.isArray(output.contexts)) { throw new Error('Check response is incomplete') diff --git a/apps/sim/executor/handlers/pi/cloud/github-pr.ts b/apps/sim/executor/handlers/pi/cloud/github-pr.ts index dc72e46aced..9c17a2a5a37 100644 --- a/apps/sim/executor/handlers/pi/cloud/github-pr.ts +++ b/apps/sim/executor/handlers/pi/cloud/github-pr.ts @@ -7,6 +7,7 @@ */ import { isRecordLike } from '@sim/utils/object' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { executeTool } from '@/tools' import { GITHUB_GRAPHQL_URL, githubGraphQlHeaders, readGraphQlData } from '@/tools/github/graphql' import { @@ -109,7 +110,10 @@ export async function fetchPrSnapshot( ) if (!result.success) { - throw new Error(`Failed to fetch PR #${params.pullNumber}: ${result.error ?? 'unknown error'}`) + throw adoptToolFailure( + new Error(`Failed to fetch PR #${params.pullNumber}: ${result.error ?? 'unknown error'}`), + result + ) } return parsePullRequestSnapshot(result.output) @@ -173,8 +177,11 @@ export async function findOpenPrForBranch( { signal } ) if (!result.success) { - throw new Error( - `Failed to find an open PR for branch ${params.branch}: ${result.error ?? 'unknown error'}` + throw adoptToolFailure( + new Error( + `Failed to find an open PR for branch ${params.branch}: ${result.error ?? 'unknown error'}` + ), + result ) } diff --git a/apps/sim/executor/handlers/pi/cloud/review/backend.ts b/apps/sim/executor/handlers/pi/cloud/review/backend.ts index f648700b33e..d0b587c94d2 100644 --- a/apps/sim/executor/handlers/pi/cloud/review/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/review/backend.ts @@ -11,6 +11,7 @@ import { join } from 'node:path' import { createLogger } from '@sim/logger' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { withPiSandbox } from '@/lib/execution/remote-sandbox' import { resolvePiRunLifetimeMs } from '@/lib/execution/remote-sandbox/pi-lifetime' import { @@ -186,8 +187,11 @@ async function submitReview( ) if (!result.success) { - throw new Error( - `Failed to submit review for PR #${params.pullNumber}: ${result.error ?? 'unknown error'}` + throw adoptToolFailure( + new Error( + `Failed to submit review for PR #${params.pullNumber}: ${result.error ?? 'unknown error'}` + ), + result ) } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 2629d5b79d6..9bf190252c6 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -18,6 +18,7 @@ import { import { permissionsMock } from '@sim/testing/mocks/permissions.mock' import { usersQueriesMock, usersQueriesMockFns } from '@sim/testing/mocks/users-queries.mock' import { afterAll, beforeAll, beforeEach, describe, expect, it, type Mock, vi } from 'vitest' +import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' import { createTimeoutAbortController, getExecutionDeadlineAt } from '@/lib/core/execution-limits' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getBlock } from '@/blocks/registry' @@ -370,6 +371,21 @@ describe('WorkflowBlockHandler', () => { ) }) + it('leaves an internal child load fault for the block executor to log with the run identity', async () => { + mockReadWorkflowDefinitionAsExecutor.mockRejectedValueOnce( + new TypeError('definition is undefined') + ) + + const thrown = await handler + .execute({ ...mockContext, executionId: 'parent-execution-id' }, mockBlock, inputs) + .catch((error: unknown) => error) + + expect(thrown).toBeInstanceOf(Error) + /** Logged here, the block executor would skip its log carrying block, run, and stack. */ + expect(wasFailureLogged(thrown)).toBe(false) + expect(classifyFailure(thrown)).toBe('internal') + }) + it("runs a non-custom child under the parent's env and redaction policy", async () => { const piiBlockOutputRedaction = { enabled: true, diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index fd3eec57f3a..d10f7f74478 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -6,9 +6,9 @@ import type { Variable, WorkflowState } from '@sim/workflow-types/workflow' import { resolveBillingAttribution } from '@/lib/billing/core/billing-attribution' import { classifyFailure, - logFailureOnce, markFailureKind, markFailureLogged, + wasFailureLogged, } from '@/lib/core/errors/failure-log' import { getExecutionDeadlineAt } from '@/lib/core/execution-limits' import { withResourceOutboundScope } from '@/lib/core/network/resource-scope.server' @@ -973,7 +973,8 @@ export class WorkflowBlockHandler implements BlockHandler { childWorkflowName, instanceId, childTraceSpans, - childWorkflowSnapshotId + childWorkflowSnapshotId, + { executionId: ctx.executionId, blockId: block.id } ) // Custom blocks expose only curated outputs — never the child workflow id, @@ -1016,11 +1017,6 @@ export class WorkflowBlockHandler implements BlockHandler { return mappedResult } catch (error: unknown) { - logFailureOnce(logger, 'Error executing child workflow', error, { - errorName: toError(error).name, - hasWorkflowId: workflowId.length > 0, - }) - // The child's own log row records the real failure in the source workspace, // so the publisher sees what the consumer deliberately cannot. if (childSession && childSessionStarted && !childSessionFinalized) { @@ -1041,8 +1037,8 @@ export class WorkflowBlockHandler implements BlockHandler { childExecutionId, traceChildRuns ) - /** The boundary severs `cause`, so the logged mark has to cross it explicitly. */ - markFailureLogged(boundaryFailure) + /** The boundary severs `cause`, so a logged mark has to cross it explicitly. */ + if (wasFailureLogged(error)) markFailureLogged(boundaryFailure) throw boundaryFailure } @@ -1569,13 +1565,17 @@ export class WorkflowBlockHandler implements BlockHandler { childWorkflowName: string, instanceId: string, childTraceSpans?: WorkflowTraceSpan[], - childWorkflowSnapshotId?: string + childWorkflowSnapshotId?: string, + parent?: { executionId?: string; blockId: string } ): BlockOutput { const success = childResult.success !== false const result = childResult.output || {} if (!success) { - logger.warn(`Child workflow ${childWorkflowName} failed`) + logger.warn(`Child workflow ${childWorkflowName} failed`, { + executionId: parent?.executionId, + blockId: parent?.blockId, + }) const rootErrorMessage = childResult.error || 'Child workflow execution failed' const chain = [childWorkflowName] const childFailure = new ChildWorkflowError({ diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index f60ffd801bf..5ac9f360734 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -1,7 +1,9 @@ +import { createLogger } from '@sim/logger' import { DrizzleQueryError } from 'drizzle-orm/errors' import { describe, expect, it } from 'vitest' import { classifyFailure, + logFailureOnce, markDeliberateFailure, markFailureKind, markFailureLogged, @@ -78,4 +80,19 @@ describe('wasFailureLogged', () => { markFailureLogged(new Error('logged elsewhere')) expect(wasFailureLogged(new Error('fresh'))).toBe(false) }) + + it('scopes a raw value logged at an execution boundary to that execution only', () => { + const persistentFault = new Error('module failed to load') + logFailureOnce( + createLogger('FailureLogTest'), + 'Execution failed', + persistentFault, + {}, + 'exec-1' + ) + + expect(wasFailureLogged(persistentFault, 'exec-1')).toBe(true) + expect(wasFailureLogged(persistentFault, 'exec-2')).toBe(false) + expect(wasFailureLogged(persistentFault)).toBe(false) + }) }) diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index c55f68d1fc4..73c8acaf922 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -11,8 +11,8 @@ import { HttpError } from '@/lib/core/utils/http-error' * - `user`: the workflow author's input, configuration, or code. Expected, logged at info. * - `third_party_client`: an external service rejected the request (4xx), usually because of * the author's input or credentials. Logged at warn. - * - `third_party_server`: an external service failed (5xx). Not ours to fix, and the server that - * produced the status owns the fault (Sim's own internal routes log their own 5xx), so warn. + * - `third_party_server`: an external service failed (5xx). Not Sim's to fix, so warn. Sim's own + * in-process operations are attributed explicitly at their boundary and never land here. * - `internal`: Sim's own fault, or a cause nothing attributed. Always logged at error. */ export type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal' @@ -21,11 +21,18 @@ export type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | const MAX_CAUSE_DEPTH = 8 /** - * Side tables keyed by the thrown value, not properties on it: a thrown value may be frozen or + * Side tables keyed by identity, not properties on the value: a thrown value may be frozen or * sealed, and identity keying adds nothing to a serialized error. + * + * `loggedCarriers` holds only values created during the propagation they describe (a flattened + * tool failure's output, a block error, a handler's rebuilt error), never a raw thrown value: a + * persistent fault can rethrow one object forever (a rejected dynamic `import()`, a memoized + * rejected promise), and marking it would silence every later occurrence process-wide. + * `loggedInExecution` scopes a raw value's mark to the one execution that logged it. */ const failureKinds = new WeakMap() -const loggedFailures = new WeakSet() +const loggedCarriers = new WeakSet() +const loggedInExecution = new WeakMap() function isKeyable(value: unknown): value is object { return (typeof value === 'object' || typeof value === 'function') && value !== null @@ -42,31 +49,6 @@ function causeChain(error: unknown): object[] { return chain } -/** - * The flattened tool failure a block handler copies onto the error it throws. `executeTool` - * turns a thrown failure into a `{ success: false, output }` result, so the handler's new error - * shares only `output` with the failure the tool layer saw. - */ -function readToolFailureOutput(link: object): object | undefined { - const output = (link as { output?: unknown }).output - return isKeyable(output) ? output : undefined -} - -/** - * Upstream HTTP status of an external call: `status` on a transformed tool error, or on the - * tool failure output a block handler copied. Never Sim's own `statusCode`. - */ -function readUpstreamStatus(link: object): number | undefined { - const status = (link as { status?: unknown }).status - if (typeof status === 'number' && status >= 400 && status < 600) return status - const output = readToolFailureOutput(link) as { status?: unknown } | undefined - const outputStatus = output?.status - if (typeof outputStatus === 'number' && outputStatus >= 400 && outputStatus < 600) { - return outputStatus - } - return undefined -} - /** Records who `error` is attributable to, overriding what its shape would imply. */ export function markFailureKind(error: T, kind: FailureKind): T { if (isKeyable(error)) failureKinds.set(error, kind) @@ -85,10 +67,15 @@ export function markDeliberateFailure(error: T, kind: FailureKind): T { return error } +/** A provider refusing the key it was given; Sim's fault when the key was Sim's own. */ +export function isProviderKeyRejection(status: unknown): boolean { + return status === 401 || status === 402 || status === 403 +} + /** * Attributes `error` from its cause chain. A database failure anywhere is always internal, - * then the outermost link with an explicit mark, a Sim `HttpError` status, or an upstream status - * decides. Anything unattributed is internal. + * then the outermost link with an explicit mark, a Sim `HttpError` status, or an upstream + * `status` decides. Anything unattributed is internal. */ export function classifyFailure(error: unknown): FailureKind { if (findDatabaseQueryError(error)) return 'internal' @@ -96,17 +83,16 @@ export function classifyFailure(error: unknown): FailureKind { for (const link of causeChain(error)) { if (isRetryableSetupError(link)) return 'internal' - const output = readToolFailureOutput(link) - const marked = failureKinds.get(link) ?? (output ? failureKinds.get(output) : undefined) + const marked = failureKinds.get(link) if (marked) return marked if (link instanceof HttpError) { return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal' } - const upstreamStatus = readUpstreamStatus(link) - if (upstreamStatus !== undefined) { - return upstreamStatus >= 500 ? 'third_party_server' : 'third_party_client' + const status = (link as { status?: unknown }).status + if (typeof status === 'number' && status >= 400 && status < 600) { + return status >= 500 ? 'third_party_server' : 'third_party_client' } } @@ -114,25 +100,37 @@ export function classifyFailure(error: unknown): FailureKind { } /** - * Records that the boundary owning this failure has logged it. `subject` is the thrown value, - * or the output of a flattened tool failure. + * Records that the boundary owning a failure logged it, on a value created during this + * propagation (see `loggedCarriers`). Never pass a raw thrown value. + */ +export function markFailureLogged(carrier: unknown): void { + if (isKeyable(carrier)) loggedCarriers.add(carrier) +} + +/** + * Moves a failed tool result's marks onto the error a handler rebuilds from it. `executeTool` + * flattens a thrown failure into `{ success: false, output }` and logs it once; without this, the + * handler's fresh error looks unlogged and unattributed and is logged again at error. */ -export function markFailureLogged(subject: unknown): void { - if (isKeyable(subject)) loggedFailures.add(subject) +export function adoptToolFailure(error: T, result: { output?: unknown }): T { + const output = result.output + if (!isKeyable(error) || !isKeyable(output)) return error + if (loggedCarriers.has(output)) loggedCarriers.add(error) + const kind = failureKinds.get(output) + if (kind && !failureKinds.has(error)) failureKinds.set(error, kind) + return error } /** - * Whether any link of the cause chain, or the tool failure output a link carries, was already - * logged. Survives `buildBlockExecutionError` and `ChildWorkflowError` (both keep `cause`), the - * engine's identity-preserving rethrow, and a handler rebuilding a failed tool result as a new - * error that carries the result's `output`. + * Whether a boundary already logged this failure: a logged carrier anywhere in the cause chain, + * or, given `executionId`, a raw value that boundary logged during this same execution. */ -export function wasFailureLogged(error: unknown): boolean { - return causeChain(error).some((link) => { - if (loggedFailures.has(link)) return true - const output = readToolFailureOutput(link) - return output !== undefined && loggedFailures.has(output) - }) +export function wasFailureLogged(error: unknown, executionId?: string): boolean { + return causeChain(error).some( + (link) => + loggedCarriers.has(link) || + (executionId !== undefined && loggedInExecution.get(link) === executionId) + ) } const LOG_LEVEL_BY_KIND = { @@ -143,17 +141,20 @@ const LOG_LEVEL_BY_KIND = { } as const satisfies Record /** - * Logs a failure at the severity its cause earns, unless a boundary closer to the cause already - * logged it, then marks it logged so every boundary it propagates through stays quiet. + * Logs a failure at the severity its cause earns unless a boundary closer to the cause already + * logged it. Pass `executionId` at execution-level boundaries (engine, execution core, trigger + * surfaces) so the raw value is marked for that execution only; below that level the caller + * marks the fresh carrier it throws with {@link markFailureLogged}. */ export function logFailureOnce( logger: Logger, message: string, error: unknown, - metadata: Record = {} + metadata: Record = {}, + executionId?: string ): void { - if (wasFailureLogged(error)) return + if (wasFailureLogged(error, executionId)) return const failureKind = classifyFailure(error) logger[LOG_LEVEL_BY_KIND[failureKind]](message, { ...metadata, failureKind }) - markFailureLogged(error) + if (executionId !== undefined && isKeyable(error)) loggedInExecution.set(error, executionId) } diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 30d3de73d96..0d96d5d1d81 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -959,8 +959,7 @@ async function executeInSandboxWithinBudget( if (execution.error) { const errorMessage = `${execution.error.name}: ${execution.error.value}` /** The author's code raised; only a provider-side failure is ours to look at. */ - const logFailure = execution.providerFailure ? logger.error : logger.info - logFailure.call(logger, 'Sandbox execution failed', { + logger[execution.providerFailure ? 'error' : 'info']('Sandbox execution failed', { sandboxId, hasTraceback: Boolean(execution.error.traceback), providerFailure: execution.providerFailure, @@ -1157,8 +1156,7 @@ async function executeShellInSandboxWithinBudget( const errorMessage = result.stderr || result.stdout || `Process exited with code ${result.exitCode}` /** The author's command exited non-zero; only a provider-side failure is ours to look at. */ - const logFailure = result.providerFailure ? logger.error : logger.info - logFailure.call(logger, 'Sandbox shell execution error', { + logger[result.providerFailure ? 'error' : 'info']('Sandbox shell execution error', { sandboxId, exitCode: result.exitCode, providerFailure: result.providerFailure, diff --git a/apps/sim/lib/function-execution/execute-request.ts b/apps/sim/lib/function-execution/execute-request.ts index e26dbd6c91d..476018bd8d0 100644 --- a/apps/sim/lib/function-execution/execute-request.ts +++ b/apps/sim/lib/function-execution/execute-request.ts @@ -3302,14 +3302,16 @@ export async function executeFunctionRequest( ) /** The author's code threw unless the isolate itself failed. */ - const logFailure = isSystemError ? logger.error : logger.info - logFailure.call(logger, `[${requestId}] Function execution failed in isolated-vm`, { - executionTime, - isSystemError, - hasStack: Boolean(ivmError.stack), - line: enhancedError.line, - column: enhancedError.column, - }) + logger[isSystemError ? 'error' : 'info']( + `[${requestId}] Function execution failed in isolated-vm`, + { + executionTime, + isSystemError, + hasStack: Boolean(ivmError.stack), + line: enhancedError.line, + column: enhancedError.column, + } + ) return functionJsonResponse( { diff --git a/apps/sim/lib/internal/tool-operations/execute-json-operation.ts b/apps/sim/lib/internal/tool-operations/execute-json-operation.ts index 10cc01c903b..f31b6ba769b 100644 --- a/apps/sim/lib/internal/tool-operations/execute-json-operation.ts +++ b/apps/sim/lib/internal/tool-operations/execute-json-operation.ts @@ -1,7 +1,11 @@ -import { toError } from '@sim/utils/errors' +import { createLogger } from '@sim/logger' +import { describeError, toError } from '@sim/utils/errors' import type { AnyApiRouteContract, ContractBody } from '@/lib/api/contracts' +import { logFailureOnce } from '@/lib/core/errors/failure-log' import { parseInternalToolInput } from '@/lib/internal/tool-operations/parse-input' +const logger = createLogger('InternalJsonToolOperation') + export async function executeInternalJsonToolOperation( contract: C, input: unknown, @@ -19,6 +23,8 @@ export async function executeInternalJsonToolOperation | null = null diff --git a/apps/sim/lib/workflows/executor/execution-core.test.ts b/apps/sim/lib/workflows/executor/execution-core.test.ts index ca2f5ec5c2d..543a557c01c 100644 --- a/apps/sim/lib/workflows/executor/execution-core.test.ts +++ b/apps/sim/lib/workflows/executor/execution-core.test.ts @@ -160,11 +160,13 @@ vi.mock('@/serializer', () => ({ }, })) +import { classifyFailure } from '@/lib/core/errors/failure-log' import { executeWorkflowCore, FINALIZED_EXECUTION_ID_TTL_MS, wasExecutionFinalizedByCore, } from '@/lib/workflows/executor/execution-core' +import { MissingRequiredFieldsError } from '@/serializer/errors' const uploadWorkflowInputMock = uploadsExecutionMockFns.mockUploadExecutionFile largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true) @@ -701,6 +703,32 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { ) }) + it.each([ + [ + 'a block missing a required field', + new MissingRequiredFieldsError('Slack', ['Slack Account']), + 'user', + ], + [ + 'an unknown block type, a registry regression', + new Error('Invalid block type: retired'), + 'internal', + ], + ] as const)('attributes a serializer refusal for %s', async (_name, refusal, kind) => { + serializeWorkflowMock.mockImplementationOnce(() => { + throw refusal + }) + + const thrown = await executeWorkflowCore({ + snapshot: createSnapshot() as any, + callbacks: {}, + loggingSession: loggingSession as any, + }).catch((error: unknown) => error) + + expect(thrown).toBe(refusal) + expect(classifyFailure(thrown)).toBe(kind) + }) + it('activates trusted pre-execution provenance on the installed execution registry', async () => { executorExecuteMock.mockResolvedValue({ success: true, diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index d8333f66681..ca991b81f40 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -14,11 +14,7 @@ import type { Edge } from '@xyflow/react' import { eq } from 'drizzle-orm' import { z } from 'zod' import { type EffectivePiiRedaction, resolveEffectivePiiRedaction } from '@/lib/billing/retention' -import { - logFailureOnce, - markDeliberateFailure, - markFailureKind, -} from '@/lib/core/errors/failure-log' +import { logFailureOnce, markFailureKind } from '@/lib/core/errors/failure-log' import { getExecutionDeadlineAt, getTimeoutErrorMessage, @@ -79,6 +75,7 @@ import { buildParallelSentinelEndId, } from '@/executor/utils/subflow-node-id-codec' import { Serializer } from '@/serializer' +import { MissingRequiredFieldsError } from '@/serializer/errors' import type { SerializedWorkflow } from '@/serializer/types' const logger = createLogger('ExecutionCore') @@ -905,8 +902,10 @@ async function executeWorkflowCoreImpl( true ) } catch (serializeError) { - /** The serializer throws a plain `Error` for the author's configuration (missing required fields, unknown block type). */ - throw markDeliberateFailure(serializeError, 'user') + /** An unknown block type stays internal: a registry regression or an unregistered block. */ + throw serializeError instanceof MissingRequiredFieldsError + ? markFailureKind(serializeError, 'user') + : serializeError } const inputFileKeys = new Set() processedInput = @@ -1368,7 +1367,8 @@ async function executeWorkflowCoreImpl( workflowId, executionId, ...(errorCause ? { cause: errorCause } : {}), - }) + }), + executionId ) await waitForLifecycleCallbacks() diff --git a/apps/sim/serializer/errors.ts b/apps/sim/serializer/errors.ts new file mode 100644 index 00000000000..381f82601b7 --- /dev/null +++ b/apps/sim/serializer/errors.ts @@ -0,0 +1,10 @@ +/** + * A block the author left without a value it requires. Raised before execution starts; the + * author's configuration, not a Sim fault, so execution logs it at info. + */ +export class MissingRequiredFieldsError extends Error { + constructor(blockName: string, missingFields: string[]) { + super(`${blockName} is missing required fields: ${missingFields.join(', ')}`) + this.name = 'MissingRequiredFieldsError' + } +} diff --git a/apps/sim/serializer/index.ts b/apps/sim/serializer/index.ts index ff362692c7b..41fa200cc38 100644 --- a/apps/sim/serializer/index.ts +++ b/apps/sim/serializer/index.ts @@ -23,6 +23,7 @@ import { import { getBlock } from '@/blocks' import { isCustomBlockType, RESERVED_PARAMS } from '@/blocks/custom/build-config' import type { SubBlockConfig } from '@/blocks/types' +import { MissingRequiredFieldsError } from '@/serializer/errors' import type { SerializedBlock, SerializedWorkflow } from '@/serializer/types' import type { BlockState, Loop, Parallel } from '@/stores/workflows/workflow/types' import { getToolParams } from '@/tools/metadata' @@ -259,9 +260,7 @@ export class Serializer { const { missingRequiredFields } = collectBlockFieldIssues(block, blockConfig, params) if (missingRequiredFields.length > 0) { const blockName = block.name || blockConfig.name || 'Block' - throw new Error( - `${blockName} is missing required fields: ${missingRequiredFields.join(', ')}` - ) + throw new MissingRequiredFieldsError(blockName, missingRequiredFields) } } diff --git a/apps/sim/tools/index.test.ts b/apps/sim/tools/index.test.ts index 636fba47f62..455ec0964c7 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -420,7 +420,7 @@ vi.mock('@/tools/utils.server', async (importOriginal) => { }) import type { QueryClient } from '@tanstack/react-query' -import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { adoptToolFailure, classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' import * as getQueryClientModule from '@/app/_shell/providers/get-query-client' import { ApiBlockHandler } from '@/executor/handlers/api/api-handler' import { buildBlockExecutionError } from '@/executor/utils/errors' @@ -1127,6 +1127,78 @@ describe('executeTool Function', () => { expect(fetchSpy).not.toHaveBeenCalled() }) + it('logs a fault that rethrows one object every time once per occurrence, not once per process', async () => { + /** A rejected dynamic `import()` or memoized rejected promise rethrows the same object. */ + const persistentFault = new Error('internal operation module failed to load') + mockAssertPermissionsAllowed.mockRejectedValue(persistentFault) + mockToolsLogger.error.mockClear() + + for (const executionId of ['execution-a', 'execution-b']) { + const result = await executeTool( + 'http_request', + { url: 'https://example.com' }, + { executionContext: createToolExecutionContext({ userId: 'user-123', executionId }) } + ) + expect(result.success).toBe(false) + } + + const toolFailureLogs = mockToolsLogger.error.mock.calls.filter(([message]) => + String(message).includes('Error executing tool http_request') + ) + expect(toolFailureLogs).toHaveLength(2) + }) + + it.each([ + [400, 'user'], + [500, 'internal'], + ] as const)( + 'attributes an in-process operation %i to Sim, never to a third party', + async (status, kind) => { + mockExecuteInternalToolOperation.mockResolvedValueOnce( + jsonResponse({ error: 'operation failed' }, status) + ) + + const result = await executeTool( + 'sts_get_caller_identity', + { region: 'us-east-1', accessKeyId: 'access-key', secretAccessKey: 'secret-key' }, + { + executionContext: createToolExecutionContext({ + userId: 'user-1', + workspaceId: 'workspace-456', + workflowId: 'workflow-1', + executionId: 'execution-1', + }), + } + ) + + expect(result.success).toBe(false) + expect(classifyFailure(adoptToolFailure(new Error(result.error), result))).toBe(kind) + } + ) + + it('never logs the response body of an in-process operation failure', async () => { + const rowContent = 'author-row-content-7f3a' + mockExecuteInternalToolOperation.mockResolvedValueOnce( + jsonResponse({ error: 'duplicate row', details: { row: rowContent } }, 400) + ) + + const result = await executeTool( + 'sts_get_caller_identity', + { region: 'us-east-1', accessKeyId: 'access-key', secretAccessKey: 'secret-key' }, + { + executionContext: createToolExecutionContext({ + userId: 'user-1', + workspaceId: 'workspace-456', + workflowId: 'workflow-1', + executionId: 'execution-1', + }), + } + ) + + expect(result.success).toBe(false) + expect(JSON.stringify(allToolLogCalls())).not.toContain(rowContent) + }) + it('preserves a registered operation failure without turning it into success', async () => { const mockTool = { id: 'test_registered_operation_failure', @@ -1779,7 +1851,8 @@ describe('executeTool Function', () => { JSON.stringify({ success: false, error: `Execution failed with ${secret} via ${runtimeAlias}`, - output: { result: null, stdout: 'trace', cost }, + output: { result: null, stdout: 'author-stdout-91c', cost }, + debug: { lineContent: 'author-source-line-55a', stack: 'author-stack-3e1' }, __resolvedSecretNames: ['API_KEY'], }), { @@ -1809,6 +1882,15 @@ describe('executeTool Function', () => { expect(result.output?.cost).toEqual(cost) expect(JSON.stringify(allToolLogCalls())).not.toContain(secret) expect(JSON.stringify(allToolLogCalls())).not.toContain(runtimeAlias) + for (const authorContent of [ + 'author-stdout-91c', + 'author-source-line-55a', + 'author-stack-3e1', + ]) { + expect(JSON.stringify(allToolLogCalls())).not.toContain(authorContent) + } + /** A Function's 422 is the author's code throwing, not a third party refusing. */ + expect(classifyFailure(adoptToolFailure(new Error(result.error), result))).toBe('user') expect(JSON.stringify(projectToolResultForCopilot(result, registry))).not.toContain(secret) expect(JSON.stringify(projectToolResultForCopilot(result, registry))).not.toContain( runtimeAlias diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 778cc614eec..12e4600beb6 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -19,6 +19,7 @@ import { isHosted } from '@/lib/core/config/env-flags' import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' import { classifyFailure, + isProviderKeyRejection, logFailureOnce, markDeliberateFailure, markFailureKind, @@ -1061,31 +1062,14 @@ const RESPONSE_SIZE_LIMIT_ERROR_MESSAGE = const SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE = 'External integration tools cannot target this Sim instance; use an internal operation' -/** - * Validates request body size and throws a user-friendly error if exceeded - * @param body - The request body string to check - * @param requestId - Request ID for logging - * @param context - Context string for logging (e.g., toolId) - * @throws Error if body size exceeds the limit - */ -function validateRequestBodySize( - body: string | undefined, - requestId: string, - context: string -): void { - if (!body) return - - const bodySize = Buffer.byteLength(body, 'utf8') - if (bodySize > MAX_REQUEST_BODY_SIZE_BYTES) { - const bodySizeMB = (bodySize / (1024 * 1024)).toFixed(2) - const maxSizeMB = (MAX_REQUEST_BODY_SIZE_BYTES / (1024 * 1024)).toFixed(0) - logger.error(`[${requestId}] Request body size exceeds limit for ${context}:`, { - bodySize, - bodySizeMB: `${bodySizeMB}MB`, - maxSize: MAX_REQUEST_BODY_SIZE_BYTES, - maxSizeMB: `${maxSizeMB}MB`, - }) - throw new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE) +/** The author's workflow data is too large to send; their fault, logged once by the tool catch. */ +function bodySizeLimitError(): Error { + return markFailureKind(new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE), 'user') +} + +function validateRequestBodySize(body: string | undefined): void { + if (body && Buffer.byteLength(body, 'utf8') > MAX_REQUEST_BODY_SIZE_BYTES) { + throw bodySizeLimitError() } } @@ -1106,39 +1090,9 @@ function isBodySizeLimitError(errorMessage: string): boolean { ) } -/** - * Handles body size limit errors by logging and throwing a user-friendly error - * @param error - The original error - * @param requestId - Request ID for logging - * @param context - Context string for logging (e.g., toolId) - * @throws Error with user-friendly message if it's a size limit error - * @returns false if not a size limit error (caller should continue handling) - */ -function handleBodySizeLimitError( - error: unknown, - requestId: string, - context: string, - resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry, - structuralOnlyWithoutRegistry = false -): boolean { - const errorMessage = toError(error).message - - if (isBodySizeLimitError(errorMessage)) { - logger.error( - `[${requestId}] Request body size limit exceeded for ${context}:`, - projectToolLogMetadata( - { originalError: errorMessage }, - resolvedSecretTraceRegistry, - { - hasOriginalError: errorMessage.length > 0, - }, - structuralOnlyWithoutRegistry - ) - ) - throw new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE) - } - - return false +/** Rethrows a transport-level body size rejection as the user-facing size limit error. */ +function handleBodySizeLimitError(error: unknown): void { + if (isBodySizeLimitError(toError(error).message)) throw bodySizeLimitError() } function handleResponseSizeLimitError(error: unknown, requestId: string, context: string): boolean { @@ -1204,9 +1158,28 @@ async function readToolResponseBody( } } -/** A provider refusing Sim's own hosted key is Sim's fault, not the workflow author's. */ -function isOwnHostedKeyRejection(status: unknown): boolean { - return status === 401 || status === 402 || status === 403 +/** + * Upstream rejections from the external HTTP path. Only their (extractor-redacted) response body + * is logged: an internal operation's or a Function's body carries the author's data, stdout, and + * source lines. + */ +const externalHttpFailures = new WeakSet() + +function createExternalHttpFailure(errorInfo?: ErrorInfo, extractorId?: string): Error { + const failure = createTransformedErrorFromErrorInfo(errorInfo, extractorId) + externalHttpFailures.add(failure) + return failure +} + +/** + * Attributes a failed in-process operation. Its status is Sim's own, not a third party's: a 4xx + * is the author's input or code (a Function's 422), a 5xx is Sim's fault. + */ +function createInternalOperationFailure(errorInfo: ErrorInfo, extractorId?: string): Error { + return markFailureKind( + createTransformedErrorFromErrorInfo(errorInfo, extractorId), + errorInfo.status !== undefined && errorInfo.status < 500 ? 'user' : 'internal' + ) } /** @@ -2341,7 +2314,8 @@ async function executeToolImplementation( const databaseQueryError = findDatabaseQueryError(error) const databaseErrorCause = databaseQueryError ? describeError(error) : undefined const upstreamStatus = (error as { status?: unknown } | null)?.status - if (hostedKeyForMetrics && isOwnHostedKeyRejection(upstreamStatus)) { + /** Sim's own hosted key being refused or throttled is Sim's fault and Sim's capacity. */ + if (hostedKeyForMetrics && (isProviderKeyRejection(upstreamStatus) || upstreamStatus === 429)) { markFailureKind(error, 'internal') } const toolContext = params._context as Record | undefined @@ -2358,7 +2332,9 @@ async function executeToolImplementation( : { error: normalizedError.message, stack: error instanceof Error ? error.stack : undefined, - errorData: (error as { data?: unknown } | null)?.data, + ...(error instanceof Error && externalHttpFailures.has(error) + ? { errorData: (error as { data?: unknown }).data } + : {}), }), }, resolvedSecretTraceRegistry, @@ -2739,7 +2715,7 @@ async function executeDeclaredInternalOperation({ if (privateToolMetadataType) { headers.set(PRIVATE_TOOL_METADATA_REQUEST_HEADER, privateToolMetadataType) } - validateRequestBodySize(JSON.stringify(operationInput), requestId, toolId) + validateRequestBodySize(JSON.stringify(operationInput)) const deadline = serializeExecutionDeadlineHeader(signal) if (deadline) headers.set(INTERNAL_EXECUTION_DEADLINE_HEADER, deadline) const billingAttribution = context.billingAttribution @@ -2845,7 +2821,7 @@ async function executeDeclaredInternalOperation({ } catch { errorData = errorText } - throw createTransformedErrorFromErrorInfo( + throw createInternalOperationFailure( { status: response.status, statusText: response.statusText, data: errorData }, tool.errorExtractor ) @@ -2873,7 +2849,6 @@ async function executeToolRequest( resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry ): Promise { const requestId = generateRequestId() - const structuralOnlyToolLogs = false try { const requestParams = prepareToolRequest(tool, params, resolvedSecretTraceRegistry) const { headers } = requestParams @@ -2891,7 +2866,7 @@ async function executeToolRequest( } } - validateRequestBodySize(requestParams.body, requestId, toolId) + validateRequestBodySize(requestParams.body) const headersRecord: Record = {} headers.forEach((value, key) => { @@ -3054,29 +3029,12 @@ async function executeToolRequest( data: errorData, } - const errorToTransform = createTransformedErrorFromErrorInfo(errorInfo, tool.errorExtractor) + const errorToTransform = createExternalHttpFailure(errorInfo, tool.errorExtractor) const hasStructuredErrorPayload = isRecordLike(errorData) && ('error' in errorData || 'message' in errorData) if (response.status === 413 && !hasStructuredErrorPayload) { - logger.error( - `[${requestId}] Request body too large for ${toolId} (HTTP 413):`, - projectToolLogMetadata( - { - status: response.status, - statusText: response.statusText, - errorData, - }, - resolvedSecretTraceRegistry, - { - status: response.status, - statusText: response.statusText, - hasErrorData: errorData !== null, - }, - structuralOnlyToolLogs - ) - ) - throw new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE) + throw bodySizeLimitError() } throw errorToTransform @@ -3093,16 +3051,6 @@ async function executeToolRequest( try { responseData = await response.json() } catch (jsonError) { - const normalizedError = toError(jsonError) - logger.error( - `[${requestId}] JSON parse error for ${toolId}:`, - projectToolLogMetadata( - { error: normalizedError.message }, - resolvedSecretTraceRegistry, - { errorName: normalizedError.name }, - structuralOnlyToolLogs - ) - ) throw new Error(`Failed to parse response from ${toolId}: ${jsonError}`) } } @@ -3111,7 +3059,7 @@ async function executeToolRequest( const { isError, errorInfo } = isErrorResponse(response, responseData) if (isError) { - throw createTransformedErrorFromErrorInfo(errorInfo, tool.errorExtractor) + throw createExternalHttpFailure(errorInfo, tool.errorExtractor) } if (tool.transformResponse) { @@ -3176,13 +3124,7 @@ async function executeToolRequest( } catch (error: any) { handleResponseSizeLimitError(error, requestId, toolId) - handleBodySizeLimitError( - error, - requestId, - toolId, - resolvedSecretTraceRegistry, - structuralOnlyToolLogs - ) + handleBodySizeLimitError(error) if (isRetryableNetworkError(error)) markFailureKind(error, 'third_party_server') @@ -3274,7 +3216,7 @@ async function executeMcpTool( try { logger.info(`[${actualRequestId}] Executing MCP tool: ${toolId}`) - validateRequestBodySize(JSON.stringify(params), actualRequestId, `mcp:${toolId}`) + validateRequestBodySize(JSON.stringify(params)) const handler = await getInternalToolOperationHandler(toolId) if (!handler) throw new Error(`No internal operation registered for ${toolId}`) const resultResponse = await handler({ From 30d9895095926849c09c9f1e5d3b5be83b82fe9d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:25:57 -0700 Subject: [PATCH 04/10] 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. --- .../app/api/workflows/[id]/execute/route.ts | 23 ++---- .../async-preprocessing-correlation.test.ts | 4 +- apps/sim/background/webhook-execution.test.ts | 2 +- apps/sim/background/webhook-execution.ts | 19 ++--- apps/sim/background/workflow-execution.ts | 11 +-- apps/sim/executor/errors/boundary.ts | 5 +- apps/sim/executor/execution/block-executor.ts | 54 +++++++----- apps/sim/executor/execution/engine.ts | 29 +++---- .../executor/handlers/agent/agent-handler.ts | 16 +++- .../handlers/pi/cloud/authoring/backend.ts | 21 +---- .../executor/handlers/pi/cloud/github-pr.ts | 14 +--- .../handlers/pi/cloud/review/backend.ts | 9 +- apps/sim/executor/handlers/pi/cloud/shared.ts | 10 +++ .../handlers/workflow/workflow-handler.ts | 25 ++---- apps/sim/lib/core/errors/failure-log.test.ts | 32 ++++---- apps/sim/lib/core/errors/failure-log.ts | 78 ++++++++++-------- apps/sim/lib/core/errors/user-failure.ts | 6 ++ .../sim/lib/execution/remote-sandbox/index.ts | 28 ++++--- .../lib/function-execution/execute-request.ts | 12 ++- .../tool-operations/execute-json-operation.ts | 2 +- .../lib/workflows/executor/execute-service.ts | 2 +- .../lib/workflows/executor/execution-core.ts | 54 +++++------- apps/sim/serializer/errors.ts | 9 +- apps/sim/tools/index.ts | 82 ++++++++++--------- 24 files changed, 262 insertions(+), 285 deletions(-) create mode 100644 apps/sim/lib/core/errors/user-failure.ts diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 59914e7d276..7a768159e64 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -1616,13 +1616,11 @@ async function handleExecutePost( return payloadTooLargeResponse() } - logFailureOnce( - reqLogger, - 'Non-SSE execution failed', - error, - loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }), - executionId - ) + logFailureOnce(reqLogger, 'Non-SSE execution failed', error, { + metadata: () => + loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }), + executionId, + }) const executionResult = hasExecutionResult(error) ? error.executionResult : undefined const status = executionTimedOut ? 408 : getExecutionErrorStatus(error) @@ -2424,13 +2422,10 @@ async function handleExecutePost( ? getTimeoutErrorMessage(timeoutController.timeoutMs) : getErrorMessage(error, 'Unknown error') - logFailureOnce( - reqLogger, - 'SSE execution failed', - error, - loggingSession.projectDiagnosticError(error, { isTimeout }), - executionId - ) + logFailureOnce(reqLogger, 'SSE execution failed', error, { + metadata: () => loggingSession.projectDiagnosticError(error, { isTimeout }), + executionId, + }) const executionResult = hasExecutionResult(error) ? error.executionResult : undefined let compactErrorLogs: BlockLog[] | undefined diff --git a/apps/sim/background/async-preprocessing-correlation.test.ts b/apps/sim/background/async-preprocessing-correlation.test.ts index b4a26a04147..0a789bc5dd6 100644 --- a/apps/sim/background/async-preprocessing-correlation.test.ts +++ b/apps/sim/background/async-preprocessing-correlation.test.ts @@ -488,9 +488,7 @@ describe('async preprocessing correlation threading', () => { }), }) ) - expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError, { - executionId: 'execution-fault', - }) + expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError) expect(workflowExecutionLogger.error).toHaveBeenCalledWith( '[request-fault] Workflow execution failed: workflow-1', { executionId: 'execution-fault', error: projectedError, failureKind: 'internal' } diff --git a/apps/sim/background/webhook-execution.test.ts b/apps/sim/background/webhook-execution.test.ts index ba58994a3ca..eaa2c6f8de9 100644 --- a/apps/sim/background/webhook-execution.test.ts +++ b/apps/sim/background/webhook-execution.test.ts @@ -482,12 +482,12 @@ describe('executeWebhookJob fault vs error handling', () => { expect(loggingSessionMockFns.mockSafeCompleteWithError).toHaveBeenCalled() expect(loggingSessionMockFns.mockProjectDiagnosticError).toHaveBeenCalledWith(rawError, { workflowId: 'workflow-1', - executionId: 'execution-1', provider: 'gmail', }) expect(webhookExecutionLogger.error).toHaveBeenCalledWith( '[request-1] Webhook execution failed', { + executionId: 'execution-1', workflowId: 'workflow-1', provider: 'gmail', error: projectedError, diff --git a/apps/sim/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index 5737307bf7b..632185c7a9f 100644 --- a/apps/sim/background/webhook-execution.ts +++ b/apps/sim/background/webhook-execution.ts @@ -1268,17 +1268,14 @@ async function executeWebhookJobInternal( throw new RetryableSetupError(errorMessage, { cause: retryableSetupCause }) } - logFailureOnce( - logger, - `[${requestId}] Webhook execution failed`, - error, - loggingSession.projectDiagnosticError(error, { - workflowId: payload.workflowId, - executionId, - provider: payload.provider, - }), - executionId - ) + logFailureOnce(logger, `[${requestId}] Webhook execution failed`, error, { + metadata: () => + loggingSession.projectDiagnosticError(error, { + workflowId: payload.workflowId, + provider: payload.provider, + }), + executionId, + }) // The finalized flag is set inside a fire-and-forget post-execution promise; await it so the // signal is reliable and the failure is fully persisted before we decide fault vs error. diff --git a/apps/sim/background/workflow-execution.ts b/apps/sim/background/workflow-execution.ts index 259035d0bac..145ab0970f0 100644 --- a/apps/sim/background/workflow-execution.ts +++ b/apps/sim/background/workflow-execution.ts @@ -307,13 +307,10 @@ export async function executeWorkflowJob( metadata: payload.metadata, } } catch (error: unknown) { - logFailureOnce( - logger, - `[${requestId}] Workflow execution failed: ${workflowId}`, - error, - loggingSession.projectDiagnosticError(error, { executionId }), - executionId - ) + logFailureOnce(logger, `[${requestId}] Workflow execution failed: ${workflowId}`, error, { + metadata: () => loggingSession.projectDiagnosticError(error), + executionId, + }) if (error instanceof ExecutionTimeoutError) throw error diff --git a/apps/sim/executor/errors/boundary.ts b/apps/sim/executor/errors/boundary.ts index 4371c174bce..bf24caf4e89 100644 --- a/apps/sim/executor/errors/boundary.ts +++ b/apps/sim/executor/errors/boundary.ts @@ -1,4 +1,4 @@ -import { markFailureKind } from '@/lib/core/errors/failure-log' +import { UserFailure } from '@/lib/core/errors/user-failure' /** * Machine-readable class of a custom-block failure. Every member describes a @@ -41,14 +41,13 @@ export interface CustomBlockFailure { * replaces the older convention of throwing *before* the `try` block to dodge * the catch's sanitizer, where redaction depended on lexical position. */ -export class BoundarySafeError extends Error { +export class BoundarySafeError extends UserFailure { readonly errorType: CustomBlockErrorType constructor(options: { message: string; errorType: CustomBlockErrorType }) { super(options.message) this.name = 'BoundarySafeError' this.errorType = options.errorType - markFailureKind(this, 'user') } } diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 1bdc2c61dd6..c8bd6a8707e 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -789,23 +789,31 @@ export class BlockExecutor { } } - const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry - ? ctx.errorResolvedSecretTraceRegistry - : inputDisplayRegistry?.forkForToolCall() - if ( - !ctx.errorResolvedSecretTraceRegistry && - diagnosticRegistry && - ctx.resolvedSecretTraceRegistry && - ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry - ) { - diagnosticRegistry.mergeToolCallRegistry(ctx.resolvedSecretTraceRegistry) + /** Projected only when logged: a tool failure arrives already logged and skips it. */ + let errorDiagnostic: Record | undefined + const getErrorDiagnostic = () => { + if (errorDiagnostic) return errorDiagnostic + if (isDatabaseError) { + errorDiagnostic = { cause: describeError(error) } + return errorDiagnostic + } + const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry + ? ctx.errorResolvedSecretTraceRegistry + : inputDisplayRegistry?.forkForToolCall() + if ( + !ctx.errorResolvedSecretTraceRegistry && + diagnosticRegistry && + ctx.resolvedSecretTraceRegistry && + ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry + ) { + diagnosticRegistry.mergeToolCallRegistry(ctx.resolvedSecretTraceRegistry) + } + errorDiagnostic = projectResolvedSecretDiagnosticError( + error, + diagnosticRegistry ?? ctx.resolvedSecretTraceRegistry + ) + return errorDiagnostic } - const errorDiagnostic = isDatabaseError - ? { cause: describeError(error) } - : projectResolvedSecretDiagnosticError( - error, - diagnosticRegistry ?? ctx.resolvedSecretTraceRegistry - ) /** A user Stop or the run's own time limit aborted this block; neither is a Sim fault. */ if (isAbort && ctx.abortSignal?.aborted) markFailureKind(error, 'user') @@ -814,11 +822,13 @@ export class BlockExecutor { phase === 'input_resolution' ? 'Failed to resolve block inputs' : 'Block execution failed', error, { - blockId: node.id, - blockType: block.metadata?.id, - executionId: ctx.executionId, - workflowId: ctx.workflowId, - ...errorDiagnostic, + metadata: () => ({ + blockId: node.id, + blockType: block.metadata?.id, + executionId: ctx.executionId, + workflowId: ctx.workflowId, + ...getErrorDiagnostic(), + }), } ) @@ -857,7 +867,7 @@ export class BlockExecutor { } this.execLogger.info('Block has error port - returning error output instead of throwing', { blockId: node.id, - ...errorDiagnostic, + ...getErrorDiagnostic(), }) return errorOutput } diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index 2bc5f448a5a..48dbf220770 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -197,13 +197,11 @@ export class ExecutionEngine { this.finalizeIncompleteLogs() const errorMessage = normalizeError(error) - logFailureOnce( - this.execLogger, - 'Execution failed', - error, - projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), - this.context.executionId - ) + logFailureOnce(this.execLogger, 'Execution failed', error, { + metadata: () => + projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), + executionId: this.context.executionId, + }) const executionResult: ExecutionResult = { success: false, @@ -485,16 +483,13 @@ export class ExecutionEngine { * Block failures were logged by the block executor. This catches a completion-handling * fault, which only this frame sees when a concurrent failure already won `executionError`. */ - logFailureOnce( - this.execLogger, - 'Node execution failed', - error, - { - nodeId, - ...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), - }, - this.context.executionId - ) + logFailureOnce(this.execLogger, 'Node execution failed', error, { + metadata: () => + projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry, { + nodeId, + }), + executionId: this.context.executionId, + }) throw error } } diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 0ad3fbfdaad..ac9521d0b94 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { isPlainRecord, omit } from '@sim/utils/object' import { truncate } from '@sim/utils/string' -import { isProviderKeyRejection, markFailureKind } from '@/lib/core/errors/failure-log' +import { markFailureKind } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { projectModelSchemaAnnotations, @@ -327,6 +327,12 @@ function describeProviderTransportFailure(error: Error): string | null { return null } +/** A provider refusing the key it was given. */ +function isProviderKeyRejection(error: unknown): boolean { + const status = (error as { status?: unknown } | null)?.status + return status === 401 || status === 402 || status === 403 +} + /** * Handler for Agent blocks that process LLM requests with optional tools. */ @@ -3042,8 +3048,10 @@ export class AgentBlockHandler implements BlockHandler { return this.processProviderResponse(response, block, responseFormat, ctx) } catch (error) { - const errorRegistry = this.createErrorRegistry(providerErrorRegistry, modelRuntimeRegistry) - ctx.errorResolvedSecretTraceRegistry = errorRegistry + ctx.errorResolvedSecretTraceRegistry = this.createErrorRegistry( + providerErrorRegistry, + modelRuntimeRegistry + ) try { this.handleExecutionError(error) } finally { @@ -3076,7 +3084,7 @@ export class AgentBlockHandler implements BlockHandler { throw markFailureKind(new Error(transportFailure), 'third_party_server') } /** The handler cannot tell a hosted provider key (Sim's) from the author's own. */ - if (isProviderKeyRejection((error as { status?: unknown } | null)?.status)) { + if (isProviderKeyRejection(error)) { markFailureKind(error, 'internal') } } diff --git a/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts index 9f913a62ad9..20f0c447f5f 100644 --- a/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts @@ -22,7 +22,6 @@ import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' -import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getMaxExecutionTimeout, getRemainingExecutionMs } from '@/lib/core/execution-limits' import { withPiSandbox } from '@/lib/execution/remote-sandbox' import { @@ -58,6 +57,7 @@ import { raceAbort, resolvePiTimeoutMs, scrubGitSecrets, + toolResultError, } from '@/executor/handlers/pi/cloud/shared' import type { PiBackendRun, @@ -186,10 +186,7 @@ async function openPullRequest( ) if (!result.success) { - throw adoptToolFailure( - new Error(`PR creation failed for branch ${branch}: ${result.error ?? 'unknown error'}`), - result - ) + throw toolResultError(`PR creation failed for branch ${branch}`, result) } if (!isRecordLike(result.output)) { @@ -225,12 +222,7 @@ async function repositoryDefaultBranch( { signal } ) if (!result.success) { - throw adoptToolFailure( - new Error( - `Failed to determine the repository default branch: ${result.error ?? 'unknown error'}` - ), - result - ) + throw toolResultError('Failed to determine the repository default branch', result) } if (!isRecordLike(result.output)) { throw new Error('GitHub repository response must be an object') @@ -271,12 +263,7 @@ async function updatePullRequest( { signal } ) if (!result.success) { - throw adoptToolFailure( - new Error( - `PR update failed for branch ${params.targetBranch}: ${result.error ?? 'unknown error'}` - ), - result - ) + throw toolResultError(`PR update failed for branch ${params.targetBranch}`, result) } } diff --git a/apps/sim/executor/handlers/pi/cloud/github-pr.ts b/apps/sim/executor/handlers/pi/cloud/github-pr.ts index 9c17a2a5a37..290a38da059 100644 --- a/apps/sim/executor/handlers/pi/cloud/github-pr.ts +++ b/apps/sim/executor/handlers/pi/cloud/github-pr.ts @@ -7,7 +7,7 @@ */ import { isRecordLike } from '@sim/utils/object' -import { adoptToolFailure } from '@/lib/core/errors/failure-log' +import { toolResultError } from '@/executor/handlers/pi/cloud/shared' import { executeTool } from '@/tools' import { GITHUB_GRAPHQL_URL, githubGraphQlHeaders, readGraphQlData } from '@/tools/github/graphql' import { @@ -110,10 +110,7 @@ export async function fetchPrSnapshot( ) if (!result.success) { - throw adoptToolFailure( - new Error(`Failed to fetch PR #${params.pullNumber}: ${result.error ?? 'unknown error'}`), - result - ) + throw toolResultError(`Failed to fetch PR #${params.pullNumber}`, result) } return parsePullRequestSnapshot(result.output) @@ -177,12 +174,7 @@ export async function findOpenPrForBranch( { signal } ) if (!result.success) { - throw adoptToolFailure( - new Error( - `Failed to find an open PR for branch ${params.branch}: ${result.error ?? 'unknown error'}` - ), - result - ) + throw toolResultError(`Failed to find an open PR for branch ${params.branch}`, result) } const output = result.output diff --git a/apps/sim/executor/handlers/pi/cloud/review/backend.ts b/apps/sim/executor/handlers/pi/cloud/review/backend.ts index d0b587c94d2..83263157fdb 100644 --- a/apps/sim/executor/handlers/pi/cloud/review/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/review/backend.ts @@ -11,7 +11,6 @@ import { join } from 'node:path' import { createLogger } from '@sim/logger' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' -import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { withPiSandbox } from '@/lib/execution/remote-sandbox' import { resolvePiRunLifetimeMs } from '@/lib/execution/remote-sandbox/pi-lifetime' import { @@ -32,6 +31,7 @@ import { REPO_DIR, raceAbort, scrubGitSecrets, + toolResultError, } from '@/executor/handlers/pi/cloud/shared' import type { PiBackendRun, PiCloudReviewRunParams } from '@/executor/handlers/pi/core/backend' import { buildPiPrompt } from '@/executor/handlers/pi/core/context' @@ -187,12 +187,7 @@ async function submitReview( ) if (!result.success) { - throw adoptToolFailure( - new Error( - `Failed to submit review for PR #${params.pullNumber}: ${result.error ?? 'unknown error'}` - ), - result - ) + throw toolResultError(`Failed to submit review for PR #${params.pullNumber}`, result) } const output: unknown = result.output diff --git a/apps/sim/executor/handlers/pi/cloud/shared.ts b/apps/sim/executor/handlers/pi/cloud/shared.ts index a1bf348d2ae..58da46e6573 100644 --- a/apps/sim/executor/handlers/pi/cloud/shared.ts +++ b/apps/sim/executor/handlers/pi/cloud/shared.ts @@ -5,11 +5,13 @@ * security-sensitive details. */ +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { resolvePiSandboxLifetimeMs } from '@/lib/execution/remote-sandbox/pi-lifetime' import { PI_EVENT_FILTER_PATH } from '@/executor/handlers/pi/cloud/event-filter-source' import { scrubPiSecrets } from '@/executor/handlers/pi/core/redaction' import { PI_PACKAGE_VERSION } from '@/scripts/pi-sandbox-packages' +import type { ToolResponse } from '@/tools/types' export const REPO_DIR = '/workspace/repo' export const PROMPT_PATH = '/workspace/pi-prompt.txt' @@ -220,3 +222,11 @@ export function scrubGitSecrets(text: string, token: string): string { const withoutToken = scrubPiSecrets(text, [token]) return withoutToken.replace(/\/\/[^/@\s]+@/g, '//***@') } + +/** + * The error a backend throws for a failed GitHub tool call. Carries the tool layer's marks so the + * failure `executeTool` already logged is not logged again at error. + */ +export function toolResultError(label: string, result: ToolResponse): Error { + return adoptToolFailure(new Error(`${label}: ${result.error ?? 'unknown error'}`), result) +} diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index d10f7f74478..78fc360071a 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -5,10 +5,9 @@ import { isRecordLike } from '@sim/utils/object' import type { Variable, WorkflowState } from '@sim/workflow-types/workflow' import { resolveBillingAttribution } from '@/lib/billing/core/billing-attribution' import { - classifyFailure, + inheritFailureMarks, markFailureKind, markFailureLogged, - wasFailureLogged, } from '@/lib/core/errors/failure-log' import { getExecutionDeadlineAt } from '@/lib/core/execution-limits' import { withResourceOutboundScope } from '@/lib/core/network/resource-scope.server' @@ -1030,16 +1029,7 @@ export class WorkflowBlockHandler implements BlockHandler { // `buildBoundaryFailure` preserves an already-attached `consumerFacing`, so the // depth guard keeps its own classification. if (isCustomBlock) { - const boundaryFailure = this.buildBoundaryFailure( - error, - block, - instanceId, - childExecutionId, - traceChildRuns - ) - /** The boundary severs `cause`, so a logged mark has to cross it explicitly. */ - if (wasFailureLogged(error)) markFailureLogged(boundaryFailure) - throw boundaryFailure + throw this.buildBoundaryFailure(error, block, instanceId, childExecutionId, traceChildRuns) } // An error this same invocation already attributed (e.g. the depth guard, or @@ -1186,10 +1176,8 @@ export class WorkflowBlockHandler implements BlockHandler { ChildWorkflowError.isChildWorkflowError(error) && error.consumerFacing ? error.consumerFacing : undefined - /** The boundary severs `cause`, so the failure's attribution has to cross it explicitly. */ - const failureKind = classifyFailure(error) if (alreadyClassified) { - return markFailureKind( + return inheritFailureMarks( new ChildWorkflowError({ message: alreadyClassified.message, childWorkflowName: blockName, @@ -1197,7 +1185,7 @@ export class WorkflowBlockHandler implements BlockHandler { consumerFacing: alreadyClassified, ...traceHandle, }), - failureKind + error ) } @@ -1212,7 +1200,8 @@ export class WorkflowBlockHandler implements BlockHandler { ? `Custom block execution failed (ref: ${ref})` : 'Custom block execution failed' - return markFailureKind( + /** No `cause` crosses the boundary, so the failure's marks are carried explicitly. */ + return inheritFailureMarks( new ChildWorkflowError({ message, childWorkflowName: blockName, @@ -1223,7 +1212,7 @@ export class WorkflowBlockHandler implements BlockHandler { // needed to join the child's own run at read time. ...traceHandle, }), - failureKind + error ) } diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index 5ac9f360734..74a2369f0ee 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -3,13 +3,14 @@ import { DrizzleQueryError } from 'drizzle-orm/errors' import { describe, expect, it } from 'vitest' import { classifyFailure, + inheritFailureMarks, logFailureOnce, - markDeliberateFailure, markFailureKind, markFailureLogged, wasFailureLogged, } from '@/lib/core/errors/failure-log' import { RetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' +import { UserFailure } from '@/lib/core/errors/user-failure' import { HostedKeyRateLimitedError, HostedKeyUnavailableError } from '@/tools/errors' describe('classifyFailure', () => { @@ -56,14 +57,17 @@ describe('classifyFailure', () => { }) }) -describe('markDeliberateFailure', () => { - it('attributes a plain Error but not a TypeError raised by a bug in the same code', () => { - expect(classifyFailure(markDeliberateFailure(new Error('channel_not_found'), 'user'))).toBe( - 'user' - ) - expect(classifyFailure(markDeliberateFailure(new TypeError('x is undefined'), 'user'))).toBe( - 'internal' - ) +describe('inheritFailureMarks', () => { + it('carries the logged mark and attribution across a boundary that drops cause', () => { + const logged = new Error('child failed') + markFailureLogged(logged) + const boundary = inheritFailureMarks(new Error('Custom block execution failed'), logged) + expect(wasFailureLogged(boundary)).toBe(true) + expect(classifyFailure(boundary)).toBe('internal') + + const authored = inheritFailureMarks(new Error('wrapped'), new UserFailure('missing input')) + expect(wasFailureLogged(authored)).toBe(false) + expect(classifyFailure(authored)).toBe('user') }) }) @@ -83,13 +87,9 @@ describe('wasFailureLogged', () => { it('scopes a raw value logged at an execution boundary to that execution only', () => { const persistentFault = new Error('module failed to load') - logFailureOnce( - createLogger('FailureLogTest'), - 'Execution failed', - persistentFault, - {}, - 'exec-1' - ) + logFailureOnce(createLogger('FailureLogTest'), 'Execution failed', persistentFault, { + executionId: 'exec-1', + }) expect(wasFailureLogged(persistentFault, 'exec-1')).toBe(true) expect(wasFailureLogged(persistentFault, 'exec-2')).toBe(false) diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 73c8acaf922..a50409b876e 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -1,6 +1,7 @@ import type { Logger } from '@sim/logger' import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' +import { UserFailure } from '@/lib/core/errors/user-failure' import { HttpError } from '@/lib/core/utils/http-error' /** @@ -55,27 +56,10 @@ export function markFailureKind(error: T, kind: FailureKind): T { return error } -/** - * Records `kind` only when `error` is a plain `Error` deliberately thrown with a message. A - * `TypeError`, `RangeError`, or other subclass raised by a bug in the same code stays - * unattributed, so it keeps logging at error. - */ -export function markDeliberateFailure(error: T, kind: FailureKind): T { - if (error instanceof Error && Object.getPrototypeOf(error) === Error.prototype) { - failureKinds.set(error, kind) - } - return error -} - -/** A provider refusing the key it was given; Sim's fault when the key was Sim's own. */ -export function isProviderKeyRejection(status: unknown): boolean { - return status === 401 || status === 402 || status === 403 -} - /** * Attributes `error` from its cause chain. A database failure anywhere is always internal, - * then the outermost link with an explicit mark, a Sim `HttpError` status, or an upstream - * `status` decides. Anything unattributed is internal. + * then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError` status, + * or an upstream `status` decides. Anything unattributed is internal. */ export function classifyFailure(error: unknown): FailureKind { if (findDatabaseQueryError(error)) return 'internal' @@ -85,6 +69,7 @@ export function classifyFailure(error: unknown): FailureKind { const marked = failureKinds.get(link) if (marked) return marked + if (link instanceof UserFailure) return 'user' if (link instanceof HttpError) { return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal' @@ -108,17 +93,26 @@ export function markFailureLogged(carrier: unknown): void { } /** - * Moves a failed tool result's marks onto the error a handler rebuilds from it. `executeTool` - * flattens a thrown failure into `{ success: false, output }` and logs it once; without this, the - * handler's fresh error looks unlogged and unattributed and is logged again at error. + * Carries `source`'s marks onto `target`, a fresh error built at a boundary that drops `cause`: + * the logged mark when `source` was logged, and `source`'s attribution unless `target` has its own. + */ +export function inheritFailureMarks(target: T, source: unknown): T { + if (!isKeyable(target)) return target + if (wasFailureLogged(source)) loggedCarriers.add(target) + if (!failureKinds.has(target)) failureKinds.set(target, classifyFailure(source)) + return target +} + +/** + * Carries a failed tool result's marks onto the error a handler rebuilds from it. `executeTool` + * flattens a thrown failure into `{ success: false, output }`, logs it once, and marks `output`; + * without this, the handler's fresh error looks unlogged and is logged again at error. */ export function adoptToolFailure(error: T, result: { output?: unknown }): T { const output = result.output - if (!isKeyable(error) || !isKeyable(output)) return error - if (loggedCarriers.has(output)) loggedCarriers.add(error) - const kind = failureKinds.get(output) - if (kind && !failureKinds.has(error)) failureKinds.set(error, kind) - return error + return isKeyable(output) && loggedCarriers.has(output) + ? inheritFailureMarks(error, output) + : error } /** @@ -140,21 +134,35 @@ const LOG_LEVEL_BY_KIND = { internal: 'error', } as const satisfies Record +interface LogFailureOptions { + /** Built only when the line is logged, so a skipped boundary does no projection work. */ + metadata?: Record | (() => Record) + /** + * Set at execution-level boundaries (engine, execution core, trigger surfaces): the raw value is + * marked for this execution only, and the id is added to the line. Below that level the caller + * marks the fresh carrier it throws with {@link markFailureLogged}. + */ + executionId?: string +} + /** * Logs a failure at the severity its cause earns unless a boundary closer to the cause already - * logged it. Pass `executionId` at execution-level boundaries (engine, execution core, trigger - * surfaces) so the raw value is marked for that execution only; below that level the caller - * marks the fresh carrier it throws with {@link markFailureLogged}. + * logged it. Returns the attribution it logged with, or `undefined` when it skipped. */ export function logFailureOnce( logger: Logger, message: string, error: unknown, - metadata: Record = {}, - executionId?: string -): void { - if (wasFailureLogged(error, executionId)) return + { metadata, executionId }: LogFailureOptions = {} +): FailureKind | undefined { + if (wasFailureLogged(error, executionId)) return undefined const failureKind = classifyFailure(error) - logger[LOG_LEVEL_BY_KIND[failureKind]](message, { ...metadata, failureKind }) + const fields = typeof metadata === 'function' ? metadata() : metadata + logger[LOG_LEVEL_BY_KIND[failureKind]](message, { + ...(executionId !== undefined ? { executionId } : {}), + ...fields, + failureKind, + }) if (executionId !== undefined && isKeyable(error)) loggedInExecution.set(error, executionId) + return failureKind } diff --git a/apps/sim/lib/core/errors/user-failure.ts b/apps/sim/lib/core/errors/user-failure.ts new file mode 100644 index 00000000000..7a881d777b0 --- /dev/null +++ b/apps/sim/lib/core/errors/user-failure.ts @@ -0,0 +1,6 @@ +/** + * A failure the workflow author caused and can fix: a missing required field, a block that is not + * deployed, a call chain nested too deep. `classifyFailure` attributes any subclass to the author, + * so it logs at info wherever it surfaces. Dependency-free so client-reachable code can throw it. + */ +export class UserFailure extends Error {} diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 0d96d5d1d81..316699637bc 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -958,12 +958,14 @@ async function executeInSandboxWithinBudget( if (execution.error) { const errorMessage = `${execution.error.name}: ${execution.error.value}` - /** The author's code raised; only a provider-side failure is ours to look at. */ - logger[execution.providerFailure ? 'error' : 'info']('Sandbox execution failed', { - sandboxId, - hasTraceback: Boolean(execution.error.traceback), - providerFailure: execution.providerFailure, - }) + /** The author's code raising is logged once by the tool boundary; a provider failure is ours. */ + if (execution.providerFailure) { + logger.error('Sandbox execution failed', { + sandboxId, + hasTraceback: Boolean(execution.error.traceback), + providerFailure: execution.providerFailure, + }) + } const executionResult = { result: null, stdout: execution.error.traceback || errorMessage, @@ -1155,12 +1157,14 @@ async function executeShellInSandboxWithinBudget( // back to stdout for the real command output before the generic message. const errorMessage = result.stderr || result.stdout || `Process exited with code ${result.exitCode}` - /** The author's command exited non-zero; only a provider-side failure is ours to look at. */ - logger[result.providerFailure ? 'error' : 'info']('Sandbox shell execution error', { - sandboxId, - exitCode: result.exitCode, - providerFailure: result.providerFailure, - }) + /** A non-zero exit is logged once by the tool boundary; a provider failure is ours. */ + if (result.providerFailure) { + logger.error('Sandbox shell execution error', { + sandboxId, + exitCode: result.exitCode, + providerFailure: result.providerFailure, + }) + } const executionResult = { result: null, stdout, diff --git a/apps/sim/lib/function-execution/execute-request.ts b/apps/sim/lib/function-execution/execute-request.ts index 476018bd8d0..05584300c7a 100644 --- a/apps/sim/lib/function-execution/execute-request.ts +++ b/apps/sim/lib/function-execution/execute-request.ts @@ -3301,17 +3301,15 @@ export async function executeFunctionRequest( errorDisplayCode ) - /** The author's code threw unless the isolate itself failed. */ - logger[isSystemError ? 'error' : 'info']( - `[${requestId}] Function execution failed in isolated-vm`, - { + /** The author's code throwing is logged once by the tool boundary; an isolate failure is ours. */ + if (isSystemError) { + logger.error(`[${requestId}] Function execution failed in isolated-vm`, { executionTime, - isSystemError, hasStack: Boolean(ivmError.stack), line: enhancedError.line, column: enhancedError.column, - } - ) + }) + } return functionJsonResponse( { diff --git a/apps/sim/lib/internal/tool-operations/execute-json-operation.ts b/apps/sim/lib/internal/tool-operations/execute-json-operation.ts index f31b6ba769b..0809f7d11be 100644 --- a/apps/sim/lib/internal/tool-operations/execute-json-operation.ts +++ b/apps/sim/lib/internal/tool-operations/execute-json-operation.ts @@ -24,7 +24,7 @@ export async function executeInternalJsonToolOperation | null = null diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index ca991b81f40..e5e1a8780fe 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -14,7 +14,8 @@ import type { Edge } from '@xyflow/react' import { eq } from 'drizzle-orm' import { z } from 'zod' import { type EffectivePiiRedaction, resolveEffectivePiiRedaction } from '@/lib/billing/retention' -import { logFailureOnce, markFailureKind } from '@/lib/core/errors/failure-log' +import { logFailureOnce } from '@/lib/core/errors/failure-log' +import { UserFailure } from '@/lib/core/errors/user-failure' import { getExecutionDeadlineAt, getTimeoutErrorMessage, @@ -75,8 +76,6 @@ import { buildParallelSentinelEndId, } from '@/executor/utils/subflow-node-id-codec' import { Serializer } from '@/serializer' -import { MissingRequiredFieldsError } from '@/serializer/errors' -import type { SerializedWorkflow } from '@/serializer/types' const logger = createLogger('ExecutionCore') @@ -877,10 +876,7 @@ async function executeWorkflowCoreImpl( const startBlock = TriggerUtils.findStartBlock(mergedStates, executionKind, false) if (!startBlock) { - throw markFailureKind( - new Error('No start block found. Add a start block to this workflow.'), - 'user' - ) + throw new UserFailure('No start block found. Add a start block to this workflow.') } resolvedTriggerBlockId = startBlock.blockId @@ -892,21 +888,13 @@ async function executeWorkflowCoreImpl( } // Serialize workflow - let serializedWorkflow: SerializedWorkflow - try { - serializedWorkflow = new Serializer().serializeWorkflow( - mergedStates, - filteredEdges, - loops, - parallels, - true - ) - } catch (serializeError) { - /** An unknown block type stays internal: a registry regression or an unregistered block. */ - throw serializeError instanceof MissingRequiredFieldsError - ? markFailureKind(serializeError, 'user') - : serializeError - } + const serializedWorkflow = new Serializer().serializeWorkflow( + mergedStates, + filteredEdges, + loops, + parallels, + true + ) const inputFileKeys = new Set() processedInput = resumeFromSnapshot || runFromBlock @@ -1358,18 +1346,16 @@ async function executeWorkflowCoreImpl( return result } catch (error: unknown) { - const errorCause = describeErrorCause(error) - logFailureOnce( - logger, - `[${requestId}] Execution failed:`, - error, - projectResolvedSecretDiagnosticError(error, resolvedSecretTraceRegistry, { - workflowId, - executionId, - ...(errorCause ? { cause: errorCause } : {}), - }), - executionId - ) + logFailureOnce(logger, `[${requestId}] Execution failed:`, error, { + metadata: () => { + const errorCause = describeErrorCause(error) + return projectResolvedSecretDiagnosticError(error, resolvedSecretTraceRegistry, { + workflowId, + ...(errorCause ? { cause: errorCause } : {}), + }) + }, + executionId, + }) await waitForLifecycleCallbacks() diff --git a/apps/sim/serializer/errors.ts b/apps/sim/serializer/errors.ts index 381f82601b7..657f4dc7eec 100644 --- a/apps/sim/serializer/errors.ts +++ b/apps/sim/serializer/errors.ts @@ -1,8 +1,7 @@ -/** - * A block the author left without a value it requires. Raised before execution starts; the - * author's configuration, not a Sim fault, so execution logs it at info. - */ -export class MissingRequiredFieldsError extends Error { +import { UserFailure } from '@/lib/core/errors/user-failure' + +/** A block the author left without a value it requires, refused before execution starts. */ +export class MissingRequiredFieldsError extends UserFailure { constructor(blockName: string, missingFields: string[]) { super(`${blockName} is missing required fields: ${missingFields.join(', ')}`) this.name = 'MissingRequiredFieldsError' diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 12e4600beb6..d7d408c828c 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -19,9 +19,7 @@ import { isHosted } from '@/lib/core/config/env-flags' import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' import { classifyFailure, - isProviderKeyRejection, logFailureOnce, - markDeliberateFailure, markFailureKind, markFailureLogged, } from '@/lib/core/errors/failure-log' @@ -2314,44 +2312,47 @@ async function executeToolImplementation( const databaseQueryError = findDatabaseQueryError(error) const databaseErrorCause = databaseQueryError ? describeError(error) : undefined const upstreamStatus = (error as { status?: unknown } | null)?.status + const hostedKeyFailure = hostedKeyForMetrics ? classifyHostedKeyFailure(error) : undefined /** Sim's own hosted key being refused or throttled is Sim's fault and Sim's capacity. */ - if (hostedKeyForMetrics && (isProviderKeyRejection(upstreamStatus) || upstreamStatus === 429)) { - markFailureKind(error, 'internal') - } + if (hostedKeyFailure && hostedKeyFailure !== 'other') markFailureKind(error, 'internal') const toolContext = params._context as Record | undefined - logFailureOnce(logger, `[${requestId}] Error executing tool ${toolId}:`, error, { - toolId, - workflowId: executionContext?.workflowId ?? undefined, - executionId: executionContext?.executionId, - blockId: typeof toolContext?.blockId === 'string' ? toolContext.blockId : undefined, - ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), - ...projectToolLogMetadata( - { - ...(databaseErrorCause - ? { cause: databaseErrorCause } - : { - error: normalizedError.message, - stack: error instanceof Error ? error.stack : undefined, - ...(error instanceof Error && externalHttpFailures.has(error) - ? { errorData: (error as { data?: unknown }).data } - : {}), - }), - }, - resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - hasStack: !databaseErrorCause && Boolean(error instanceof Error && error.stack), - ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), - }, - structuralOnlyToolLogs - ), - }) + const loggedKind = logFailureOnce( + logger, + `[${requestId}] Error executing tool ${toolId}:`, + error, + { + metadata: () => ({ + toolId, + workflowId: executionContext?.workflowId ?? undefined, + executionId: executionContext?.executionId, + blockId: typeof toolContext?.blockId === 'string' ? toolContext.blockId : undefined, + ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), + ...projectToolLogMetadata( + { + ...(databaseErrorCause + ? { cause: databaseErrorCause } + : { + error: normalizedError.message, + stack: error instanceof Error ? error.stack : undefined, + ...(error instanceof Error && externalHttpFailures.has(error) + ? { errorData: (error as { data?: unknown }).data } + : {}), + }), + }, + resolvedSecretTraceRegistry, + { + errorName: normalizedError.name, + hasStack: !databaseErrorCause && Boolean(error instanceof Error && error.stack), + ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), + }, + structuralOnlyToolLogs + ), + }), + } + ) - if (hostedKeyForMetrics) { - hostedKeyMetrics.recordFailed({ - ...hostedKeyForMetrics, - reason: classifyHostedKeyFailure(error), - }) + if (hostedKeyForMetrics && hostedKeyFailure) { + hostedKeyMetrics.recordFailed({ ...hostedKeyForMetrics, reason: hostedKeyFailure }) } let errorMessage = 'Unknown error occurred' @@ -2429,7 +2430,7 @@ async function executeToolImplementation( } /** A handler rebuilding this result as a thrown error carries `output`, and with it both marks. */ markFailureLogged(failureOutput) - markFailureKind(failureOutput, classifyFailure(error)) + markFailureKind(failureOutput, loggedKind ?? classifyFailure(error)) return { success: false, output: failureOutput, @@ -3087,7 +3088,10 @@ async function executeToolRequest( try { data = await tool.transformResponse(mockResponse, params, { signal }) } catch (transformError) { - throw markDeliberateFailure(transformError, 'third_party_client') + throw transformError instanceof Error && + Object.getPrototypeOf(transformError) === Error.prototype + ? markFailureKind(transformError, 'third_party_client') + : transformError } if (tool.request.responseType === 'binary' && data.success) { if (!context) throw new Error('Binary file output requires trusted execution context') From 7569afb80b8b3a6a88165a5f1c845eade626d0c4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:30:10 -0700 Subject: [PATCH 05/10] test(logs): attribute the serializer refusal at its owner and pin block log redaction Moves the missing-required-fields attribution check to the serializer that throws it, drops an execution-core case whose internal row passed by default, and covers the block failure line's secret projection now that it, not the Agent handler, logs provider errors. Tightens comments the diff added. --- .../executor/execution/block-executor.test.ts | 42 +++++++++++++++++++ apps/sim/executor/execution/block-executor.ts | 4 +- .../executor/handlers/agent/agent-handler.ts | 1 - .../human-in-the-loop-handler.ts | 1 - .../handlers/workflow/workflow-handler.ts | 1 - apps/sim/lib/core/errors/failure-log.ts | 6 +-- .../workflows/executor/execution-core.test.ts | 28 ------------- apps/sim/serializer/index.test.ts | 12 +++++- apps/sim/tools/index.ts | 4 +- 9 files changed, 59 insertions(+), 40 deletions(-) diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index 6c0f5d59bed..df9b398c70d 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -622,6 +622,48 @@ describe('BlockExecutor', () => { expect(blockFailureLogsSince(loggerIndex)).toHaveLength(2) }) + it.each([ + ['projects secrets and runtime identifiers out of', false], + ['fails closed to a structural', true], + ] as const)('%s the block failure line for a provider error', async (_name, incomplete) => { + const secret = 'block-failure-secret' + /** The Agent handler hands its provider error registry to the block executor this way. */ + const errorRegistry = new ResolvedSecretTraceRegistry([ + { name: 'TOKEN', plaintext: secret, encryptedValue: 'encrypted-block-failure-secret' }, + ]) + errorRegistry.recordResolved('TOKEN', secret) + if (incomplete) errorRegistry.markIncomplete('unspecified') + const block = createBlock() + const workflow: SerializedWorkflow = { + version: '1', + blocks: [block], + connections: [], + loops: {}, + parallels: {}, + } + const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length + const state = new ExecutionState() + const handler: BlockHandler = { + canHandle: () => true, + execute: async (ctx) => { + ctx.errorResolvedSecretTraceRegistry = errorRegistry + throw new Error(`provider failed with ${secret} __var_TOKEN __sim_runtime_test_1`) + }, + } + const executor = new BlockExecutor( + [handler], + new VariableResolver(workflow, {}, state), + {}, + state + ) + + await executor.execute(createContext(state), createNode(block), block).catch(() => undefined) + + const logged = JSON.stringify(blockFailureLogsSince(loggerIndex)) + expect(logged).toContain('Block execution failed') + for (const leaked of [secret, '__var_', '__sim_']) expect(logged).not.toContain(leaked) + }) + it('logs an internal child workflow fault with the block and run identity', async () => { const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index c8bd6a8707e..088118c3434 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -789,7 +789,7 @@ export class BlockExecutor { } } - /** Projected only when logged: a tool failure arrives already logged and skips it. */ + /** Lazy, so a failure a tool already logged skips the secret projection. */ let errorDiagnostic: Record | undefined const getErrorDiagnostic = () => { if (errorDiagnostic) return errorDiagnostic @@ -887,7 +887,7 @@ export class BlockExecutor { executionTime: duration, }, }) - /** A thrown primitive has no `cause` link back to the value logged above. */ + /** The raw thrown value is never marked logged, so the fresh block error carries the mark. */ markFailureLogged(blockError) throw blockError } diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index ac9521d0b94..532c3bb7f36 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -327,7 +327,6 @@ function describeProviderTransportFailure(error: Error): string | null { return null } -/** A provider refusing the key it was given. */ function isProviderKeyRejection(error: unknown): boolean { const status = (error as { status?: unknown } | null)?.status return status === 401 || status === 402 || status === 403 diff --git a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts index c929f82c1d8..66d98a7431e 100644 --- a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts +++ b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts @@ -560,7 +560,6 @@ export class HumanInTheLoopBlockHandler implements BlockHandler { const result = await executeTool(toolId, toolParams, { executionContext: ctx }) const durationMs = Date.now() - startTime - /** `executeTool` already logged the failure with its tool id and attribution. */ if (!result.success) { return { toolId, diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 78fc360071a..6cd0351a677 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -1200,7 +1200,6 @@ export class WorkflowBlockHandler implements BlockHandler { ? `Custom block execution failed (ref: ${ref})` : 'Custom block execution failed' - /** No `cause` crosses the boundary, so the failure's marks are carried explicitly. */ return inheritFailureMarks( new ChildWorkflowError({ message, diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index a50409b876e..3425f13521d 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -57,9 +57,9 @@ export function markFailureKind(error: T, kind: FailureKind): T { } /** - * Attributes `error` from its cause chain. A database failure anywhere is always internal, - * then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError` status, - * or an upstream `status` decides. Anything unattributed is internal. + * Attributes `error` from its cause chain. A database or retryable setup failure is always + * internal, then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError` + * status, or an upstream `status` decides. Anything unattributed is internal. */ export function classifyFailure(error: unknown): FailureKind { if (findDatabaseQueryError(error)) return 'internal' diff --git a/apps/sim/lib/workflows/executor/execution-core.test.ts b/apps/sim/lib/workflows/executor/execution-core.test.ts index 543a557c01c..ca2f5ec5c2d 100644 --- a/apps/sim/lib/workflows/executor/execution-core.test.ts +++ b/apps/sim/lib/workflows/executor/execution-core.test.ts @@ -160,13 +160,11 @@ vi.mock('@/serializer', () => ({ }, })) -import { classifyFailure } from '@/lib/core/errors/failure-log' import { executeWorkflowCore, FINALIZED_EXECUTION_ID_TTL_MS, wasExecutionFinalizedByCore, } from '@/lib/workflows/executor/execution-core' -import { MissingRequiredFieldsError } from '@/serializer/errors' const uploadWorkflowInputMock = uploadsExecutionMockFns.mockUploadExecutionFile largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true) @@ -703,32 +701,6 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { ) }) - it.each([ - [ - 'a block missing a required field', - new MissingRequiredFieldsError('Slack', ['Slack Account']), - 'user', - ], - [ - 'an unknown block type, a registry regression', - new Error('Invalid block type: retired'), - 'internal', - ], - ] as const)('attributes a serializer refusal for %s', async (_name, refusal, kind) => { - serializeWorkflowMock.mockImplementationOnce(() => { - throw refusal - }) - - const thrown = await executeWorkflowCore({ - snapshot: createSnapshot() as any, - callbacks: {}, - loggingSession: loggingSession as any, - }).catch((error: unknown) => error) - - expect(thrown).toBe(refusal) - expect(classifyFailure(thrown)).toBe(kind) - }) - it('activates trusted pre-execution provenance on the installed execution registry', async () => { executorExecuteMock.mockResolvedValue({ success: true, diff --git a/apps/sim/serializer/index.test.ts b/apps/sim/serializer/index.test.ts index e85f9325295..551bfb76c33 100644 --- a/apps/sim/serializer/index.test.ts +++ b/apps/sim/serializer/index.test.ts @@ -23,6 +23,7 @@ import { toolsUtilsMock, } from '@sim/testing/mocks' import { describe, expect, it, vi } from 'vitest' +import { classifyFailure } from '@/lib/core/errors/failure-log' import { DAGBuilder } from '@/executor/dag/builder' import { Serializer } from '@/serializer/index' import { getToolMetadata, getToolParams } from '@/tools/metadata' @@ -356,7 +357,8 @@ describe('Serializer', () => { enabled: true, } - expect(() => { + let refusal: unknown + try { serializer.serializeWorkflow( { 'test-block': blockWithMissingUserOnlyField }, [], @@ -364,7 +366,13 @@ describe('Serializer', () => { undefined, true ) - }).toThrow('Test Jina Block is missing required fields: API Key') + } catch (error) { + refusal = error + } + expect(refusal).toBeInstanceOf(Error) + expect((refusal as Error).message).toBe('Test Jina Block is missing required fields: API Key') + /** The author's configuration, so execution logs it at info rather than paging at error. */ + expect(classifyFailure(refusal)).toBe('user') }) it.concurrent('should not validate user-or-llm fields during serialization', () => { diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index d7d408c828c..9b85269af54 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -1060,7 +1060,7 @@ const RESPONSE_SIZE_LIMIT_ERROR_MESSAGE = const SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE = 'External integration tools cannot target this Sim instance; use an internal operation' -/** The author's workflow data is too large to send; their fault, logged once by the tool catch. */ +/** The author's data is too large to send, so it is a user failure. */ function bodySizeLimitError(): Error { return markFailureKind(new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE), 'user') } @@ -2428,7 +2428,7 @@ async function executeToolImplementation( ...errorDetails, ...(functionSandboxCost ? { cost: functionSandboxCost } : {}), } - /** A handler rebuilding this result as a thrown error carries `output`, and with it both marks. */ + /** Lets `adoptToolFailure` carry both marks onto a handler's rebuilt error. */ markFailureLogged(failureOutput) markFailureKind(failureOutput, loggedKind ?? classifyFailure(error)) return { From 2f93553cd5a383f57e9ac054083c90334397c93f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:52:14 -0700 Subject: [PATCH 06/10] improvement(logs): drop casts and test-only exports from the failure log Reads status and cause without casts, keeps the thrown missing-required-fields error's name and stack unchanged, ignores an empty execution id when scoping a logged mark, unexports wasFailureLogged and FailureKind (tests observe the outer boundary through logFailureOnce instead), and shrinks the explicit-any baseline for the Agent handler. --- .../executor/execution/block-executor.test.ts | 5 +-- .../executor/execution/failure-trace.test.ts | 5 +-- .../executor/handlers/agent/agent-handler.ts | 4 +-- .../workflow/workflow-handler.test.ts | 5 +-- .../handlers/workflow/workflow-handler.ts | 2 +- apps/sim/lib/core/errors/failure-log.test.ts | 33 ++++++++++--------- apps/sim/lib/core/errors/failure-log.ts | 17 +++++----- apps/sim/serializer/errors.ts | 1 - apps/sim/serializer/index.test.ts | 6 ++-- apps/sim/tools/index.test.ts | 7 ++-- apps/sim/tools/index.ts | 10 +++--- scripts/check-explicit-any.baseline.json | 2 +- 12 files changed, 53 insertions(+), 44 deletions(-) diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index df9b398c70d..90c158957ab 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { loggerMock } from '@sim/testing' import { maskClientMock, maskClientMockFns } from '@sim/testing/mocks/mask-client.mock' import { permissionCheckMock } from '@sim/testing/mocks/permission-check.mock' @@ -5,7 +6,7 @@ import { storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' import { uploadsMock } from '@sim/testing/mocks/uploads.mock' import { DrizzleQueryError } from 'drizzle-orm/errors' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log' import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache' import { createLargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest' import { isLargeValueRef } from '@/lib/execution/payloads/large-value-ref' @@ -500,7 +501,7 @@ describe('BlockExecutor', () => { expect(JSON.stringify(logged)).not.toContain('owner-secret-id') /** Logged here at error, so the engine and the run surfaces must see it as already logged. */ expect(classifyFailure(thrown)).toBe('internal') - expect(wasFailureLogged(thrown)).toBe(true) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined() }) it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => { diff --git a/apps/sim/executor/execution/failure-trace.test.ts b/apps/sim/executor/execution/failure-trace.test.ts index d76ed6884a9..3d57fa547d8 100644 --- a/apps/sim/executor/execution/failure-trace.test.ts +++ b/apps/sim/executor/execution/failure-trace.test.ts @@ -1,6 +1,7 @@ +import { createLogger } from '@sim/logger' import { permissionCheckMock } from '@sim/testing/mocks/permission-check.mock' import { describe, expect, it, vi } from 'vitest' -import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' import { DAGExecutor } from '@/executor/execution/executor' import { hasExecutionResult } from '@/executor/utils/errors' @@ -87,7 +88,7 @@ describe('failed run trace', () => { * The block executor logged it; the block wrap and the engine's rethrow must keep that * visible so execution-core and the trigger surfaces do not log it again. */ - expect(wasFailureLogged(thrown)).toBe(true) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined() expect(classifyFailure(thrown)).toBe('user') }) }) diff --git a/apps/sim/executor/handlers/agent/agent-handler.ts b/apps/sim/executor/handlers/agent/agent-handler.ts index 532c3bb7f36..c35b521d580 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' -import { isPlainRecord, omit } from '@sim/utils/object' +import { isPlainRecord, isRecordLike, omit } from '@sim/utils/object' import { truncate } from '@sim/utils/string' import { markFailureKind } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' @@ -328,7 +328,7 @@ function describeProviderTransportFailure(error: Error): string | null { } function isProviderKeyRejection(error: unknown): boolean { - const status = (error as { status?: unknown } | null)?.status + const status = isRecordLike(error) ? error.status : undefined return status === 401 || status === 402 || status === 403 } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 9bf190252c6..02245ba2594 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { encryptionMockFns, environmentUtilsMockFns, resetEnvironmentUtilsMock } from '@sim/testing' import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' import { authInternalMock, authInternalMockFns } from '@sim/testing/mocks/auth-internal.mock' @@ -18,7 +19,7 @@ import { import { permissionsMock } from '@sim/testing/mocks/permissions.mock' import { usersQueriesMock, usersQueriesMockFns } from '@sim/testing/mocks/users-queries.mock' import { afterAll, beforeAll, beforeEach, describe, expect, it, type Mock, vi } from 'vitest' -import { classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log' import { createTimeoutAbortController, getExecutionDeadlineAt } from '@/lib/core/execution-limits' import { OrchestrationError } from '@/lib/core/orchestration/types' import { getBlock } from '@/blocks/registry' @@ -382,7 +383,7 @@ describe('WorkflowBlockHandler', () => { expect(thrown).toBeInstanceOf(Error) /** Logged here, the block executor would skip its log carrying block, run, and stack. */ - expect(wasFailureLogged(thrown)).toBe(false) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBe('internal') expect(classifyFailure(thrown)).toBe('internal') }) diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 6cd0351a677..d668a6f2e15 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -1575,7 +1575,7 @@ export class WorkflowBlockHandler implements BlockHandler { childWorkflowSnapshotId, childWorkflowInstanceId: instanceId, }) - /** The child run logged its own failure; the warning above names it for this run. */ + /** The warning above is this failure's log line. */ markFailureLogged(childFailure) throw childFailure } diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index 74a2369f0ee..da90f8ee1af 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -7,12 +7,18 @@ import { logFailureOnce, markFailureKind, markFailureLogged, - wasFailureLogged, } from '@/lib/core/errors/failure-log' import { RetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' import { UserFailure } from '@/lib/core/errors/user-failure' import { HostedKeyRateLimitedError, HostedKeyUnavailableError } from '@/tools/errors' +const logger = createLogger('FailureLogTest') + +/** What an outer boundary does with the failure: the kind it logs at, or undefined when it skips. */ +function outerBoundary(error: unknown, executionId?: string) { + return logFailureOnce(logger, 'probe', error, { executionId }) +} + describe('classifyFailure', () => { it('keeps a database failure internal even beneath a user mark', () => { const queryError = new DrizzleQueryError('select 1', [], new Error('connection reset')) @@ -52,8 +58,7 @@ describe('classifyFailure', () => { const first = new Error('first') const second = new Error('second', { cause: first }) Object.assign(first, { cause: second }) - expect(classifyFailure(first)).toBe('internal') - expect(wasFailureLogged(first)).toBe(false) + expect(outerBoundary(first)).toBe('internal') }) }) @@ -62,37 +67,35 @@ describe('inheritFailureMarks', () => { const logged = new Error('child failed') markFailureLogged(logged) const boundary = inheritFailureMarks(new Error('Custom block execution failed'), logged) - expect(wasFailureLogged(boundary)).toBe(true) + expect(outerBoundary(boundary)).toBeUndefined() expect(classifyFailure(boundary)).toBe('internal') const authored = inheritFailureMarks(new Error('wrapped'), new UserFailure('missing input')) - expect(wasFailureLogged(authored)).toBe(false) - expect(classifyFailure(authored)).toBe('user') + expect(outerBoundary(authored)).toBe('user') }) }) -describe('wasFailureLogged', () => { +describe('logFailureOnce', () => { it('marks a frozen error, which a property write would throw on', () => { const frozen = Object.freeze(new Error('frozen')) markFailureLogged(frozen) markFailureKind(frozen, 'user') - expect(wasFailureLogged(frozen)).toBe(true) + expect(outerBoundary(frozen)).toBeUndefined() expect(classifyFailure(frozen)).toBe('user') }) it('does not treat an unrelated error as logged', () => { markFailureLogged(new Error('logged elsewhere')) - expect(wasFailureLogged(new Error('fresh'))).toBe(false) + expect(outerBoundary(new Error('fresh'))).toBe('internal') }) it('scopes a raw value logged at an execution boundary to that execution only', () => { const persistentFault = new Error('module failed to load') - logFailureOnce(createLogger('FailureLogTest'), 'Execution failed', persistentFault, { - executionId: 'exec-1', - }) + expect(outerBoundary(persistentFault, 'exec-1')).toBe('internal') - expect(wasFailureLogged(persistentFault, 'exec-1')).toBe(true) - expect(wasFailureLogged(persistentFault, 'exec-2')).toBe(false) - expect(wasFailureLogged(persistentFault)).toBe(false) + expect(outerBoundary(persistentFault, 'exec-1')).toBeUndefined() + expect(outerBoundary(persistentFault, 'exec-2')).toBe('internal') + expect(outerBoundary(persistentFault)).toBe('internal') + expect(outerBoundary(persistentFault, '')).toBe('internal') }) }) diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 3425f13521d..2ad53f7dda5 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -16,9 +16,9 @@ import { HttpError } from '@/lib/core/utils/http-error' * in-process operations are attributed explicitly at their boundary and never land here. * - `internal`: Sim's own fault, or a cause nothing attributed. Always logged at error. */ -export type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal' +type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal' -/** Mirrors the cause depth `describeError` and `readStatusCode` follow. */ +/** Same bound `readStatusCode` puts on its cause walk. */ const MAX_CAUSE_DEPTH = 8 /** @@ -45,7 +45,7 @@ function causeChain(error: unknown): object[] { let current = error while (isKeyable(current) && chain.length < MAX_CAUSE_DEPTH && !chain.includes(current)) { chain.push(current) - current = (current as { cause?: unknown }).cause + current = 'cause' in current ? current.cause : undefined } return chain } @@ -75,7 +75,7 @@ export function classifyFailure(error: unknown): FailureKind { return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal' } - const status = (link as { status?: unknown }).status + const status = 'status' in link ? link.status : undefined if (typeof status === 'number' && status >= 400 && status < 600) { return status >= 500 ? 'third_party_server' : 'third_party_client' } @@ -119,11 +119,11 @@ export function adoptToolFailure(error: T, result: { output?: unknown }): T { * Whether a boundary already logged this failure: a logged carrier anywhere in the cause chain, * or, given `executionId`, a raw value that boundary logged during this same execution. */ -export function wasFailureLogged(error: unknown, executionId?: string): boolean { +function wasFailureLogged(error: unknown, executionId?: string): boolean { return causeChain(error).some( (link) => loggedCarriers.has(link) || - (executionId !== undefined && loggedInExecution.get(link) === executionId) + (Boolean(executionId) && loggedInExecution.get(link) === executionId) ) } @@ -159,10 +159,11 @@ export function logFailureOnce( const failureKind = classifyFailure(error) const fields = typeof metadata === 'function' ? metadata() : metadata logger[LOG_LEVEL_BY_KIND[failureKind]](message, { - ...(executionId !== undefined ? { executionId } : {}), + ...(executionId ? { executionId } : {}), ...fields, failureKind, }) - if (executionId !== undefined && isKeyable(error)) loggedInExecution.set(error, executionId) + /** An empty id (a request that failed before minting one) scopes nothing. */ + if (executionId && isKeyable(error)) loggedInExecution.set(error, executionId) return failureKind } diff --git a/apps/sim/serializer/errors.ts b/apps/sim/serializer/errors.ts index 657f4dc7eec..e934af33677 100644 --- a/apps/sim/serializer/errors.ts +++ b/apps/sim/serializer/errors.ts @@ -4,6 +4,5 @@ import { UserFailure } from '@/lib/core/errors/user-failure' export class MissingRequiredFieldsError extends UserFailure { constructor(blockName: string, missingFields: string[]) { super(`${blockName} is missing required fields: ${missingFields.join(', ')}`) - this.name = 'MissingRequiredFieldsError' } } diff --git a/apps/sim/serializer/index.test.ts b/apps/sim/serializer/index.test.ts index 551bfb76c33..81cf7cc0bd1 100644 --- a/apps/sim/serializer/index.test.ts +++ b/apps/sim/serializer/index.test.ts @@ -369,8 +369,10 @@ describe('Serializer', () => { } catch (error) { refusal = error } - expect(refusal).toBeInstanceOf(Error) - expect((refusal as Error).message).toBe('Test Jina Block is missing required fields: API Key') + expect(refusal).toMatchObject({ + name: 'Error', + message: 'Test Jina Block is missing required fields: API Key', + }) /** The author's configuration, so execution logs it at info rather than paging at error. */ expect(classifyFailure(refusal)).toBe('user') }) diff --git a/apps/sim/tools/index.test.ts b/apps/sim/tools/index.test.ts index 455ec0964c7..a9d8386f09d 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { createSessionPrincipal } from '@sim/testing/factories/principal.factory' import { jsonResponse } from '@sim/testing/helpers/http' import { apiKeyByokMock, apiKeyByokMockFns } from '@sim/testing/mocks/api-key-byok.mock' @@ -420,7 +421,7 @@ vi.mock('@/tools/utils.server', async (importOriginal) => { }) import type { QueryClient } from '@tanstack/react-query' -import { adoptToolFailure, classifyFailure, wasFailureLogged } from '@/lib/core/errors/failure-log' +import { adoptToolFailure, classifyFailure, logFailureOnce } from '@/lib/core/errors/failure-log' import * as getQueryClientModule from '@/app/_shell/providers/get-query-client' import { ApiBlockHandler } from '@/executor/handlers/api/api-handler' import { buildBlockExecutionError } from '@/executor/utils/errors' @@ -5923,7 +5924,7 @@ describe('Centralized Error Handling', () => { async (status, kind) => { const blockError = await failApiBlock(jsonResponse({ error: 'rejected' }, status)) expect(classifyFailure(blockError)).toBe(kind) - expect(wasFailureLogged(blockError)).toBe(true) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined() } ) @@ -5942,7 +5943,7 @@ describe('Centralized Error Handling', () => { try { const blockError = await failApiBlock(jsonResponse({ ok: false })) expect(classifyFailure(blockError)).toBe(kind) - expect(wasFailureLogged(blockError)).toBe(true) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined() } finally { tools.http_request.transformResponse = originalTransform } diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 9b85269af54..217b31568d1 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger' import { isLoopbackIp, unwrapIpv6Brackets } from '@sim/security/ssrf' import { describeError, getErrorMessage, toError } from '@sim/utils/errors' import { sleep } from '@sim/utils/helpers' -import { isPlainRecord, isRecordLike } from '@sim/utils/object' +import { isPlainRecord, isRecordLike, toRecord } from '@sim/utils/object' import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry' import { ApiClientError } from '@/lib/api/client/errors' import { requestJson } from '@/lib/api/client/request' @@ -2311,11 +2311,11 @@ async function executeToolImplementation( const normalizedError = toError(error) const databaseQueryError = findDatabaseQueryError(error) const databaseErrorCause = databaseQueryError ? describeError(error) : undefined - const upstreamStatus = (error as { status?: unknown } | null)?.status + const upstreamStatus: unknown = error?.status const hostedKeyFailure = hostedKeyForMetrics ? classifyHostedKeyFailure(error) : undefined /** Sim's own hosted key being refused or throttled is Sim's fault and Sim's capacity. */ if (hostedKeyFailure && hostedKeyFailure !== 'other') markFailureKind(error, 'internal') - const toolContext = params._context as Record | undefined + const toolContext = toRecord(params._context) const loggedKind = logFailureOnce( logger, `[${requestId}] Error executing tool ${toolId}:`, @@ -2325,7 +2325,7 @@ async function executeToolImplementation( toolId, workflowId: executionContext?.workflowId ?? undefined, executionId: executionContext?.executionId, - blockId: typeof toolContext?.blockId === 'string' ? toolContext.blockId : undefined, + blockId: typeof toolContext.blockId === 'string' ? toolContext.blockId : undefined, ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), ...projectToolLogMetadata( { @@ -2335,7 +2335,7 @@ async function executeToolImplementation( error: normalizedError.message, stack: error instanceof Error ? error.stack : undefined, ...(error instanceof Error && externalHttpFailures.has(error) - ? { errorData: (error as { data?: unknown }).data } + ? { errorData: error.data } : {}), }), }, diff --git a/scripts/check-explicit-any.baseline.json b/scripts/check-explicit-any.baseline.json index faa6d583874..75c4908b83e 100644 --- a/scripts/check-explicit-any.baseline.json +++ b/scripts/check-explicit-any.baseline.json @@ -143,7 +143,7 @@ "apps/sim/executor/execution/state.ts": 3, "apps/sim/executor/execution/types.ts": 6, "apps/sim/executor/handlers/agent/agent-handler.test.ts": 1, - "apps/sim/executor/handlers/agent/agent-handler.ts": 31, + "apps/sim/executor/handlers/agent/agent-handler.ts": 30, "apps/sim/executor/handlers/agent/memory.test.ts": 8, "apps/sim/executor/handlers/agent/types.ts": 3, "apps/sim/executor/handlers/api/api-handler.ts": 3, From b82d21f1785579a503015ebf5d193435f57a872a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:57:36 -0700 Subject: [PATCH 07/10] fix(logs): close the remaining double logs found in review Carries failure marks through the scrubbed Pi error and normalizes a non-Error node failure before logging so the engine sees its mark; drops the condition handler's and response-size handler's own error lines in favor of the owning boundary; logs MCP failures once and marks their output; passes the block id to the API and Function tool calls so the tool line carries it; attributes custom tool parameter validation and condition expression errors to the author; checks the whole cause chain for a retryable setup failure before honoring a mark; and restores the sandbox failure logs, whose adapters cannot yet tell a provider exception from a non-zero exit. --- apps/sim/executor/execution/engine.ts | 8 +- apps/sim/executor/handlers/api/api-handler.ts | 1 + .../handlers/condition/condition-handler.ts | 13 +- .../handlers/function/function-handler.ts | 1 + .../executor/handlers/pi/core/redaction.ts | 8 +- apps/sim/lib/core/errors/failure-log.test.ts | 7 + apps/sim/lib/core/errors/failure-log.ts | 6 +- .../sim/lib/execution/remote-sandbox/index.ts | 24 +-- apps/sim/tools/index.ts | 173 ++++++++++-------- 9 files changed, 131 insertions(+), 110 deletions(-) diff --git a/apps/sim/executor/execution/engine.ts b/apps/sim/executor/execution/engine.ts index 48dbf220770..ffb20f432f9 100644 --- a/apps/sim/executor/execution/engine.ts +++ b/apps/sim/executor/execution/engine.ts @@ -482,15 +482,17 @@ export class ExecutionEngine { /** * Block failures were logged by the block executor. This catches a completion-handling * fault, which only this frame sees when a concurrent failure already won `executionError`. + * Normalized first, as `trackExecution` would, so `run()` sees the mark on the same object. */ - logFailureOnce(this.execLogger, 'Node execution failed', error, { + const failure = toError(error) + logFailureOnce(this.execLogger, 'Node execution failed', failure, { metadata: () => - projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry, { + projectResolvedSecretDiagnosticError(failure, this.context.resolvedSecretTraceRegistry, { nodeId, }), executionId: this.context.executionId, }) - throw error + throw failure } } diff --git a/apps/sim/executor/handlers/api/api-handler.ts b/apps/sim/executor/handlers/api/api-handler.ts index 87bf254930a..29a0ba43877 100644 --- a/apps/sim/executor/handlers/api/api-handler.ts +++ b/apps/sim/executor/handlers/api/api-handler.ts @@ -72,6 +72,7 @@ export class ApiBlockHandler implements BlockHandler { isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, callChain: ctx.callChain, + blockId: block.id, }, }, { executionContext: ctx } diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index c841bc0208f..1ccfd658fdb 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -1,6 +1,6 @@ import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' -import { adoptToolFailure } from '@/lib/core/errors/failure-log' +import { getErrorMessage } from '@sim/utils/errors' +import { adoptToolFailure, markFailureKind } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { isNonRetryableExecutionError, @@ -508,8 +508,11 @@ export class ConditionBlockHandler implements BlockHandler { case 'no-match': return null case 'expression-threw': - logger.error('Failed to evaluate condition', { conditionCount: conditions.length }) - throw conditionError(conditions[evaluation.index], evaluation.message) + /** The author's expression threw; the block executor logs it once. */ + throw markFailureKind( + conditionError(conditions[evaluation.index], evaluation.message), + 'user' + ) case 'no-verdict': if (!evaluation.retryable) { throw new NonRetryableExecutionError( @@ -523,7 +526,6 @@ export class ConditionBlockHandler implements BlockHandler { // failure as it stands. The whole list was one call, so no single // branch owns that failure; name the first, where evaluation started. if (evaluation.timedOut || ctx.abortSignal?.aborted) { - logger.error('Failed to evaluate conditions', { conditionCount: conditions.length }) throw conditionError(conditions[0], evaluation.message) } logger.warn('Batched condition evaluation produced no verdict, retrying one at a time', { @@ -549,7 +551,6 @@ export class ConditionBlockHandler implements BlockHandler { ) if (conditionMet) return condition } catch (error) { - logger.error('Failed to evaluate condition', { errorName: toError(error).name }) throw conditionError( condition, getErrorMessage(error, 'Condition evaluation failed'), diff --git a/apps/sim/executor/handlers/function/function-handler.ts b/apps/sim/executor/handlers/function/function-handler.ts index 94f7bab2ced..0244263f51d 100644 --- a/apps/sim/executor/handlers/function/function-handler.ts +++ b/apps/sim/executor/handlers/function/function-handler.ts @@ -107,6 +107,7 @@ export class FunctionBlockHandler implements BlockHandler { userId: ctx.userId, isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, + blockId: block.id, }, } diff --git a/apps/sim/executor/handlers/pi/core/redaction.ts b/apps/sim/executor/handlers/pi/core/redaction.ts index f557ea48052..27466974530 100644 --- a/apps/sim/executor/handlers/pi/core/redaction.ts +++ b/apps/sim/executor/handlers/pi/core/redaction.ts @@ -1,4 +1,5 @@ import { getErrorMessage } from '@sim/utils/errors' +import { inheritFailureMarks } from '@/lib/core/errors/failure-log' import type { PiEvent } from '@/executor/handlers/pi/core/events' /** @@ -36,11 +37,14 @@ export function getScrubbedPiErrorMessage( return scrubPiSecrets(getErrorMessage(error, fallback), secrets) } -/** Creates a boundary-safe error without retaining a potentially secret-bearing cause. */ +/** + * Creates a boundary-safe error without retaining a potentially secret-bearing cause. The failure + * marks still cross, so a GitHub tool failure the tool layer logged is not logged again. + */ export function createScrubbedPiError( error: unknown, secrets: readonly string[], fallback?: string ): Error { - return new Error(getScrubbedPiErrorMessage(error, secrets, fallback)) + return inheritFailureMarks(new Error(getScrubbedPiErrorMessage(error, secrets, fallback)), error) } diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index da90f8ee1af..cbcd00b7146 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -26,6 +26,13 @@ describe('classifyFailure', () => { expect(classifyFailure(wrapped)).toBe('internal') }) + it('keeps a retryable setup failure internal even beneath a marked wrapper', () => { + const setup = new RetryableSetupError('setup') + expect(classifyFailure(markFailureKind(new Error('wrapped', { cause: setup }), 'user'))).toBe( + 'internal' + ) + }) + it('keeps a retryable setup failure internal even when its cause was the author’s', () => { const cause = markFailureKind(new Error('missing field'), 'user') expect(classifyFailure(new RetryableSetupError('setup', { cause }))).toBe('internal') diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 2ad53f7dda5..1a76f491347 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -63,10 +63,10 @@ export function markFailureKind(error: T, kind: FailureKind): T { */ export function classifyFailure(error: unknown): FailureKind { if (findDatabaseQueryError(error)) return 'internal' + const chain = causeChain(error) + if (chain.some(isRetryableSetupError)) return 'internal' - for (const link of causeChain(error)) { - if (isRetryableSetupError(link)) return 'internal' - + for (const link of chain) { const marked = failureKinds.get(link) if (marked) return marked if (link instanceof UserFailure) return 'user' diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 316699637bc..16b8fa7547b 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -958,14 +958,10 @@ async function executeInSandboxWithinBudget( if (execution.error) { const errorMessage = `${execution.error.name}: ${execution.error.value}` - /** The author's code raising is logged once by the tool boundary; a provider failure is ours. */ - if (execution.providerFailure) { - logger.error('Sandbox execution failed', { - sandboxId, - hasTraceback: Boolean(execution.error.traceback), - providerFailure: execution.providerFailure, - }) - } + logger.error('Sandbox execution failed', { + sandboxId, + hasTraceback: Boolean(execution.error.traceback), + }) const executionResult = { result: null, stdout: execution.error.traceback || errorMessage, @@ -1157,14 +1153,10 @@ async function executeShellInSandboxWithinBudget( // back to stdout for the real command output before the generic message. const errorMessage = result.stderr || result.stdout || `Process exited with code ${result.exitCode}` - /** A non-zero exit is logged once by the tool boundary; a provider failure is ours. */ - if (result.providerFailure) { - logger.error('Sandbox shell execution error', { - sandboxId, - exitCode: result.exitCode, - providerFailure: result.providerFailure, - }) - } + logger.error('Sandbox shell execution error', { + sandboxId, + exitCode: result.exitCode, + }) const executionResult = { result: null, stdout, diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 217b31568d1..24966250bfd 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -1093,16 +1093,11 @@ function handleBodySizeLimitError(error: unknown): void { if (isBodySizeLimitError(toError(error).message)) throw bodySizeLimitError() } -function handleResponseSizeLimitError(error: unknown, requestId: string, context: string): boolean { - if (!isPayloadSizeLimitError(error)) return false - - logger.error(`[${requestId}] Response body size limit exceeded for ${context}:`, { - label: error.label, - maxBytes: error.maxBytes, - observedBytes: error.observedBytes, - }) +/** The author's request returned more than a tool response may carry; a user failure. */ +function handleResponseSizeLimitError(error: unknown): void { + if (!isPayloadSizeLimitError(error)) return if (error.maxBytes !== MAX_TOOL_RESPONSE_BODY_BYTES) throw error - throw new Error(RESPONSE_SIZE_LIMIT_ERROR_MESSAGE) + throw markFailureKind(new Error(RESPONSE_SIZE_LIMIT_ERROR_MESSAGE), 'user') } function cloneResponseHeaders(headers: Headers | HeadersInit | undefined): Headers { @@ -1180,6 +1175,19 @@ function createInternalOperationFailure(errorInfo: ErrorInfo, extractorId?: stri ) } +/** + * The `output` of a failed tool result this layer has logged. Marked so that `adoptToolFailure` + * carries the logged mark and attribution onto the error a block handler rebuilds from it. + */ +function loggedFailureOutput( + error: unknown, + output: Record = {} +): Record { + markFailureLogged(output) + markFailureKind(output, classifyFailure(error)) + return output +} + /** * Create an Error instance from errorInfo and attach useful context * Uses the error extractor registry to find the best error message @@ -2316,40 +2324,35 @@ async function executeToolImplementation( /** Sim's own hosted key being refused or throttled is Sim's fault and Sim's capacity. */ if (hostedKeyFailure && hostedKeyFailure !== 'other') markFailureKind(error, 'internal') const toolContext = toRecord(params._context) - const loggedKind = logFailureOnce( - logger, - `[${requestId}] Error executing tool ${toolId}:`, - error, - { - metadata: () => ({ - toolId, - workflowId: executionContext?.workflowId ?? undefined, - executionId: executionContext?.executionId, - blockId: typeof toolContext.blockId === 'string' ? toolContext.blockId : undefined, - ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), - ...projectToolLogMetadata( - { - ...(databaseErrorCause - ? { cause: databaseErrorCause } - : { - error: normalizedError.message, - stack: error instanceof Error ? error.stack : undefined, - ...(error instanceof Error && externalHttpFailures.has(error) - ? { errorData: error.data } - : {}), - }), - }, - resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - hasStack: !databaseErrorCause && Boolean(error instanceof Error && error.stack), - ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), - }, - structuralOnlyToolLogs - ), - }), - } - ) + logFailureOnce(logger, `[${requestId}] Error executing tool ${toolId}:`, error, { + metadata: () => ({ + toolId, + workflowId: executionContext?.workflowId ?? undefined, + executionId: executionContext?.executionId, + blockId: typeof toolContext.blockId === 'string' ? toolContext.blockId : undefined, + ...(typeof upstreamStatus === 'number' ? { status: upstreamStatus } : {}), + ...projectToolLogMetadata( + { + ...(databaseErrorCause + ? { cause: databaseErrorCause } + : { + error: normalizedError.message, + stack: error instanceof Error ? error.stack : undefined, + ...(error instanceof Error && externalHttpFailures.has(error) + ? { errorData: error.data } + : {}), + }), + }, + resolvedSecretTraceRegistry, + { + errorName: normalizedError.name, + hasStack: !databaseErrorCause && Boolean(error instanceof Error && error.stack), + ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), + }, + structuralOnlyToolLogs + ), + }), + }) if (hostedKeyForMetrics && hostedKeyFailure) { hostedKeyMetrics.recordFailed({ ...hostedKeyForMetrics, reason: hostedKeyFailure }) @@ -2424,16 +2427,12 @@ async function executeToolImplementation( const responseData = isRecordLike(rawResponseData) ? rawResponseData : undefined const functionSandboxCost = normalizedToolId === 'function_execute' ? readFunctionSandboxCost(responseData) : undefined - const failureOutput = { - ...errorDetails, - ...(functionSandboxCost ? { cost: functionSandboxCost } : {}), - } - /** Lets `adoptToolFailure` carry both marks onto a handler's rebuilt error. */ - markFailureLogged(failureOutput) - markFailureKind(failureOutput, loggedKind ?? classifyFailure(error)) return { success: false, - output: failureOutput, + output: loggedFailureOutput(error, { + ...errorDetails, + ...(functionSandboxCost ? { cost: functionSandboxCost } : {}), + }), error: errorMessage, ...(responseData?.retryable === false ? { retryable: false } : {}), // Sim's own status (hosted-key 429/503) survives the flattening from a @@ -2667,14 +2666,18 @@ async function executeDeclaredInternalOperation({ 'schema' in operationInput && 'params' in operationInput ) { - validateClientSideParams( - operationInput.params as Record, - operationInput.schema as { - type: string - properties: Record - required?: string[] - } - ) + try { + validateClientSideParams( + operationInput.params as Record, + operationInput.schema as { + type: string + properties: Record + required?: string[] + } + ) + } catch (validationError) { + throw markFailureKind(validationError, 'user') + } } const headers = new Headers() @@ -3126,7 +3129,7 @@ async function executeToolRequest( error: undefined, } } catch (error: any) { - handleResponseSizeLimitError(error, requestId, toolId) + handleResponseSizeLimitError(error) handleBodySizeLimitError(error) @@ -3313,15 +3316,25 @@ async function executeMcpTool( const errorMsg = toError(error).message if (isBodySizeLimitError(errorMsg)) { - logger.error( + const failure = bodySizeLimitError() + logFailureOnce( + logger, `[${actualRequestId}] Request body size limit exceeded for mcp:${toolId}:`, - projectToolLogMetadata({ originalError: errorMsg }, context?.resolvedSecretTraceRegistry, { - hasOriginalError: errorMsg.length > 0, - }) + failure, + { + metadata: () => + projectToolLogMetadata( + { originalError: errorMsg }, + context?.resolvedSecretTraceRegistry, + { + hasOriginalError: errorMsg.length > 0, + } + ), + } ) return { success: false, - output: {}, + output: loggedFailureOutput(failure), error: BODY_SIZE_LIMIT_ERROR_MESSAGE, timing: { startTime: actualStartTime, @@ -3332,26 +3345,26 @@ async function executeMcpTool( } const normalizedError = toError(error) - logger.error( - `[${actualRequestId}] Error executing MCP tool ${toolId}:`, - projectToolLogMetadata( - { - error: normalizedError.message, - stack: error instanceof Error ? error.stack : undefined, - }, - context?.resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - hasStack: Boolean(error instanceof Error && error.stack), - } - ) - ) + logFailureOnce(logger, `[${actualRequestId}] Error executing MCP tool ${toolId}:`, error, { + metadata: () => + projectToolLogMetadata( + { + error: normalizedError.message, + stack: error instanceof Error ? error.stack : undefined, + }, + context?.resolvedSecretTraceRegistry, + { + errorName: normalizedError.name, + hasStack: Boolean(error instanceof Error && error.stack), + } + ), + }) const errorMessage = getErrorMessage(error, `Failed to execute MCP tool ${toolId}`) return { success: false, - output: {}, + output: loggedFailureOutput(error), error: errorMessage, timing: { startTime: actualStartTime, From 623de811ee620a723788339eee633c0cbf5868f5 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 19:31:51 -0700 Subject: [PATCH 08/10] fix(logs): keep run identity on every remaining failure line Fixes the type error on the external failure body, attributes a provider error payload on a 2xx to the provider, and adds workflow, execution, and block ids to the MCP failure lines that replace the block executor's. The condition batch carries its failed result's marks and passes its block id. Pi GitHub calls carry no run identity, so their errors now carry only the tool layer's attribution and the block executor still logs them with ids. Restores the HITL notification warning, which covers soft failures the tool layer never logs, and attributes refused tool and proxy URLs to the author. --- .../handlers/condition/condition-handler.ts | 20 +++++++-- .../human-in-the-loop-handler.ts | 4 ++ .../handlers/pi/cloud/babysit/github.ts | 14 +++--- apps/sim/executor/handlers/pi/cloud/shared.ts | 12 +++-- .../executor/handlers/pi/core/redaction.ts | 11 +++-- apps/sim/tools/index.ts | 45 ++++++++++++------- 6 files changed, 73 insertions(+), 33 deletions(-) diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index 1ccfd658fdb..bb198033611 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -40,7 +40,14 @@ type ConditionEvaluation = | { status: 'matched'; index: number } | { status: 'no-match' } | { status: 'expression-threw'; index: number; message: string } - | { status: 'no-verdict'; message: string; retryable: boolean; timedOut: boolean } + | { + status: 'no-verdict' + message: string + retryable: boolean + timedOut: boolean + /** The failed tool result's output, so a terminal error carries its logged mark. */ + output?: unknown + } /** * Wraps one expression as a boolean test, on its own line so a trailing line @@ -173,6 +180,7 @@ async function runConditionCode( userId: ctx.userId, isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, + blockId: currentNodeId, }, }, { executionContext: ctx } @@ -215,6 +223,7 @@ async function evaluateConditionList( message, retryable: result.retryable !== false, timedOut: isTimeoutFailure(result.error), + output: result.output, } } @@ -515,8 +524,11 @@ export class ConditionBlockHandler implements BlockHandler { ) case 'no-verdict': if (!evaluation.retryable) { - throw new NonRetryableExecutionError( - `Evaluation error in condition "${conditions[0].title}": ${evaluation.message}` + throw adoptToolFailure( + new NonRetryableExecutionError( + `Evaluation error in condition "${conditions[0].title}": ${evaluation.message}` + ), + evaluation ) } // Retrying one branch at a time is what recovers a batch the sandbox @@ -526,7 +538,7 @@ export class ConditionBlockHandler implements BlockHandler { // failure as it stands. The whole list was one call, so no single // branch owns that failure; name the first, where evaluation started. if (evaluation.timedOut || ctx.abortSignal?.aborted) { - throw conditionError(conditions[0], evaluation.message) + throw adoptToolFailure(conditionError(conditions[0], evaluation.message), evaluation) } logger.warn('Batched condition evaluation produced no verdict, retrying one at a time', { conditionCount: conditions.length, diff --git a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts index 66d98a7431e..1ac7e5d5a8f 100644 --- a/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts +++ b/apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts @@ -561,6 +561,10 @@ export class HumanInTheLoopBlockHandler implements BlockHandler { const durationMs = Date.now() - startTime if (!result.success) { + logger.warn('Notification tool execution failed', { + toolId, + error: result.error, + }) return { toolId, title: toolConfig.title, diff --git a/apps/sim/executor/handlers/pi/cloud/babysit/github.ts b/apps/sim/executor/handlers/pi/cloud/babysit/github.ts index e44b17ec287..8920c380832 100644 --- a/apps/sim/executor/handlers/pi/cloud/babysit/github.ts +++ b/apps/sim/executor/handlers/pi/cloud/babysit/github.ts @@ -1,7 +1,7 @@ import { getErrorMessage } from '@sim/utils/errors' import { isRecordLike } from '@sim/utils/object' import { truncate } from '@sim/utils/string' -import { adoptToolFailure } from '@/lib/core/errors/failure-log' +import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log' import type { BabysitRoundDecision } from '@/executor/handlers/pi/cloud/babysit/round' import { fetchPrSnapshot, @@ -24,6 +24,7 @@ import type { StatusCheckRollupContext, SubmittedReviewSummary, } from '@/tools/github/types' +import type { ToolResponse } from '@/tools/types' const MAX_PAGES = 10 const MAX_COMMENTS_PER_THREAD = 50 @@ -205,6 +206,11 @@ function toolFailure(label: string, error: unknown): Error { ) } +/** Carries the tool layer's attribution; these calls carry no run identity, so not its logged mark. */ +function attributedToolFailure(label: string, result: ToolResponse): Error { + return markFailureKind(toolFailure(label, result.error), classifyFailure(result.output)) +} + function parseReviewThread(value: unknown, index: number): ReviewThread { if (!isRecordLike(value)) throw new Error(`Review thread ${index} must be an object`) const commentsValue = value.comments @@ -289,8 +295,7 @@ export async function fetchBabysitThreads( }, { signal } ) - if (!result.success) - throw adoptToolFailure(toolFailure('Failed to fetch review threads', result.error), result) + if (!result.success) throw attributedToolFailure('Failed to fetch review threads', result) const output = result.output if (!isRecordLike(output) || !Array.isArray(output.threads)) { throw new Error('Review thread response is incomplete') @@ -425,8 +430,7 @@ export async function fetchBabysitCheckState( }, { signal } ) - if (!result.success) - throw adoptToolFailure(toolFailure('Failed to fetch checks', result.error), result) + if (!result.success) throw attributedToolFailure('Failed to fetch checks', result) const output = result.output if (!isRecordLike(output) || !Array.isArray(output.contexts)) { throw new Error('Check response is incomplete') diff --git a/apps/sim/executor/handlers/pi/cloud/shared.ts b/apps/sim/executor/handlers/pi/cloud/shared.ts index 58da46e6573..691e331599f 100644 --- a/apps/sim/executor/handlers/pi/cloud/shared.ts +++ b/apps/sim/executor/handlers/pi/cloud/shared.ts @@ -5,7 +5,7 @@ * security-sensitive details. */ -import { adoptToolFailure } from '@/lib/core/errors/failure-log' +import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log' import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { resolvePiSandboxLifetimeMs } from '@/lib/execution/remote-sandbox/pi-lifetime' import { PI_EVENT_FILTER_PATH } from '@/executor/handlers/pi/cloud/event-filter-source' @@ -224,9 +224,13 @@ export function scrubGitSecrets(text: string, token: string): string { } /** - * The error a backend throws for a failed GitHub tool call. Carries the tool layer's marks so the - * failure `executeTool` already logged is not logged again at error. + * The error a backend throws for a failed GitHub tool call. Carries the tool layer's attribution + * but not its logged mark: these calls carry no run identity, so the block executor's line, which + * does, must still be written. */ export function toolResultError(label: string, result: ToolResponse): Error { - return adoptToolFailure(new Error(`${label}: ${result.error ?? 'unknown error'}`), result) + return markFailureKind( + new Error(`${label}: ${result.error ?? 'unknown error'}`), + classifyFailure(result.output) + ) } diff --git a/apps/sim/executor/handlers/pi/core/redaction.ts b/apps/sim/executor/handlers/pi/core/redaction.ts index 27466974530..3dcf7886d6a 100644 --- a/apps/sim/executor/handlers/pi/core/redaction.ts +++ b/apps/sim/executor/handlers/pi/core/redaction.ts @@ -1,5 +1,5 @@ import { getErrorMessage } from '@sim/utils/errors' -import { inheritFailureMarks } from '@/lib/core/errors/failure-log' +import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log' import type { PiEvent } from '@/executor/handlers/pi/core/events' /** @@ -38,13 +38,16 @@ export function getScrubbedPiErrorMessage( } /** - * Creates a boundary-safe error without retaining a potentially secret-bearing cause. The failure - * marks still cross, so a GitHub tool failure the tool layer logged is not logged again. + * Creates a boundary-safe error without retaining a potentially secret-bearing cause. The failure's + * attribution still crosses, so a GitHub 404 is not logged as a Sim fault. */ export function createScrubbedPiError( error: unknown, secrets: readonly string[], fallback?: string ): Error { - return inheritFailureMarks(new Error(getScrubbedPiErrorMessage(error, secrets, fallback)), error) + return markFailureKind( + new Error(getScrubbedPiErrorMessage(error, secrets, fallback)), + classifyFailure(error) + ) } diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 24966250bfd..49c37395f8a 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -1156,12 +1156,13 @@ async function readToolResponseBody( * is logged: an internal operation's or a Function's body carries the author's data, stdout, and * source lines. */ -const externalHttpFailures = new WeakSet() +const externalHttpFailures = new WeakSet() function createExternalHttpFailure(errorInfo?: ErrorInfo, extractorId?: string): Error { const failure = createTransformedErrorFromErrorInfo(errorInfo, extractorId) externalHttpFailures.add(failure) - return failure + /** An error payload on a 2xx carries no status but is still the provider refusing. */ + return errorInfo?.status === undefined ? markFailureKind(failure, 'third_party_client') : failure } /** @@ -2338,9 +2339,7 @@ async function executeToolImplementation( : { error: normalizedError.message, stack: error instanceof Error ? error.stack : undefined, - ...(error instanceof Error && externalHttpFailures.has(error) - ? { errorData: error.data } - : {}), + ...(externalHttpFailures.has(error) ? { errorData: error.data } : {}), }), }, resolvedSecretTraceRegistry, @@ -2843,6 +2842,11 @@ async function executeDeclaredInternalOperation({ } } +/** A tool or proxy URL the author configured that Sim refuses to call; the author's to fix. */ +function invalidToolTarget(message: string): Error { + return markFailureKind(new Error(message), 'user') +} + /** Executes one external tool request with DNS validation and IP pinning. */ async function executeToolRequest( toolId: string, @@ -2860,7 +2864,7 @@ async function executeToolRequest( const targetsThisSimInstance = isSelfOriginUrl(fullUrl) if (targetsThisSimInstance && tool.request.allowSameOrigin !== true) { - throw new Error(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE) + throw invalidToolTarget(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE) } if (targetsThisSimInstance) { @@ -2892,14 +2896,14 @@ async function executeToolRequest( try { const urlValidation = await validateUrlWithDNS(fullUrl, 'toolUrl', 'requestTarget') if (!urlValidation.isValid) { - throw new Error(`Invalid tool URL: ${urlValidation.error}`) + throw invalidToolTarget(`Invalid tool URL: ${urlValidation.error}`) } let proxyOption: string | undefined if (requestParams.proxyUrl) { const proxyValidation = await validateAndPinProxyUrl(requestParams.proxyUrl) if (!proxyValidation.isValid) { - throw new Error(`Invalid proxy URL: ${proxyValidation.error}`) + throw invalidToolTarget(`Invalid proxy URL: ${proxyValidation.error}`) } proxyOption = proxyValidation.pinnedProxyUrl } @@ -2920,7 +2924,7 @@ async function executeToolRequest( ? undefined : (redirectUrl) => { if (isSelfOriginUrl(redirectUrl)) { - throw new Error(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE) + throw invalidToolTarget(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE) } }, }) @@ -3314,6 +3318,13 @@ async function executeMcpTool( const endTimeISO = endTime.toISOString() const duration = endTime.getTime() - new Date(actualStartTime).getTime() + /** These lines replace the block executor's, so they carry the run identity themselves. */ + const blockId = toRecord(params._context).blockId + const runIdentity = { + workflowId: context?.workflowId, + executionId: context?.executionId, + blockId: typeof blockId === 'string' ? blockId : undefined, + } const errorMsg = toError(error).message if (isBodySizeLimitError(errorMsg)) { const failure = bodySizeLimitError() @@ -3322,14 +3333,14 @@ async function executeMcpTool( `[${actualRequestId}] Request body size limit exceeded for mcp:${toolId}:`, failure, { - metadata: () => - projectToolLogMetadata( + metadata: () => ({ + ...runIdentity, + ...projectToolLogMetadata( { originalError: errorMsg }, context?.resolvedSecretTraceRegistry, - { - hasOriginalError: errorMsg.length > 0, - } + { hasOriginalError: errorMsg.length > 0 } ), + }), } ) return { @@ -3346,8 +3357,9 @@ async function executeMcpTool( const normalizedError = toError(error) logFailureOnce(logger, `[${actualRequestId}] Error executing MCP tool ${toolId}:`, error, { - metadata: () => - projectToolLogMetadata( + metadata: () => ({ + ...runIdentity, + ...projectToolLogMetadata( { error: normalizedError.message, stack: error instanceof Error ? error.stack : undefined, @@ -3358,6 +3370,7 @@ async function executeMcpTool( hasStack: Boolean(error instanceof Error && error.stack), } ), + }), }) const errorMessage = getErrorMessage(error, `Failed to execute MCP tool ${toolId}`) From eb8e73a8e3eef5cef2e42270a9b0f090be957214 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 19:42:21 -0700 Subject: [PATCH 09/10] fix(logs): attribute a revoked OAuth credential to its owner A CredentialRevokedError anywhere in the cause chain classifies as a user failure, so the boundaries that log it after token resolution's WARN write INFO rather than ERROR. --- apps/sim/lib/core/errors/failure-log.test.ts | 6 ++++++ apps/sim/lib/core/errors/failure-log.ts | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index cbcd00b7146..48a06a4b30e 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -10,6 +10,7 @@ import { } from '@/lib/core/errors/failure-log' import { RetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' import { UserFailure } from '@/lib/core/errors/user-failure' +import { CredentialRevokedError } from '@/lib/oauth/credential-revoked' import { HostedKeyRateLimitedError, HostedKeyUnavailableError } from '@/tools/errors' const logger = createLogger('FailureLogTest') @@ -56,6 +57,11 @@ describe('classifyFailure', () => { expect(classifyFailure(upstream)).toBe('third_party_client') }) + it('attributes a revoked OAuth credential to its owner, not to Sim', () => { + const revoked = new CredentialRevokedError('Reconnect your account') + expect(classifyFailure(new Error('Tool failed', { cause: revoked }))).toBe('user') + }) + it('leaves an unattributed failure internal', () => { expect(classifyFailure(new Error('something broke'))).toBe('internal') expect(classifyFailure('a thrown string')).toBe('internal') diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 1a76f491347..3150dc05320 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -3,6 +3,7 @@ import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure' import { UserFailure } from '@/lib/core/errors/user-failure' import { HttpError } from '@/lib/core/utils/http-error' +import { CredentialRevokedError } from '@/lib/oauth/credential-revoked' /** * Who a failure is attributable to, which decides how loudly the server logs it. The user @@ -58,8 +59,9 @@ export function markFailureKind(error: T, kind: FailureKind): T { /** * Attributes `error` from its cause chain. A database or retryable setup failure is always - * internal, then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError` - * status, or an upstream `status` decides. Anything unattributed is internal. + * internal, then the outermost link with an explicit mark, a {@link UserFailure} or revoked + * credential, a Sim `HttpError` status, or an upstream `status` decides. Anything unattributed is + * internal. */ export function classifyFailure(error: unknown): FailureKind { if (findDatabaseQueryError(error)) return 'internal' @@ -69,7 +71,8 @@ export function classifyFailure(error: unknown): FailureKind { for (const link of chain) { const marked = failureKinds.get(link) if (marked) return marked - if (link instanceof UserFailure) return 'user' + /** Only the credential's owner reconnecting restores a revoked grant. */ + if (link instanceof UserFailure || link instanceof CredentialRevokedError) return 'user' if (link instanceof HttpError) { return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal' From 5a86b023319abb31a4fc4cca6b05113e64ddfa3d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 20:03:27 -0700 Subject: [PATCH 10/10] fix(logs): keep one logged mark per execution and attribute unparseable responses A raw value logged at an execution boundary now remembers every execution that logged it (bounded), so overlapping runs sharing one persistent rejection each log it once. A 2xx body that is not JSON is the endpoint's failure, not Sim's. --- apps/sim/lib/core/errors/failure-log.test.ts | 9 +++++++++ apps/sim/lib/core/errors/failure-log.ts | 20 ++++++++++++++++---- apps/sim/tools/index.ts | 6 +++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index 48a06a4b30e..481b3b3ceb3 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -111,4 +111,13 @@ describe('logFailureOnce', () => { expect(outerBoundary(persistentFault)).toBe('internal') expect(outerBoundary(persistentFault, '')).toBe('internal') }) + + it('logs one persistent value once in each of two overlapping executions', () => { + const persistentFault = new Error('module failed to load') + expect(outerBoundary(persistentFault, 'exec-a')).toBe('internal') + expect(outerBoundary(persistentFault, 'exec-b')).toBe('internal') + + expect(outerBoundary(persistentFault, 'exec-a')).toBeUndefined() + expect(outerBoundary(persistentFault, 'exec-b')).toBeUndefined() + }) }) diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 3150dc05320..63a3b5d3cc1 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -30,11 +30,15 @@ const MAX_CAUSE_DEPTH = 8 * tool failure's output, a block error, a handler's rebuilt error), never a raw thrown value: a * persistent fault can rethrow one object forever (a rejected dynamic `import()`, a memoized * rejected promise), and marking it would silence every later occurrence process-wide. - * `loggedInExecution` scopes a raw value's mark to the one execution that logged it. + * `loggedInExecution` scopes a raw value's mark to the executions that logged it, so concurrent + * runs sharing one persistent rejection each log it once. */ const failureKinds = new WeakMap() const loggedCarriers = new WeakSet() -const loggedInExecution = new WeakMap() +const loggedInExecution = new WeakMap>() + +/** Bounds the executions remembered per raw value, which a persistent fault shares across runs. */ +const MAX_EXECUTIONS_PER_VALUE = 32 function isKeyable(value: unknown): value is object { return (typeof value === 'object' || typeof value === 'function') && value !== null @@ -126,7 +130,7 @@ function wasFailureLogged(error: unknown, executionId?: string): boolean { return causeChain(error).some( (link) => loggedCarriers.has(link) || - (Boolean(executionId) && loggedInExecution.get(link) === executionId) + (executionId !== undefined && loggedInExecution.get(link)?.has(executionId) === true) ) } @@ -167,6 +171,14 @@ export function logFailureOnce( failureKind, }) /** An empty id (a request that failed before minting one) scopes nothing. */ - if (executionId && isKeyable(error)) loggedInExecution.set(error, executionId) + if (executionId && isKeyable(error)) { + const executions = loggedInExecution.get(error) ?? new Set() + if (executions.size >= MAX_EXECUTIONS_PER_VALUE) { + const oldest = executions.values().next().value + if (oldest !== undefined) executions.delete(oldest) + } + executions.add(executionId) + loggedInExecution.set(error, executions) + } return failureKind } diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 49c37395f8a..e86fecee0fd 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -3059,7 +3059,11 @@ async function executeToolRequest( try { responseData = await response.json() } catch (jsonError) { - throw new Error(`Failed to parse response from ${toolId}: ${jsonError}`) + /** The endpoint answered with a body that is not JSON; not Sim's fault. */ + throw markFailureKind( + new Error(`Failed to parse response from ${toolId}: ${jsonError}`), + 'third_party_server' + ) } } }