diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index f81c4c43ade..7a768159e64 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,10 +1616,11 @@ async function handleExecutePost( return payloadTooLargeResponse() } - reqLogger.error( - 'Non-SSE execution failed', - loggingSession.projectDiagnosticError(error, { isTimeout: executionTimedOut }) - ) + 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) @@ -2420,10 +2422,10 @@ async function handleExecutePost( ? getTimeoutErrorMessage(timeoutController.timeoutMs) : getErrorMessage(error, 'Unknown error') - reqLogger.error( - 'SSE execution failed', - loggingSession.projectDiagnosticError(error, { isTimeout }) - ) + 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 bdc774bc027..0a789bc5dd6 100644 --- a/apps/sim/background/async-preprocessing-correlation.test.ts +++ b/apps/sim/background/async-preprocessing-correlation.test.ts @@ -488,12 +488,10 @@ 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 } + { 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..eaa2c6f8de9 100644 --- a/apps/sim/background/webhook-execution.test.ts +++ b/apps/sim/background/webhook-execution.test.ts @@ -486,7 +486,13 @@ describe('executeWebhookJob fault vs error handling', () => { }) expect(webhookExecutionLogger.error).toHaveBeenCalledWith( '[request-1] Webhook execution failed', - { workflowId: 'workflow-1', provider: 'gmail', error: projectedError } + { + executionId: 'execution-1', + 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/background/webhook-execution.ts b/apps/sim/background/webhook-execution.ts index 5e4f5a6476a..632185c7a9f 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,13 +1268,14 @@ async function executeWebhookJobInternal( throw new RetryableSetupError(errorMessage, { cause: retryableSetupCause }) } - logger.error( - `[${requestId}] Webhook execution failed`, - loggingSession.projectDiagnosticError(error, { - workflowId: payload.workflowId, - provider: payload.provider, - }) - ) + 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 33acc5ed146..145ab0970f0 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,10 +307,10 @@ export async function executeWorkflowJob( metadata: payload.metadata, } } catch (error: unknown) { - logger.error( - `[${requestId}] Workflow execution failed: ${workflowId}`, - loggingSession.projectDiagnosticError(error, { 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 c25d45c1648..bf24caf4e89 100644 --- a/apps/sim/executor/errors/boundary.ts +++ b/apps/sim/executor/errors/boundary.ts @@ -1,3 +1,5 @@ +import { UserFailure } from '@/lib/core/errors/user-failure' + /** * 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 @@ -39,7 +41,7 @@ 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 }) { diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index e9f7f9cf8fe..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,6 +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, 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' @@ -12,6 +14,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' @@ -496,6 +499,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(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined() }) it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => { @@ -572,6 +578,116 @@ 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.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 + + 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 = { @@ -614,11 +730,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/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 4b1a84cd33b..088118c3434 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' @@ -788,30 +789,46 @@ export class BlockExecutor { } } - const diagnosticRegistry = ctx.errorResolvedSecretTraceRegistry - ? ctx.errorResolvedSecretTraceRegistry - : inputDisplayRegistry?.forkForToolCall() - if ( - !ctx.errorResolvedSecretTraceRegistry && - diagnosticRegistry && - ctx.resolvedSecretTraceRegistry && - ctx.resolvedSecretTraceRegistry !== inputDisplayRegistry - ) { - diagnosticRegistry.mergeToolCallRegistry(ctx.resolvedSecretTraceRegistry) + /** Lazy, so a failure a tool already logged skips the secret projection. */ + 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 - ) - 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, - ...errorDiagnostic, + metadata: () => ({ + blockId: node.id, + blockType: block.metadata?.id, + executionId: ctx.executionId, + workflowId: ctx.workflowId, + ...getErrorDiagnostic(), + }), } ) @@ -850,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 } @@ -861,7 +878,7 @@ export class BlockExecutor { ? error : new Error(errorMessage) - throw buildBlockExecutionError({ + const blockError = buildBlockExecutionError({ block, error: errorToThrow, context: ctx, @@ -870,6 +887,9 @@ export class BlockExecutor { executionTime: duration, }, }) + /** The raw thrown value is never marked logged, so the fresh block error carries the mark. */ + 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..ffb20f432f9 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,10 +197,11 @@ export class ExecutionEngine { this.finalizeIncompleteLogs() const errorMessage = normalizeError(error) - this.execLogger.error( - 'Execution failed', - projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry) - ) + logFailureOnce(this.execLogger, 'Execution failed', error, { + metadata: () => + projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), + executionId: this.context.executionId, + }) const executionResult: ExecutionResult = { success: false, @@ -477,11 +479,20 @@ export class ExecutionEngine { }) } } catch (error) { - this.execLogger.error('Node execution failed', { - nodeId, - ...projectResolvedSecretDiagnosticError(error, this.context.resolvedSecretTraceRegistry), + /** + * 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. + */ + const failure = toError(error) + logFailureOnce(this.execLogger, 'Node execution failed', failure, { + metadata: () => + projectResolvedSecretDiagnosticError(failure, this.context.resolvedSecretTraceRegistry, { + nodeId, + }), + executionId: this.context.executionId, }) - throw error + throw failure } } diff --git a/apps/sim/executor/execution/failure-trace.test.ts b/apps/sim/executor/execution/failure-trace.test.ts index 929cb6707c4..3d57fa547d8 100644 --- a/apps/sim/executor/execution/failure-trace.test.ts +++ b/apps/sim/executor/execution/failure-trace.test.ts @@ -1,5 +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, 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' @@ -70,4 +72,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(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined() + expect(classifyFailure(thrown)).toBe('user') + }) }) 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 2fad9b54b0e..c35b521d580 100644 --- a/apps/sim/executor/handlers/agent/agent-handler.ts +++ b/apps/sim/executor/handlers/agent/agent-handler.ts @@ -1,7 +1,8 @@ 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' import { projectModelSchemaAnnotations, @@ -307,6 +308,30 @@ function isTransportTimeout(error: unknown): boolean { return false } +/** + * 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 isProviderKeyRejection(error: unknown): boolean { + const status = isRecordLike(error) ? error.status : undefined + return status === 401 || status === 402 || status === 403 +} + /** * Handler for Agent blocks that process LLM requests with optional tools. */ @@ -2939,7 +2964,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 @@ -3023,13 +3047,12 @@ export class AgentBlockHandler implements BlockHandler { return this.processProviderResponse(response, block, responseFormat, ctx) } catch (error) { - const errorRegistry = this.createErrorRegistry(providerErrorRegistry, modelRuntimeRegistry) - ctx.errorResolvedSecretTraceRegistry = errorRegistry - const diagnosticCtx = errorRegistry - ? { ...ctx, resolvedSecretTraceRegistry: errorRegistry } - : ctx + ctx.errorResolvedSecretTraceRegistry = this.createErrorRegistry( + providerErrorRegistry, + modelRuntimeRegistry + ) try { - this.handleExecutionError(error, providerStartTime, providerId, model, diagnosticCtx, block) + this.handleExecutionError(error) } finally { if (modelRuntimeRegistry) { ctx.resolvedSecretTraceRegistry = modelRuntimeRegistry.forkForPropagatedEntries() @@ -3050,56 +3073,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 - - logger.error( - 'Error executing provider request', - projectAgentDiagnosticMetadata( - ctx, - { - executionTime, - provider, - model, - workflowId: ctx.workflowId, - blockId: block.id, - ...getErrorDiagnosticMetadata(error), - }, - { - executionTime, - workflowId: ctx.workflowId, - blockId: block.id, - ...getErrorDiagnosticFallback(error), - } - ) - ) - - 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') + /** + * 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) { + 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)) { + 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..29a0ba43877 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' @@ -71,6 +72,7 @@ export class ApiBlockHandler implements BlockHandler { isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, callChain: ctx.callChain, + blockId: block.id, }, }, { executionContext: ctx } @@ -124,7 +126,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..bb198033611 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 { getErrorMessage } from '@sim/utils/errors' +import { adoptToolFailure, markFailureKind } from '@/lib/core/errors/failure-log' import { normalizeStringRecord, normalizeWorkflowVariables } from '@/lib/core/utils/records' import { isNonRetryableExecutionError, @@ -39,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 @@ -172,6 +180,7 @@ async function runConditionCode( userId: ctx.userId, isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, + blockId: currentNodeId, }, }, { executionContext: ctx } @@ -214,6 +223,7 @@ async function evaluateConditionList( message, retryable: result.retryable !== false, timedOut: isTimeoutFailure(result.error), + output: result.output, } } @@ -293,9 +303,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) @@ -504,12 +517,18 @@ 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( - `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 @@ -519,8 +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) { - logger.error('Failed to evaluate conditions', { conditionCount: conditions.length }) - 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, @@ -545,7 +563,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 2d66a019e1f..0244263f51d 100644 --- a/apps/sim/executor/handlers/function/function-handler.ts +++ b/apps/sim/executor/handlers/function/function-handler.ts @@ -1,3 +1,4 @@ +import { adoptToolFailure } from '@/lib/core/errors/failure-log' import { getRemainingExecutionMs } from '@/lib/core/execution-limits' import { normalizeRecord, @@ -106,6 +107,7 @@ export class FunctionBlockHandler implements BlockHandler { userId: ctx.userId, isDeployedContext: ctx.isDeployedContext, enforceCredentialAccess: ctx.enforceCredentialAccess, + blockId: block.id, }, } @@ -117,7 +119,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) - 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/pi/cloud/authoring/backend.ts b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts index 8c0e16252d5..20f0c447f5f 100644 --- a/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/authoring/backend.ts @@ -57,6 +57,7 @@ import { raceAbort, resolvePiTimeoutMs, scrubGitSecrets, + toolResultError, } from '@/executor/handlers/pi/cloud/shared' import type { PiBackendRun, @@ -185,7 +186,7 @@ async function openPullRequest( ) if (!result.success) { - throw new Error(`PR creation failed for branch ${branch}: ${result.error ?? 'unknown error'}`) + throw toolResultError(`PR creation failed for branch ${branch}`, result) } if (!isRecordLike(result.output)) { @@ -221,9 +222,7 @@ async function repositoryDefaultBranch( { signal } ) if (!result.success) { - throw new Error( - `Failed to determine the repository default branch: ${result.error ?? 'unknown error'}` - ) + throw toolResultError('Failed to determine the repository default branch', result) } if (!isRecordLike(result.output)) { throw new Error('GitHub repository response must be an object') @@ -264,9 +263,7 @@ async function updatePullRequest( { signal } ) if (!result.success) { - throw new Error( - `PR update failed for branch ${params.targetBranch}: ${result.error ?? 'unknown error'}` - ) + throw toolResultError(`PR update failed for branch ${params.targetBranch}`, 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..8920c380832 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 { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log' import type { BabysitRoundDecision } from '@/executor/handlers/pi/cloud/babysit/round' import { fetchPrSnapshot, @@ -23,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 @@ -204,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 @@ -288,7 +295,7 @@ export async function fetchBabysitThreads( }, { signal } ) - if (!result.success) throw toolFailure('Failed to fetch review threads', result.error) + 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') @@ -423,7 +430,7 @@ export async function fetchBabysitCheckState( }, { signal } ) - if (!result.success) throw toolFailure('Failed to fetch checks', result.error) + 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/github-pr.ts b/apps/sim/executor/handlers/pi/cloud/github-pr.ts index dc72e46aced..290a38da059 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 { toolResultError } from '@/executor/handlers/pi/cloud/shared' import { executeTool } from '@/tools' import { GITHUB_GRAPHQL_URL, githubGraphQlHeaders, readGraphQlData } from '@/tools/github/graphql' import { @@ -109,7 +110,7 @@ export async function fetchPrSnapshot( ) if (!result.success) { - throw new Error(`Failed to fetch PR #${params.pullNumber}: ${result.error ?? 'unknown error'}`) + throw toolResultError(`Failed to fetch PR #${params.pullNumber}`, result) } return parsePullRequestSnapshot(result.output) @@ -173,9 +174,7 @@ 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 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 f648700b33e..83263157fdb 100644 --- a/apps/sim/executor/handlers/pi/cloud/review/backend.ts +++ b/apps/sim/executor/handlers/pi/cloud/review/backend.ts @@ -31,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' @@ -186,9 +187,7 @@ async function submitReview( ) if (!result.success) { - throw new Error( - `Failed to submit review for PR #${params.pullNumber}: ${result.error ?? 'unknown error'}` - ) + 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..691e331599f 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 { 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' 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,15 @@ 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 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 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 f557ea48052..3dcf7886d6a 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 { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log' import type { PiEvent } from '@/executor/handlers/pi/core/events' /** @@ -36,11 +37,17 @@ 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'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 new Error(getScrubbedPiErrorMessage(error, secrets, fallback)) + return markFailureKind( + new Error(getScrubbedPiErrorMessage(error, secrets, fallback)), + classifyFailure(error) + ) } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 2629d5b79d6..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,6 +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, 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' @@ -370,6 +372,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(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBe('internal') + 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 321ecab58b7..d668a6f2e15 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -4,6 +4,11 @@ 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 { + inheritFailureMarks, + 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 +320,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 @@ -964,7 +972,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, @@ -1007,11 +1016,6 @@ export class WorkflowBlockHandler implements BlockHandler { return mappedResult } catch (error: unknown) { - logger.error('Error executing child workflow', { - 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) { @@ -1173,13 +1177,16 @@ export class WorkflowBlockHandler implements BlockHandler { ? error.consumerFacing : undefined if (alreadyClassified) { - return new ChildWorkflowError({ - message: alreadyClassified.message, - childWorkflowName: blockName, - childWorkflowInstanceId: instanceId, - consumerFacing: alreadyClassified, - ...traceHandle, - }) + return inheritFailureMarks( + new ChildWorkflowError({ + message: alreadyClassified.message, + childWorkflowName: blockName, + childWorkflowInstanceId: instanceId, + consumerFacing: alreadyClassified, + ...traceHandle, + }), + error + ) } const safe = isBoundarySafeError(error) ? error : undefined @@ -1193,16 +1200,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 inheritFailureMarks( + 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, + }), + error + ) } /** @@ -1543,16 +1553,20 @@ 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] - throw new ChildWorkflowError({ + const childFailure = new ChildWorkflowError({ message: formatWorkflowChainMessage(chain, rootErrorMessage), childWorkflowName, workflowChain: chain, @@ -1561,6 +1575,9 @@ export class WorkflowBlockHandler implements BlockHandler { childWorkflowSnapshotId, childWorkflowInstanceId: instanceId, }) + /** The warning above is this failure's log line. */ + 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..481b3b3ceb3 --- /dev/null +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -0,0 +1,123 @@ +import { createLogger } from '@sim/logger' +import { DrizzleQueryError } from 'drizzle-orm/errors' +import { describe, expect, it } from 'vitest' +import { + classifyFailure, + inheritFailureMarks, + logFailureOnce, + markFailureKind, + markFailureLogged, +} 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') + +/** 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')) + const wrapped = markFailureKind(new Error('Block failed', { cause: queryError }), 'user') + 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') + }) + + 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('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') + }) + + 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(outerBoundary(first)).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(outerBoundary(boundary)).toBeUndefined() + expect(classifyFailure(boundary)).toBe('internal') + + const authored = inheritFailureMarks(new Error('wrapped'), new UserFailure('missing input')) + expect(outerBoundary(authored)).toBe('user') + }) +}) + +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(outerBoundary(frozen)).toBeUndefined() + expect(classifyFailure(frozen)).toBe('user') + }) + + it('does not treat an unrelated error as logged', () => { + markFailureLogged(new Error('logged elsewhere')) + 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') + expect(outerBoundary(persistentFault, 'exec-1')).toBe('internal') + + expect(outerBoundary(persistentFault, 'exec-1')).toBeUndefined() + expect(outerBoundary(persistentFault, 'exec-2')).toBe('internal') + 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 new file mode 100644 index 00000000000..63a3b5d3cc1 --- /dev/null +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -0,0 +1,184 @@ +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' +import { CredentialRevokedError } from '@/lib/oauth/credential-revoked' + +/** + * 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 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. + */ +type FailureKind = 'user' | 'third_party_client' | 'third_party_server' | 'internal' + +/** Same bound `readStatusCode` puts on its cause walk. */ +const MAX_CAUSE_DEPTH = 8 + +/** + * 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 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>() + +/** 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 +} + +/** 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 = 'cause' in current ? current.cause : undefined + } + return chain +} + +/** 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 +} + +/** + * 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} 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' + const chain = causeChain(error) + if (chain.some(isRetryableSetupError)) return 'internal' + + for (const link of chain) { + const marked = failureKinds.get(link) + if (marked) return marked + /** 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' + } + + 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' + } + } + + return 'internal' +} + +/** + * 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) +} + +/** + * 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 + return isKeyable(output) && loggedCarriers.has(output) + ? inheritFailureMarks(error, output) + : error +} + +/** + * 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. + */ +function wasFailureLogged(error: unknown, executionId?: string): boolean { + return causeChain(error).some( + (link) => + loggedCarriers.has(link) || + (executionId !== undefined && loggedInExecution.get(link)?.has(executionId) === true) + ) +} + +const LOG_LEVEL_BY_KIND = { + user: 'info', + third_party_client: 'warn', + third_party_server: 'warn', + 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. Returns the attribution it logged with, or `undefined` when it skipped. + */ +export function logFailureOnce( + logger: Logger, + message: string, + error: unknown, + { metadata, executionId }: LogFailureOptions = {} +): FailureKind | undefined { + if (wasFailureLogged(error, executionId)) return undefined + const failureKind = classifyFailure(error) + const fields = typeof metadata === 'function' ? metadata() : metadata + logger[LOG_LEVEL_BY_KIND[failureKind]](message, { + ...(executionId ? { executionId } : {}), + ...fields, + failureKind, + }) + /** An empty id (a request that failed before minting one) scopes nothing. */ + 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/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/function-execution/execute-request.ts b/apps/sim/lib/function-execution/execute-request.ts index 89348be909d..05584300c7a 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,11 +3301,15 @@ export async function executeFunctionRequest( errorDisplayCode ) - const detailLogFn = isSystemError ? logger.error.bind(logger) : logger.warn.bind(logger) - detailLogFn(`[${requestId}] Enhanced error details`, { - line: enhancedError.line, - column: enhancedError.column, - }) + /** 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, + hasStack: Boolean(ivmError.stack), + line: enhancedError.line, + column: enhancedError.column, + }) + } return functionJsonResponse( { @@ -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/internal/tool-operations/execute-json-operation.ts b/apps/sim/lib/internal/tool-operations/execute-json-operation.ts index 10cc01c903b..0809f7d11be 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.ts b/apps/sim/lib/workflows/executor/execution-core.ts index 1fafc7cb337..e5e1a8780fe 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -14,6 +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 } from '@/lib/core/errors/failure-log' +import { UserFailure } from '@/lib/core/errors/user-failure' import { getExecutionDeadlineAt, getTimeoutErrorMessage, @@ -874,9 +876,7 @@ 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 new UserFailure('No start block found. Add a start block to this workflow.') } resolvedTriggerBlockId = startBlock.blockId @@ -1346,15 +1346,16 @@ async function executeWorkflowCoreImpl( return result } catch (error: unknown) { - const errorCause = describeErrorCause(error) - logger.error( - `[${requestId}] Execution failed:`, - projectResolvedSecretDiagnosticError( - error, - resolvedSecretTraceRegistry, - errorCause ? { cause: errorCause } : undefined - ) - ) + 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 new file mode 100644 index 00000000000..e934af33677 --- /dev/null +++ b/apps/sim/serializer/errors.ts @@ -0,0 +1,8 @@ +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(', ')}`) + } +} diff --git a/apps/sim/serializer/index.test.ts b/apps/sim/serializer/index.test.ts index e85f9325295..81cf7cc0bd1 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,15 @@ describe('Serializer', () => { undefined, true ) - }).toThrow('Test Jina Block is missing required fields: API Key') + } catch (error) { + refusal = error + } + 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') }) it.concurrent('should not validate user-or-llm fields during serialization', () => { 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 3ef04348c3f..a9d8386f09d 100644 --- a/apps/sim/tools/index.test.ts +++ b/apps/sim/tools/index.test.ts @@ -1,4 +1,6 @@ +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' import { authInternalMock, authInternalMockFns } from '@sim/testing/mocks/auth-internal.mock' import { billingUsageLogMock } from '@sim/testing/mocks/billing-usage-log.mock' @@ -419,7 +421,11 @@ vi.mock('@/tools/utils.server', async (importOriginal) => { }) import type { QueryClient } from '@tanstack/react-query' +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' +import type { SerializedBlock } from '@/serializer/types' import { executeTool, postProcessToolOutput } from '@/tools' import { tools } from '@/tools/registry' import { createToolConfig, getTool } from '@/tools/utils' @@ -440,6 +446,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 @@ -1113,6 +1128,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', @@ -1610,8 +1697,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) } ) @@ -1765,7 +1852,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'], }), { @@ -1793,8 +1881,17 @@ 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) + 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 @@ -1831,8 +1928,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 +2722,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -2668,6 +2768,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -2707,6 +2810,9 @@ describe('executeTool Function', () => { mockToolsLogger.error.mockImplementation(() => { throw originalError }) + mockToolsLogger.warn.mockImplementation(() => { + throw originalError + }) const execution = executeTool( 'function_execute', @@ -5781,6 +5887,68 @@ 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 }) + } + + 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({ error: 'rejected' }, status)) + expect(classifyFailure(blockError)).toBe(kind) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined() + } + ) + + 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({ ok: false })) + expect(classifyFailure(blockError)).toBe(kind) + expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', blockError)).toBeUndefined() + } 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..e86fecee0fd 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' @@ -17,6 +17,13 @@ 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, + markFailureKind, + markFailureLogged, +} from '@/lib/core/errors/failure-log' +import { isRetryableNetworkError } from '@/lib/core/errors/retryable-infrastructure' import { createTimeoutAbortController, DEFAULT_EXECUTION_TIMEOUT_MS, @@ -1053,31 +1060,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 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') +} + +function validateRequestBodySize(body: string | undefined): void { + if (body && Buffer.byteLength(body, 'utf8') > MAX_REQUEST_BODY_SIZE_BYTES) { + throw bodySizeLimitError() } } @@ -1098,51 +1088,16 @@ 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 { - 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 { @@ -1196,6 +1151,44 @@ async function readToolResponseBody( } } +/** + * 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) + /** 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 +} + +/** + * 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' + ) +} + +/** + * 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 @@ -1838,12 +1831,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,32 +2320,41 @@ 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( - { - ...(databaseErrorCause - ? { cause: databaseErrorCause } - : { - error: normalizedError.message, - stack: error instanceof Error ? error.stack : undefined, - }), - }, - resolvedSecretTraceRegistry, - { - errorName: normalizedError.name, - hasStack: !databaseErrorCause && Boolean(error instanceof Error && error.stack), - ...(databaseErrorCause ? { cause: databaseErrorCause } : {}), - }, - structuralOnlyToolLogs - ) - ) + 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 = toRecord(params._context) + 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, + ...(externalHttpFailures.has(error) ? { errorData: error.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' @@ -2422,10 +2428,10 @@ async function executeToolImplementation( normalizedToolId === 'function_execute' ? readFunctionSandboxCost(responseData) : undefined return { success: false, - output: { + 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 @@ -2659,14 +2665,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() @@ -2708,7 +2718,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 @@ -2814,7 +2824,7 @@ async function executeDeclaredInternalOperation({ } catch { errorData = errorText } - throw createTransformedErrorFromErrorInfo( + throw createInternalOperationFailure( { status: response.status, statusText: response.statusText, data: errorData }, tool.errorExtractor ) @@ -2832,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, @@ -2842,7 +2857,6 @@ async function executeToolRequest( resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry ): Promise { const requestId = generateRequestId() - const structuralOnlyToolLogs = false try { const requestParams = prepareToolRequest(tool, params, resolvedSecretTraceRegistry) const { headers } = requestParams @@ -2850,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) { @@ -2860,7 +2874,7 @@ async function executeToolRequest( } } - validateRequestBodySize(requestParams.body, requestId, toolId) + validateRequestBodySize(requestParams.body) const headersRecord: Record = {} headers.forEach((value, key) => { @@ -2882,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 } @@ -2910,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) } }, }) @@ -3023,47 +3037,14 @@ 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() } - 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 } @@ -3078,17 +3059,11 @@ 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 - ) + /** 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' ) - throw new Error(`Failed to parse response from ${toolId}: ${jsonError}`) } } } @@ -3096,86 +3071,64 @@ 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 createExternalHttpFailure(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 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') + 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 { @@ -3184,28 +3137,11 @@ async function executeToolRequest( error: undefined, } } catch (error: any) { - handleResponseSizeLimitError(error, requestId, toolId) + handleResponseSizeLimitError(error) - handleBodySizeLimitError( - error, - requestId, - toolId, - resolvedSecretTraceRegistry, - structuralOnlyToolLogs - ) + handleBodySizeLimitError(error) - 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 } @@ -3295,7 +3231,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({ @@ -3386,17 +3322,34 @@ 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)) { - 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: () => ({ + ...runIdentity, + ...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, @@ -3407,26 +3360,28 @@ 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: () => ({ + ...runIdentity, + ...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, 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,