Skip to content

Commit fe0bc05

Browse files
committed
test(logs): attribute the serializer refusal at its owner and pin block log redaction
Moves the missing-required-fields attribution check to the serializer that throws it, drops an execution-core case whose internal row passed by default, and covers the block failure line's secret projection now that it, not the Agent handler, logs provider errors. Tightens comments the diff added.
1 parent 249a2fd commit fe0bc05

9 files changed

Lines changed: 59 additions & 40 deletions

File tree

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,48 @@ describe('BlockExecutor', () => {
625625
expect(blockFailureLogsSince(loggerIndex)).toHaveLength(2)
626626
})
627627

628+
it.each([
629+
['projects secrets and runtime identifiers out of', false],
630+
['fails closed to a structural', true],
631+
] as const)('%s the block failure line for a provider error', async (_name, incomplete) => {
632+
const secret = 'block-failure-secret'
633+
/** The Agent handler hands its provider error registry to the block executor this way. */
634+
const errorRegistry = new ResolvedSecretTraceRegistry([
635+
{ name: 'TOKEN', plaintext: secret, encryptedValue: 'encrypted-block-failure-secret' },
636+
])
637+
errorRegistry.recordResolved('TOKEN', secret)
638+
if (incomplete) errorRegistry.markIncomplete('unspecified')
639+
const block = createBlock()
640+
const workflow: SerializedWorkflow = {
641+
version: '1',
642+
blocks: [block],
643+
connections: [],
644+
loops: {},
645+
parallels: {},
646+
}
647+
const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length
648+
const state = new ExecutionState()
649+
const handler: BlockHandler = {
650+
canHandle: () => true,
651+
execute: async (ctx) => {
652+
ctx.errorResolvedSecretTraceRegistry = errorRegistry
653+
throw new Error(`provider failed with ${secret} __var_TOKEN __sim_runtime_test_1`)
654+
},
655+
}
656+
const executor = new BlockExecutor(
657+
[handler],
658+
new VariableResolver(workflow, {}, state),
659+
{},
660+
state
661+
)
662+
663+
await executor.execute(createContext(state), createNode(block), block).catch(() => undefined)
664+
665+
const logged = JSON.stringify(blockFailureLogsSince(loggerIndex))
666+
expect(logged).toContain('Block execution failed')
667+
for (const leaked of [secret, '__var_', '__sim_']) expect(logged).not.toContain(leaked)
668+
})
669+
628670
it('logs an internal child workflow fault with the block and run identity', async () => {
629671
const loggerIndex = blockExecutorBaseLogger.withMetadata.mock.results.length
630672

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

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

792-
/** Projected only when logged: a tool failure arrives already logged and skips it. */
792+
/** Lazy, so a failure a tool already logged skips the secret projection. */
793793
let errorDiagnostic: Record<string, unknown> | undefined
794794
const getErrorDiagnostic = () => {
795795
if (errorDiagnostic) return errorDiagnostic
@@ -887,7 +887,7 @@ export class BlockExecutor {
887887
executionTime: duration,
888888
},
889889
})
890-
/** A thrown primitive has no `cause` link back to the value logged above. */
890+
/** The raw thrown value is never marked logged, so the fresh block error carries the mark. */
891891
markFailureLogged(blockError)
892892
throw blockError
893893
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ function describeProviderTransportFailure(error: Error): string | null {
327327
return null
328328
}
329329

330-
/** A provider refusing the key it was given. */
331330
function isProviderKeyRejection(error: unknown): boolean {
332331
const status = (error as { status?: unknown } | null)?.status
333332
return status === 401 || status === 402 || status === 403

‎apps/sim/executor/handlers/human-in-the-loop/human-in-the-loop-handler.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,6 @@ export class HumanInTheLoopBlockHandler implements BlockHandler {
560560
const result = await executeTool(toolId, toolParams, { executionContext: ctx })
561561
const durationMs = Date.now() - startTime
562562

563-
/** `executeTool` already logged the failure with its tool id and attribution. */
564563
if (!result.success) {
565564
return {
566565
toolId,

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,6 @@ export class WorkflowBlockHandler implements BlockHandler {
12001200
? `Custom block execution failed (ref: ${ref})`
12011201
: 'Custom block execution failed'
12021202

1203-
/** No `cause` crosses the boundary, so the failure's marks are carried explicitly. */
12041203
return inheritFailureMarks(
12051204
new ChildWorkflowError({
12061205
message,

‎apps/sim/lib/core/errors/failure-log.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ export function markFailureKind<T>(error: T, kind: FailureKind): T {
5757
}
5858

5959
/**
60-
* Attributes `error` from its cause chain. A database failure anywhere is always internal,
61-
* then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError` status,
62-
* or an upstream `status` decides. Anything unattributed is internal.
60+
* Attributes `error` from its cause chain. A database or retryable setup failure is always
61+
* internal, then the outermost link with an explicit mark, a {@link UserFailure}, a Sim `HttpError`
62+
* status, or an upstream `status` decides. Anything unattributed is internal.
6363
*/
6464
export function classifyFailure(error: unknown): FailureKind {
6565
if (findDatabaseQueryError(error)) return 'internal'

‎apps/sim/lib/workflows/executor/execution-core.test.ts‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,11 @@ vi.mock('@/serializer', () => ({
162162
},
163163
}))
164164

165-
import { classifyFailure } from '@/lib/core/errors/failure-log'
166165
import {
167166
executeWorkflowCore,
168167
FINALIZED_EXECUTION_ID_TTL_MS,
169168
wasExecutionFinalizedByCore,
170169
} from '@/lib/workflows/executor/execution-core'
171-
import { MissingRequiredFieldsError } from '@/serializer/errors'
172170

173171
const uploadWorkflowInputMock = uploadsExecutionMockFns.mockUploadExecutionFile
174172
largeValueMetadataMockFns.mockRegisterLargeValueOwner.mockResolvedValue(true)
@@ -705,32 +703,6 @@ describe('executeWorkflowCore terminal finalization sequencing', () => {
705703
)
706704
})
707705

708-
it.each([
709-
[
710-
'a block missing a required field',
711-
new MissingRequiredFieldsError('Slack', ['Slack Account']),
712-
'user',
713-
],
714-
[
715-
'an unknown block type, a registry regression',
716-
new Error('Invalid block type: retired'),
717-
'internal',
718-
],
719-
] as const)('attributes a serializer refusal for %s', async (_name, refusal, kind) => {
720-
serializeWorkflowMock.mockImplementationOnce(() => {
721-
throw refusal
722-
})
723-
724-
const thrown = await executeWorkflowCore({
725-
snapshot: createSnapshot() as any,
726-
callbacks: {},
727-
loggingSession: loggingSession as any,
728-
}).catch((error: unknown) => error)
729-
730-
expect(thrown).toBe(refusal)
731-
expect(classifyFailure(thrown)).toBe(kind)
732-
})
733-
734706
it('activates trusted pre-execution provenance on the installed execution registry', async () => {
735707
executorExecuteMock.mockResolvedValue({
736708
success: true,

‎apps/sim/serializer/index.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
toolsUtilsMock,
2424
} from '@sim/testing/mocks'
2525
import { describe, expect, it, vi } from 'vitest'
26+
import { classifyFailure } from '@/lib/core/errors/failure-log'
2627
import { DAGBuilder } from '@/executor/dag/builder'
2728
import { Serializer } from '@/serializer/index'
2829
import { getToolMetadata, getToolParams } from '@/tools/metadata'
@@ -356,15 +357,22 @@ describe('Serializer', () => {
356357
enabled: true,
357358
}
358359

359-
expect(() => {
360+
let refusal: unknown
361+
try {
360362
serializer.serializeWorkflow(
361363
{ 'test-block': blockWithMissingUserOnlyField },
362364
[],
363365
{},
364366
undefined,
365367
true
366368
)
367-
}).toThrow('Test Jina Block is missing required fields: API Key')
369+
} catch (error) {
370+
refusal = error
371+
}
372+
expect(refusal).toBeInstanceOf(Error)
373+
expect((refusal as Error).message).toBe('Test Jina Block is missing required fields: API Key')
374+
/** The author's configuration, so execution logs it at info rather than paging at error. */
375+
expect(classifyFailure(refusal)).toBe('user')
368376
})
369377

370378
it.concurrent('should not validate user-or-llm fields during serialization', () => {

‎apps/sim/tools/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,7 @@ const RESPONSE_SIZE_LIMIT_ERROR_MESSAGE =
10591059
const SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE =
10601060
'External integration tools cannot target this Sim instance; use an internal operation'
10611061

1062-
/** The author's workflow data is too large to send; their fault, logged once by the tool catch. */
1062+
/** The author's data is too large to send, so it is a user failure. */
10631063
function bodySizeLimitError(): Error {
10641064
return markFailureKind(new Error(BODY_SIZE_LIMIT_ERROR_MESSAGE), 'user')
10651065
}
@@ -2424,7 +2424,7 @@ async function executeToolImplementation(
24242424
...errorDetails,
24252425
...(functionSandboxCost ? { cost: functionSandboxCost } : {}),
24262426
}
2427-
/** A handler rebuilding this result as a thrown error carries `output`, and with it both marks. */
2427+
/** Lets `adoptToolFailure` carry both marks onto a handler's rebuilt error. */
24282428
markFailureLogged(failureOutput)
24292429
markFailureKind(failureOutput, loggedKind ?? classifyFailure(error))
24302430
return {

0 commit comments

Comments
 (0)