Skip to content

[superlog] Use warn instead of error for AI tool call failures in request logger#487

Open
superlog-app[bot] wants to merge 1 commit into
stagingfrom
superlog/fix-tool-error-escalates-job-log-level
Open

[superlog] Use warn instead of error for AI tool call failures in request logger#487
superlog-app[bot] wants to merge 1 commit into
stagingfrom
superlog/fix-tool-error-escalates-job-log-level

Conversation

@superlog-app

@superlog-app superlog-app Bot commented Jun 17, 2026

Copy link
Copy Markdown

Summary

Insights jobs that completed successfully were being logged at ERROR level whenever any AI tool call failed during execution (e.g. the agent generating invalid SQL, or create_annotation receiving a domain name instead of a UUID). This produced false-positive ERROR incidents even though the job recovered and generated insights successfully.

The root cause is in createToolLogger.error() in packages/ai/src/ai/tools/utils/logger.ts. When a tool fails, it called requestLogger.error(err, context). In evlog, RequestLogger.error() permanently sets hasError = true on the request-scoped logger. The final logger.emit() call then uses level = hasError ? "error" : hasWarn ? "warn" : "info", so even a job_status: "succeeded" event was emitted at ERROR level.

The fix changes the request-logger call from requestLogger.error(err, context) to requestLogger.warn(message, context). Tool-level failures set hasWarn = true instead of hasError = true, so succeeded jobs are emitted at warn or info level. Actual job failures (caught in jobs.ts catch block, which explicitly calls logger.error(err)) remain at ERROR level. The error details (message, SQL, context) are still captured in the wide event via the merged context parameter.

Alternative: use requestLogger.info() if even WARN-level events for jobs-with-tool-failures are unwanted. That would produce INFO regardless of tool failures, at the cost of slightly less signal on partial failures.

Incident on Superlog


Was this PR helpful? Leave feedback — goes straight to the Superlog team.


Summary by cubic

Downgraded tool-call failures from ERROR to WARN in the request logger to stop successful insight jobs from emitting ERROR-level events and triggering false incidents.

  • Bug Fixes
    • In packages/ai/src/ai/tools/utils/logger.ts, changed requestLogger.error(err, ...) to requestLogger.warn(message, ...) in createToolLogger.error.
    • Keeps full context on the event; only actual job failures still log at ERROR.

Written for commit 8c73ced. Summary will update on new commits.

Review in cubic

@vercel

vercel Bot commented Jun 17, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
databuddy-status Ready Ready Preview, Comment Jun 17, 2026 9:51am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
dashboard Skipped Skipped Jun 17, 2026 9:51am
documentation Skipped Skipped Jun 17, 2026 9:51am

@unkey-deploy

unkey-deploy Bot commented Jun 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Unkey Deploy

Name Status Preview Inspect Updated (UTC)
api (preview) Ready Visit Preview Inspect Jun 17, 2026 9:52am

@blacksmith-sh

blacksmith-sh Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Found 7 test failures on Blacksmith runners:

Failures

Test View Logs
[chromium] › test/e2e/specs/regressions/
api-keys.spec.ts:4:5 › creates and deletes an API key without leaving confirmation dial
ogs open @regression @core
View Logs
[chromium] › test/e2e/specs/regressions/
website-analytics.spec.ts:31:5 › shows seeded analytics data and applies a topbar filte
r @regression @core
View Logs
[chromium] › test/e2e/specs/regressions/
website-analytics.spec.ts:7:5 › renders analytics controls in the website topbar @regre
ssion @core
View Logs
[chromium] › test/e2e/specs/smoke/
account.spec.ts:3:5 › updates the signed-in user's profile name @smoke
View Logs
[chromium] › test/e2e/specs/smoke/
auth.spec.ts:12:5 › boots an authenticated browser session @smoke
View Logs
[chromium] › test/e2e/specs/smoke/
auth.spec.ts:30:5 › signs out and protects authenticated routes @smoke @core
View Logs
[chromium] › test/e2e/specs/smoke/
auth.spec.ts:3:5 › redirects unauthenticated visitors to sign in @smoke
View Logs

Fix in Cursor

@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a false-positive alerting problem where AI insights jobs completing successfully were being emitted at ERROR level whenever any tool call (e.g. invalid SQL generation, wrong UUID passed to create_annotation) failed during execution. The fix demotes the request-scoped logging call in createToolLogger.error() from requestLogger.error() to requestLogger.warn(), preventing hasError from being set on the request logger for recoverable tool failures.

  • The one-line core fix correctly targets the evlog level-selection logic: tool failures now set hasWarn instead of hasError, so a succeeded job emits at WARN or INFO rather than ERROR. Actual job failures in the jobs.ts catch block continue to call logger.error() directly and are unaffected.
  • A secondary side-effect is that createToolLogger.error() and .warn() are now functionally identical within a request scope; callers outside a request scope still get log.error() vs log.warn() respectively, creating a subtle behavioral asymmetry.

Confidence Score: 5/5

Safe to merge — the change is minimal and well-scoped, touching only one code path in the tool logger's request-scoped error handler.

The change is a single-line swap that correctly addresses false-positive ERROR emissions without touching any job-failure paths. The fallback (log.error) is unchanged, and real failures continue to log at ERROR via the catch block in jobs.ts. The only trade-offs are cosmetic: error() and warn() are now silent synonyms inside a request scope, and the stack trace from new Error(message) is no longer attached to request-scoped tool-failure events.

No files require special attention — the single changed file has a well-contained, easy-to-verify modification.

Important Files Changed

Filename Overview
packages/ai/src/ai/tools/utils/logger.ts Changes requestLogger.error(new Error(message)) to requestLogger.warn(message) in the tool logger's error path, preventing tool-level failures from permanently setting hasError on the request-scoped logger and causing false-positive ERROR emissions for successful jobs.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Tool as AI Tool
    participant TL as createToolLogger
    participant RL as RequestLogger (evlog)
    participant Emit as logger.emit()

    Note over Tool,Emit: Before this PR (tool call fails mid-job)
    Tool->>TL: toolLogger.error("invalid SQL", ctx)
    TL->>RL: requestLogger.error(new Error(msg), ctx)
    RL-->>RL: "hasError = true"
    Note over RL: job succeeds, catch block not hit
    RL->>Emit: "level = hasError ? error : ..."
    Emit-->>Emit: emits at ERROR level

    Note over Tool,Emit: After this PR (tool call fails mid-job)
    Tool->>TL: toolLogger.error("invalid SQL", ctx)
    TL->>RL: requestLogger.warn(msg, ctx)
    RL-->>RL: "hasWarn = true"
    Note over RL: job succeeds, catch block not hit
    RL->>Emit: "level = hasError ? error : hasWarn ? warn : info"
    Emit-->>Emit: emits at WARN level

    Note over Tool,Emit: Actual job failure (unchanged)
    Tool->>TL: toolLogger.error(...)
    TL->>RL: requestLogger.warn(...)
    Note over RL: catch block in jobs.ts fires
    RL->>Emit: logger.error(err)
    RL-->>RL: "hasError = true"
    Emit-->>Emit: emits at ERROR level
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Tool as AI Tool
    participant TL as createToolLogger
    participant RL as RequestLogger (evlog)
    participant Emit as logger.emit()

    Note over Tool,Emit: Before this PR (tool call fails mid-job)
    Tool->>TL: toolLogger.error("invalid SQL", ctx)
    TL->>RL: requestLogger.error(new Error(msg), ctx)
    RL-->>RL: "hasError = true"
    Note over RL: job succeeds, catch block not hit
    RL->>Emit: "level = hasError ? error : ..."
    Emit-->>Emit: emits at ERROR level

    Note over Tool,Emit: After this PR (tool call fails mid-job)
    Tool->>TL: toolLogger.error("invalid SQL", ctx)
    TL->>RL: requestLogger.warn(msg, ctx)
    RL-->>RL: "hasWarn = true"
    Note over RL: job succeeds, catch block not hit
    RL->>Emit: "level = hasError ? error : hasWarn ? warn : info"
    Emit-->>Emit: emits at WARN level

    Note over Tool,Emit: Actual job failure (unchanged)
    Tool->>TL: toolLogger.error(...)
    TL->>RL: requestLogger.warn(...)
    Note over RL: catch block in jobs.ts fires
    RL->>Emit: logger.error(err)
    RL-->>RL: "hasError = true"
    Emit-->>Emit: emits at ERROR level
Loading

Comments Outside Diff (2)

  1. packages/ai/src/ai/tools/utils/logger.ts, line 21-35 (link)

    P2 error and warn methods are now identical within request scope

    After this change, calling toolLogger.error() vs toolLogger.warn() produces the same effect when a request scope is active — both route to requestLogger.warn(). The methods diverge only in the fallback path (log.error vs log.warn). Any future caller that uses .error() intending to signal a more-severe tool failure will silently get the same WARN treatment. Consider documenting this equivalence in a comment, or renaming the error method to warn and updating all call sites so the surface API matches actual behavior.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  2. packages/ai/src/ai/tools/utils/logger.ts, line 21-29 (link)

    P2 Stack trace removed from request-scoped tool error logs

    The removed new Error(message) construction meant that requestLogger.error(err, context) previously attached a stack trace to the wide event. The new requestLogger.warn(message, context) call passes a plain string, so the call stack that produced the tool failure is no longer captured. For invalid SQL or bad UUID errors this is probably fine since the SQL/context is in the payload, but for unexpected exceptions bubbled up as strings the stack trace could be useful during debugging. If context typically contains the original error object this is a non-issue; otherwise consider passing { error: message, stack: new Error().stack, ...context } to preserve it.

Reviews (1): Last reviewed commit: "[superlog] Use warn instead of error for..." | Re-trigger Greptile

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.

0 participants