Skip to content

fix(api): collapse the internal error envelope and restore requestId - #6584

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/internal-error-envelope
Aug 12, 2026
Merged

fix(api): collapse the internal error envelope and restore requestId#6584
waleedlatif1 merged 2 commits into
stagingfrom
fix/internal-error-envelope

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Collapse the two internal error envelopes into one. internalOrchestrationErrorPolicy emitted { success: false, error } and internalPlainOrchestrationErrorPolicy emitted { error }; both now emit { error } under the single internalOrchestrationErrorPolicy name.
  • The split approximated pre-builder behavior, where the shape depended on which branch failed — guard clauses returned { error } and a route's terminal try/catch returned { success: false, error }. A per-route policy cannot express a per-branch rule, so the two variants disagreed on the same status across families.
  • Kept the bare shape because it is what messageFromErrorBody reads on the client and what most migrated routes already emitted. requestJson throws ApiClientError for any non-2xx, so no typed client ever observes the discriminator.
  • Restore requestId to internal error bodies. withRouteHandler stamps it on the bodies it generates, but the builder overrides dropped it, leaving it only on the x-request-id header — invisible when a user pastes an error out of the UI.
  • Applied at the createJsonErrorResponse chokepoint plus both wrapper overrides, in the JSON and binary builders. Omitted when there is no active request scope, so unit tests are unaffected unless they opt in.
  • Dropped the now-redundant internalFileErrorPolicies.plain alias.

success: true on success bodies is a separate contract and is untouched — /files/[fileId]/dimensions still returns a { success: false } domain result at 200.

Internal routes only. No /api/v1/** route changes shape.

Type of Change

  • Bug fix

Testing

bun run type-check clean. 3118 API tests pass (app/api + lib/api). bun run check:api-validation passes.

Added lib/api/server/routes/request-id.test.ts and a builder test pinning the bare envelope. Verified both go red when the injection is reverted.

Checklist

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

@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedAug 12, 2026 2:22am

Request Review

@cursor

cursorBot commented Aug 12, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Cross-cutting change to the internal error response contract across many authenticated routes. Typed clients ignore success: false, but any ad-hoc consumers of that field could break; requestId injection is additive and scoped to internal builders.

Overview
Unifies internal API error responses onto a single bare envelope: { error, requestId? }.

Removes internalPlainOrchestrationErrorPolicy and makes internalOrchestrationErrorPolicy the only policy, dropping success: false from error bodies. Call sites across tables, workflows, knowledge, and workspace files are updated to the shared policy.

Restores requestId on internal error bodies via new withRequestId / responseWithRequestId helpers at the JSON and binary builder chokepoints (auth, parse, and projected errors). The field is omitted when there is no active request scope, so unit tests stay unchanged unless they opt in. v1/v2 public envelopes are untouched.

Reviewed by Cursor Bugbot for commit 8423bdc. Configure here.

@greptile-apps

greptile-appsBot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR unifies internal orchestration failures under the bare error envelope and restores request IDs across generated internal errors.

  • Replaces the split internal orchestration policies with one shared policy.
  • Adds request-ID stamping for authentication, parsing, projected, and wrapper-generated errors in JSON and binary builders.
  • Updates internal route policies and tests to reflect the unified envelope.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported authentication and parsing paths now add the active request ID to internal JSON error bodies in both builders.

Important Files Changed

FilenameOverview
apps/sim/lib/api/server/routes/internal-json-route.tsUnifies the internal JSON error envelope and routes authentication, parsing, projected, and wrapper failures through request-ID-aware responses.
apps/sim/lib/api/server/routes/internal-binary-route.tsApplies the same request-ID restoration to JSON error responses emitted by internal binary routes.
apps/sim/lib/api/server/routes/request-id.tsAdds helpers to stamp object error bodies and safely rebuild existing JSON validation responses while preserving status and headers.
apps/sim/lib/api/server/routes/internal-json-route.test.tsCovers the unified bare envelope and request IDs on authentication, parsing, and projected errors.
apps/sim/lib/api/server/routes/request-id.test.tsVerifies request-ID injection, pass-through behavior, and preservation of response metadata.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
Request[Internal API request] --> Wrapper[withRouteHandler request scope]
Wrapper --> Auth[Authentication]
Auth -->|401| Stamp[Add requestId to JSON error]
Auth --> Parse[Contract parsing]
Parse -->|400| Rebuild[Rebuild JSON response with requestId]
Parse --> UseCase[Application use case]
UseCase -->|Projected error| Stamp
UseCase -->|Unhandled error| Callback[Wrapper error callback]
Callback --> Stamp
Stamp --> Response[Error body plus x-request-id header]
Rebuild --> Response
Loading

Reviews (2): Last reviewed commit: "fix(api): stamp requestId on internal au..." | Re-trigger Greptile

Comment threadapps/sim/lib/api/server/routes/internal-json-route.ts
The builders shipped two internal error envelopes: internalOrchestrationErrorPolicy
emitted { success: false, error } while internalPlainOrchestrationErrorPolicy
emitted { error }. That split approximated pre-builder behavior, where the shape
depended on which branch failed - guard clauses returned { error } and a route's
terminal try/catch returned { success: false, error }. A per-route policy cannot
express a per-branch rule, so the two disagreed on the same status across families.
Collapse to the bare { error } shape. It is what messageFromErrorBody reads on the
client and what most migrated routes already emitted. requestJson throws
ApiClientError for any non-2xx, so no typed client ever observes the discriminator.
success: true on success bodies is a separate contract and is untouched.
Also restore requestId to internal error bodies. withRouteHandler stamps it on the
bodies it generates, but the builder overrides dropped it, leaving it only on the
x-request-id header - invisible when a user pastes an error out of the UI. It is now
applied at the createJsonErrorResponse chokepoint and in both wrapper overrides, and
is omitted when there is no active request scope.
@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

@waleedlatif1

Copy link
Copy Markdown
CollaboratorAuthor

@cursor review

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8423bdc. Configure here.

@waleedlatif1
waleedlatif1 merged commit 1874cec into stagingAug 12, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/internal-error-envelope branch August 12, 2026 02:33
Sign up for freeto 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

@waleedlatif1