Skip to content

Commit 9b1b3bc

Browse files
committed
fix(logs): keep run identity on every remaining failure line
Fixes the type error on the external failure body, attributes a provider error payload on a 2xx to the provider, and adds workflow, execution, and block ids to the MCP failure lines that replace the block executor's. The condition batch carries its failed result's marks and passes its block id. Pi GitHub calls carry no run identity, so their errors now carry only the tool layer's attribution and the block executor still logs them with ids. Restores the HITL notification warning, which covers soft failures the tool layer never logs, and attributes refused tool and proxy URLs to the author.
1 parent e998141 commit 9b1b3bc

6 files changed

Lines changed: 73 additions & 33 deletions

File tree

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,14 @@ type ConditionEvaluation =
4040
| { status: 'matched'; index: number }
4141
| { status: 'no-match' }
4242
| { status: 'expression-threw'; index: number; message: string }
43-
| { status: 'no-verdict'; message: string; retryable: boolean; timedOut: boolean }
43+
| {
44+
status: 'no-verdict'
45+
message: string
46+
retryable: boolean
47+
timedOut: boolean
48+
/** The failed tool result's output, so a terminal error carries its logged mark. */
49+
output?: unknown
50+
}
4451

4552
/**
4653
* Wraps one expression as a boolean test, on its own line so a trailing line
@@ -173,6 +180,7 @@ async function runConditionCode(
173180
userId: ctx.userId,
174181
isDeployedContext: ctx.isDeployedContext,
175182
enforceCredentialAccess: ctx.enforceCredentialAccess,
183+
blockId: currentNodeId,
176184
},
177185
},
178186
{ executionContext: ctx }
@@ -215,6 +223,7 @@ async function evaluateConditionList(
215223
message,
216224
retryable: result.retryable !== false,
217225
timedOut: isTimeoutFailure(result.error),
226+
output: result.output,
218227
}
219228
}
220229

@@ -515,8 +524,11 @@ export class ConditionBlockHandler implements BlockHandler {
515524
)
516525
case 'no-verdict':
517526
if (!evaluation.retryable) {
518-
throw new NonRetryableExecutionError(
519-
`Evaluation error in condition "${conditions[0].title}": ${evaluation.message}`
527+
throw adoptToolFailure(
528+
new NonRetryableExecutionError(
529+
`Evaluation error in condition "${conditions[0].title}": ${evaluation.message}`
530+
),
531+
evaluation
520532
)
521533
}
522534
// Retrying one branch at a time is what recovers a batch the sandbox
@@ -526,7 +538,7 @@ export class ConditionBlockHandler implements BlockHandler {
526538
// failure as it stands. The whole list was one call, so no single
527539
// branch owns that failure; name the first, where evaluation started.
528540
if (evaluation.timedOut || ctx.abortSignal?.aborted) {
529-
throw conditionError(conditions[0], evaluation.message)
541+
throw adoptToolFailure(conditionError(conditions[0], evaluation.message), evaluation)
530542
}
531543
logger.warn('Batched condition evaluation produced no verdict, retrying one at a time', {
532544
conditionCount: conditions.length,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,10 @@ export class HumanInTheLoopBlockHandler implements BlockHandler {
561561
const durationMs = Date.now() - startTime
562562

563563
if (!result.success) {
564+
logger.warn('Notification tool execution failed', {
565+
toolId,
566+
error: result.error,
567+
})
564568
return {
565569
toolId,
566570
title: toolConfig.title,

‎apps/sim/executor/handlers/pi/cloud/babysit/github.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getErrorMessage } from '@sim/utils/errors'
22
import { isRecordLike } from '@sim/utils/object'
33
import { truncate } from '@sim/utils/string'
4-
import { adoptToolFailure } from '@/lib/core/errors/failure-log'
4+
import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log'
55
import type { BabysitRoundDecision } from '@/executor/handlers/pi/cloud/babysit/round'
66
import {
77
fetchPrSnapshot,
@@ -24,6 +24,7 @@ import type {
2424
StatusCheckRollupContext,
2525
SubmittedReviewSummary,
2626
} from '@/tools/github/types'
27+
import type { ToolResponse } from '@/tools/types'
2728

2829
const MAX_PAGES = 10
2930
const MAX_COMMENTS_PER_THREAD = 50
@@ -205,6 +206,11 @@ function toolFailure(label: string, error: unknown): Error {
205206
)
206207
}
207208

209+
/** Carries the tool layer's attribution; these calls carry no run identity, so not its logged mark. */
210+
function attributedToolFailure(label: string, result: ToolResponse): Error {
211+
return markFailureKind(toolFailure(label, result.error), classifyFailure(result.output))
212+
}
213+
208214
function parseReviewThread(value: unknown, index: number): ReviewThread {
209215
if (!isRecordLike(value)) throw new Error(`Review thread ${index} must be an object`)
210216
const commentsValue = value.comments
@@ -289,8 +295,7 @@ export async function fetchBabysitThreads(
289295
},
290296
{ signal }
291297
)
292-
if (!result.success)
293-
throw adoptToolFailure(toolFailure('Failed to fetch review threads', result.error), result)
298+
if (!result.success) throw attributedToolFailure('Failed to fetch review threads', result)
294299
const output = result.output
295300
if (!isRecordLike(output) || !Array.isArray(output.threads)) {
296301
throw new Error('Review thread response is incomplete')
@@ -425,8 +430,7 @@ export async function fetchBabysitCheckState(
425430
},
426431
{ signal }
427432
)
428-
if (!result.success)
429-
throw adoptToolFailure(toolFailure('Failed to fetch checks', result.error), result)
433+
if (!result.success) throw attributedToolFailure('Failed to fetch checks', result)
430434
const output = result.output
431435
if (!isRecordLike(output) || !Array.isArray(output.contexts)) {
432436
throw new Error('Check response is incomplete')

‎apps/sim/executor/handlers/pi/cloud/shared.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* security-sensitive details.
66
*/
77

8-
import { adoptToolFailure } from '@/lib/core/errors/failure-log'
8+
import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log'
99
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
1010
import { resolvePiSandboxLifetimeMs } from '@/lib/execution/remote-sandbox/pi-lifetime'
1111
import { PI_EVENT_FILTER_PATH } from '@/executor/handlers/pi/cloud/event-filter-source'
@@ -224,9 +224,13 @@ export function scrubGitSecrets(text: string, token: string): string {
224224
}
225225

226226
/**
227-
* The error a backend throws for a failed GitHub tool call. Carries the tool layer's marks so the
228-
* failure `executeTool` already logged is not logged again at error.
227+
* The error a backend throws for a failed GitHub tool call. Carries the tool layer's attribution
228+
* but not its logged mark: these calls carry no run identity, so the block executor's line, which
229+
* does, must still be written.
229230
*/
230231
export function toolResultError(label: string, result: ToolResponse): Error {
231-
return adoptToolFailure(new Error(`${label}: ${result.error ?? 'unknown error'}`), result)
232+
return markFailureKind(
233+
new Error(`${label}: ${result.error ?? 'unknown error'}`),
234+
classifyFailure(result.output)
235+
)
232236
}

‎apps/sim/executor/handlers/pi/core/redaction.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getErrorMessage } from '@sim/utils/errors'
2-
import { inheritFailureMarks } from '@/lib/core/errors/failure-log'
2+
import { classifyFailure, markFailureKind } from '@/lib/core/errors/failure-log'
33
import type { PiEvent } from '@/executor/handlers/pi/core/events'
44

55
/**
@@ -38,13 +38,16 @@ export function getScrubbedPiErrorMessage(
3838
}
3939

4040
/**
41-
* Creates a boundary-safe error without retaining a potentially secret-bearing cause. The failure
42-
* marks still cross, so a GitHub tool failure the tool layer logged is not logged again.
41+
* Creates a boundary-safe error without retaining a potentially secret-bearing cause. The failure's
42+
* attribution still crosses, so a GitHub 404 is not logged as a Sim fault.
4343
*/
4444
export function createScrubbedPiError(
4545
error: unknown,
4646
secrets: readonly string[],
4747
fallback?: string
4848
): Error {
49-
return inheritFailureMarks(new Error(getScrubbedPiErrorMessage(error, secrets, fallback)), error)
49+
return markFailureKind(
50+
new Error(getScrubbedPiErrorMessage(error, secrets, fallback)),
51+
classifyFailure(error)
52+
)
5053
}

‎apps/sim/tools/index.ts‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,12 +1156,13 @@ async function readToolResponseBody(
11561156
* is logged: an internal operation's or a Function's body carries the author's data, stdout, and
11571157
* source lines.
11581158
*/
1159-
const externalHttpFailures = new WeakSet<Error>()
1159+
const externalHttpFailures = new WeakSet<object>()
11601160

11611161
function createExternalHttpFailure(errorInfo?: ErrorInfo, extractorId?: string): Error {
11621162
const failure = createTransformedErrorFromErrorInfo(errorInfo, extractorId)
11631163
externalHttpFailures.add(failure)
1164-
return failure
1164+
/** An error payload on a 2xx carries no status but is still the provider refusing. */
1165+
return errorInfo?.status === undefined ? markFailureKind(failure, 'third_party_client') : failure
11651166
}
11661167

11671168
/**
@@ -2338,9 +2339,7 @@ async function executeToolImplementation(
23382339
: {
23392340
error: normalizedError.message,
23402341
stack: error instanceof Error ? error.stack : undefined,
2341-
...(error instanceof Error && externalHttpFailures.has(error)
2342-
? { errorData: error.data }
2343-
: {}),
2342+
...(externalHttpFailures.has(error) ? { errorData: error.data } : {}),
23442343
}),
23452344
},
23462345
resolvedSecretTraceRegistry,
@@ -2843,6 +2842,11 @@ async function executeDeclaredInternalOperation({
28432842
}
28442843
}
28452844

2845+
/** A tool or proxy URL the author configured that Sim refuses to call; the author's to fix. */
2846+
function invalidToolTarget(message: string): Error {
2847+
return markFailureKind(new Error(message), 'user')
2848+
}
2849+
28462850
/** Executes one external tool request with DNS validation and IP pinning. */
28472851
async function executeToolRequest(
28482852
toolId: string,
@@ -2860,7 +2864,7 @@ async function executeToolRequest(
28602864
const targetsThisSimInstance = isSelfOriginUrl(fullUrl)
28612865

28622866
if (targetsThisSimInstance && tool.request.allowSameOrigin !== true) {
2863-
throw new Error(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE)
2867+
throw invalidToolTarget(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE)
28642868
}
28652869

28662870
if (targetsThisSimInstance) {
@@ -2892,14 +2896,14 @@ async function executeToolRequest(
28922896
try {
28932897
const urlValidation = await validateUrlWithDNS(fullUrl, 'toolUrl', 'requestTarget')
28942898
if (!urlValidation.isValid) {
2895-
throw new Error(`Invalid tool URL: ${urlValidation.error}`)
2899+
throw invalidToolTarget(`Invalid tool URL: ${urlValidation.error}`)
28962900
}
28972901

28982902
let proxyOption: string | undefined
28992903
if (requestParams.proxyUrl) {
29002904
const proxyValidation = await validateAndPinProxyUrl(requestParams.proxyUrl)
29012905
if (!proxyValidation.isValid) {
2902-
throw new Error(`Invalid proxy URL: ${proxyValidation.error}`)
2906+
throw invalidToolTarget(`Invalid proxy URL: ${proxyValidation.error}`)
29032907
}
29042908
proxyOption = proxyValidation.pinnedProxyUrl
29052909
}
@@ -2920,7 +2924,7 @@ async function executeToolRequest(
29202924
? undefined
29212925
: (redirectUrl) => {
29222926
if (isSelfOriginUrl(redirectUrl)) {
2923-
throw new Error(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE)
2927+
throw invalidToolTarget(SAME_ORIGIN_EXTERNAL_TOOL_ERROR_MESSAGE)
29242928
}
29252929
},
29262930
})
@@ -3314,6 +3318,13 @@ async function executeMcpTool(
33143318
const endTimeISO = endTime.toISOString()
33153319
const duration = endTime.getTime() - new Date(actualStartTime).getTime()
33163320

3321+
/** These lines replace the block executor's, so they carry the run identity themselves. */
3322+
const blockId = toRecord(params._context).blockId
3323+
const runIdentity = {
3324+
workflowId: context?.workflowId,
3325+
executionId: context?.executionId,
3326+
blockId: typeof blockId === 'string' ? blockId : undefined,
3327+
}
33173328
const errorMsg = toError(error).message
33183329
if (isBodySizeLimitError(errorMsg)) {
33193330
const failure = bodySizeLimitError()
@@ -3322,14 +3333,14 @@ async function executeMcpTool(
33223333
`[${actualRequestId}] Request body size limit exceeded for mcp:${toolId}:`,
33233334
failure,
33243335
{
3325-
metadata: () =>
3326-
projectToolLogMetadata(
3336+
metadata: () => ({
3337+
...runIdentity,
3338+
...projectToolLogMetadata(
33273339
{ originalError: errorMsg },
33283340
context?.resolvedSecretTraceRegistry,
3329-
{
3330-
hasOriginalError: errorMsg.length > 0,
3331-
}
3341+
{ hasOriginalError: errorMsg.length > 0 }
33323342
),
3343+
}),
33333344
}
33343345
)
33353346
return {
@@ -3346,8 +3357,9 @@ async function executeMcpTool(
33463357

33473358
const normalizedError = toError(error)
33483359
logFailureOnce(logger, `[${actualRequestId}] Error executing MCP tool ${toolId}:`, error, {
3349-
metadata: () =>
3350-
projectToolLogMetadata(
3360+
metadata: () => ({
3361+
...runIdentity,
3362+
...projectToolLogMetadata(
33513363
{
33523364
error: normalizedError.message,
33533365
stack: error instanceof Error ? error.stack : undefined,
@@ -3358,6 +3370,7 @@ async function executeMcpTool(
33583370
hasStack: Boolean(error instanceof Error && error.stack),
33593371
}
33603372
),
3373+
}),
33613374
})
33623375

33633376
const errorMessage = getErrorMessage(error, `Failed to execute MCP tool ${toolId}`)

0 commit comments

Comments
 (0)