Skip to content

improvement(logs): log each execution failure once at its owning boundary - #8872

Merged
waleedlatif1 merged 10 commits into
stagingfrom
improvement/failure-log-once
Oct 10, 2026
Merged

waleedlatif1 merged 10 commits into
stagingfrom
improvement/failure-log-once

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • One external tool failure (a third-party 4xx from user input) logged at ERROR up to eight times on its way out: twice in the tool layer, then in the outer tool catch, the block executor, the engine (twice), execution-core, and the trigger surface (webhook / workflow / SSE / execute service). The same failure now produces one WARN line, written by the boundary that owns it.

  • Policy: each failure logs once, at its owning boundary, at a severity set by its cause.

    • user input, configuration, or code → info
    • third-party 4xx / 5xx / network → warn
    • Sim's own faults → error
  • lib/core/errors/failure-log.ts:

    • classifyFailure walks the cause chain. A DB query error or RetryableSetupError is always internal. After that, an explicit mark, UserFailure, a Sim HttpError status, or an upstream status decides. Anything unattributed stays internal.
    • logFailureOnce skips a failure an inner boundary already logged. It builds metadata lazily, so a skipped boundary does no secret projection.
  • Logged marks are never placed on the raw thrown value, because a persistent fault (a rejected dynamic import(), a memoized rejected promise) rethrows one object forever. Marks go only on carriers created during the propagation:

    • the flattened tool failure's output
    • the block error
    • a handler's rebuilt error, via adoptToolFailure
    • the custom-block boundary error, via inheritFailureMarks

    Execution-level boundaries scope a raw value's mark to their execution id.

  • Owning boundaries:

    • executeTool logs tool failures with toolId, workflowId, executionId, blockId, and status. Only the external HTTP path also logs its redacted response body; in-process operation and Function bodies, which carry user data and stdout, are never logged.
    • The block executor logs every other block failure with block and run identity.
    • The Agent, child-workflow, and condition handlers no longer log what the block executor logs.
    • The Function route logs only isolate failures; author code errors are logged once by the tool layer.
    • MCP failures log once and mark their output, so the block executor doesn't repeat them.
    • Sandbox failure logs are unchanged: the adapters can't yet tell a provider exception from a non-zero exit.
  • Attribution fixes:

    • In-process operations are Sim's own: 4xx → user (a Function's 422 is the author's code), 5xx → internal. Before, they read as third-party.
    • MissingRequiredFieldsError, BoundarySafeError, and "no start block" are UserFailure. An unknown block type stays internal.
    • Required tool params missing, custom tool param validation, and condition expressions that throw → user.
    • A transformResponse plain Error (Slack-style ok: false) → third-party client.
  • Kept at ERROR on purpose:

    • DB errors, even beneath a user mark
    • RetryableSetupError
    • HostedKeyUnavailableError
    • a hosted key being rejected or throttled (401/403/429/503 per classifyHostedKeyFailure)
    • provider 401/402/403 on the Agent path, since the handler can't tell a hosted key from the author's own
    • transform TypeErrors
    • in-process operation 5xx; sandbox failures; isolated-vm system errors
    • anything unattributed
  • Also removed double logs on the request/response size-limit and JSON-parse paths, and through the scrubbed Pi error boundary. executeInternalJsonToolOperation now logs the real cause (DB code, stack) before flattening it to a 500.

  • Run logs, trace spans, thrown errors, tool results, and API responses are unchanged. Only server log calls and their levels moved. Block log error, error outputs, ChildWorkflowError trace spans, attachExecutionResult, and finalization paths are untouched.

What to watch in CloudWatch after deploy

  • ERROR lines for "External tool error" / "External request error" disappear; "Error executing tool", "Block execution failed", "Execution failed", and "Webhook execution failed" drop sharply at ERROR and appear once at WARN/INFO with a failureKind field.
  • What remains at ERROR should be Sim faults. Filter failureKind = "internal" and confirm the remaining lines carry ids plus message/stack.
  • Infra consumers: the only app-log metric filters are the RouteHandler latency filter and a canary marker. Neither keys on these messages or levels.

Type of Change

  • Improvement

Testing

  • New tests:
    • lib/core/errors/failure-log.test.ts: classifier precedence, cycles, frozen errors, inheritFailureMarks, execution-scoped raw marks.
    • Real ApiBlockHandler over real executeTool: upstream 404/503, transform plain Error vs TypeError.
    • The same thrown object from two tool calls / two blocks logs twice.
    • In-process 400/500 attribution.
    • No in-process body, Function stdout, source line, or stack in any log level.
    • An internal child fault is left for the block executor, whose line carries ids + message.
    • The block failure line projects or fails closed for secrets.
    • Serializer refusal attributed at its owner.
    • Real DAGExecutor run reaches the run boundary already logged.
  • Each new regression test was shown red with its guard reverted.
  • Existing secret-leak assertions in tools/index.test.ts widened from error to every log level.
  • bunx biome check on changed files. Full lint, type-check, check:audits, and suites run in CI.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Oct 10, 2026 3:13am UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 37 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/executor/execution/engine.ts Outdated
Comment thread apps/sim/executor/execution/block-executor.ts
Comment thread apps/sim/lib/core/errors/failure-log.ts Outdated
Comment thread apps/sim/executor/handlers/pi/cloud/authoring/backend.ts
Comment thread apps/sim/background/webhook-execution.ts
Comment thread apps/sim/executor/handlers/condition/condition-handler.ts
Comment thread apps/sim/lib/execution/remote-sandbox/index.ts Outdated
Comment thread apps/sim/tools/index.ts
Comment thread apps/sim/tools/index.ts Outdated
Comment thread apps/sim/tools/index.ts Outdated
@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable defect was established.

Summary

This PR moves failure logs toward the boundary that owns each failure. It adds cause-based log levels, private logged marks, and execution-scoped duplicate suppression.

  • Tool failures get one log at the level their cause calls for.
  • The block executor owns failures that handlers pass upward.
  • Outer execution surfaces skip failures already logged inside the run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Failure] --> B{Tool owns it?}
  B -->|Yes| C[Tool logs and marks returned output]
  C --> D[Handler carries marks onto error]
  B -->|No| E[Block logs and marks fresh error]
  D --> F[Outer boundaries check marks]
  E --> F
  F --> G[Skip an already logged failure]
  F --> H[Log an unmarked failure for this execution]
Loading

Reviews (8) · Last reviewed commit: "fix(logs): keep one logged mark per exec..." · Reviewed by Greptile

Comment thread apps/sim/lib/internal/tool-operations/execute-json-operation.ts
Comment thread apps/sim/tools/index.ts Outdated
Comment thread apps/sim/tools/index.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 force-pushed the improvement/failure-log-once branch from fe0bc05 to d753b48 Compare October 10, 2026 01:52
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 38 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/tools/index.ts Outdated
Comment thread apps/sim/tools/index.ts
Comment thread apps/sim/executor/handlers/workflow/workflow-handler.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/executor/handlers/condition/condition-handler.ts
Comment thread apps/sim/tools/index.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 38 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/executor/handlers/function/function-handler.ts
Comment thread apps/sim/tools/index.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 force-pushed the improvement/failure-log-once branch from 8f204a0 to 1c503e3 Compare October 10, 2026 02:11
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 38 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/tools/index.ts
Comment thread apps/sim/executor/handlers/pi/cloud/github-pr.ts
@waleedlatif1
waleedlatif1 force-pushed the improvement/failure-log-once branch from 1c503e3 to 3d0be98 Compare October 10, 2026 02:31
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 37 files

Confidence score: 4/5

  • In apps/sim/tools/index.ts, malformed JSON from an external tool is classified as internal, so provider-response or endpoint-configuration failures are logged at ERROR. Classify the parse failure at its source.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/tools/index.ts">

<violation number="1" location="apps/sim/tools/index.ts:2324">
P2: Malformed JSON from an external tool remains classified as `internal`, so this boundary logs a provider response or endpoint-configuration failure at ERROR. Mark the parse failure at its source with the appropriate user or third-party kind.</violation>
</file>

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/tools/index.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 force-pushed the improvement/failure-log-once branch from 3d0be98 to c747151 Compare October 10, 2026 02:42
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 37 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/core/errors/failure-log.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

…dary

A single external tool failure was logged at ERROR up to eight times as it
propagated from the tool layer through the block executor, the engine,
execution-core, and the trigger surfaces. Each boundary now calls
logFailureOnce, which skips a failure an inner boundary already logged and
sets severity by attribution: the author's input, configuration, or code at
info, a third-party 4xx or 5xx at warn, and Sim's own faults (database,
retryable setup, hosted-key rejection, anything unattributed) at error.

The logged mark and attribution live in WeakMap/WeakSet side tables keyed by
the thrown value, walked through the cause chain, and carried on the
flattened tool failure's output so they survive a handler rebuilding a failed
tool result as a new error. Run logs and trace spans are unchanged.
Asserts the block executor's thrown error is already logged and attributed
(database failure internal, user Stop user), switches the tool-boundary test
to the central jsonResponse helper, and updates exact-argument log
assertions for the added failureKind and executionId fields.
…process failures

Never mark a raw thrown value as logged: a persistent fault that rethrows one
object (a rejected dynamic import, a memoized rejected promise) was logged once
per process and then silenced everywhere. Only carriers created during the
propagation (the flattened tool failure output, the block error, a handler's
rebuilt error) carry the mark, and execution-level boundaries scope a raw
value's mark to their execution id.

Handlers that rebuild a failed tool result now call adoptToolFailure instead of
relying on the result output being copied by reference. The child workflow and
agent handlers no longer log failures the block executor logs with the block's
and run's identity. In-process operations are attributed to Sim (4xx user, 5xx
internal) rather than to a third party, only external HTTP failures log their
response body, a missing required field is the only serializer refusal at info,
and the size-limit and JSON-parse paths no longer log twice.
…r failures by type

logFailureOnce takes its metadata as a thunk so a skipped boundary does no
secret projection, adds the execution id itself, and returns the attribution
it logged with. UserFailure attributes author-caused failures by type
(missing required fields, boundary-safe custom block refusals, no start
block) wherever they surface, replacing a try/catch at one serializer caller.
One inheritFailureMarks carries both marks across a boundary that drops
cause, the hosted-key check reuses classifyHostedKeyFailure, the Pi backends
share one toolResultError, and the sandbox and Function route no longer log
author code failures the tool boundary already logs.
…ck 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.
Reads status and cause without casts, keeps the thrown missing-required-fields
error's name and stack unchanged, ignores an empty execution id when scoping a
logged mark, unexports wasFailureLogged and FailureKind (tests observe the
outer boundary through logFailureOnce instead), and shrinks the explicit-any
baseline for the Agent handler.
Carries failure marks through the scrubbed Pi error and normalizes a
non-Error node failure before logging so the engine sees its mark; drops the
condition handler's and response-size handler's own error lines in favor of
the owning boundary; logs MCP failures once and marks their output; passes
the block id to the API and Function tool calls so the tool line carries it;
attributes custom tool parameter validation and condition expression errors
to the author; checks the whole cause chain for a retryable setup failure
before honoring a mark; and restores the sandbox failure logs, whose adapters
cannot yet tell a provider exception from a non-zero exit.
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.
A CredentialRevokedError anywhere in the cause chain classifies as a user
failure, so the boundaries that log it after token resolution's WARN write
INFO rather than ERROR.
…le responses

A raw value logged at an execution boundary now remembers every execution
that logged it (bounded), so overlapping runs sharing one persistent
rejection each log it once. A 2xx body that is not JSON is the endpoint's
failure, not Sim's.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 force-pushed the improvement/failure-log-once branch from b96426d to 5a86b02 Compare October 10, 2026 03:07
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 37 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 47fb805 into staging Oct 10, 2026
48 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/failure-log-once branch October 10, 2026 03:27

This branch was successfully deployed

1 active deployment
Preview — 5a86b023 Deployed Oct 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant