OUT-4013: stop raw ZodError on GET / from unauthenticated renders - #232
Merged
Conversation
…f raw ZodError When the proxy does not inject auth headers (invalid/expired token), the home route render reached authenticateHeaders, where z.string().parse(undefined) threw a raw ZodError. Replace the bare parses with explicit presence checks that throw the existing AssemblyMissingHeadersError / AssemblyInvalidTokenError, and log which header is missing server-side (message kept generic to avoid leaking detail to API clients). Fixes CLIENT-HOME-V3-1K Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wrap onRequestError so 401-class auth errors (missing headers, invalid token, token parse) are skipped instead of reported as unhandled errors. Revert the global-error name-based filter to a plain capture: server errors are redacted in prod, so type-based filtering there is a no-op. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unit tests for authenticateHeaders (all header combinations, and the missing- header diagnostic naming the right header) and for onRequestError (skips expected auth errors, forwards generic and non-Error values to Sentry). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Conventions for const-over-let, short comments, and DRY/KISS/SOLID/YAGNI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR replaces raw missing-header validation failures with domain-specific authentication errors and narrows Sentry suppression to the expected missing-header case.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming request] --> B[Proxy authentication]
B --> C[Root or API authentication]
C --> D{Required headers present?}
D -- No --> E[AssemblyMissingHeadersError]
E --> F[Server warning]
E --> G[onRequestError suppresses expected noise]
D -- Yes --> H{User identity present?}
H -- No --> I[AssemblyInvalidTokenError]
I --> J[onRequestError forwards to Sentry]
H -- Yes --> K[Authenticated User]
Reviews (2): Last reviewed commit: "fix(OUT-4013): only suppress AssemblyMis..." | Re-trigger Greptile |
AssemblyClient's constructor converts any SDK init rejection (config, service, network) into AssemblyInvalidTokenError, which reaches onRequestError via the unwrapped root render path. The previous class-wide filter dropped those real failures from Sentry. Narrow the filter to AssemblyMissingHeadersError only — the genuine OUT-4013 unauthenticated-request noise — so SDK/config failures and token-parse errors keep their error-level report. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
|
@greptileai review PR again |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
authenticateHeadersno longer callsz.string().parse(undefined). When the proxy did not inject auth headers (invalid/expired token →withErrorHandlerreturnsNextResponse.next()for non-API routes, so render proceeds unauthenticated), it now throws the existing domain errorsAssemblyMissingHeadersError/AssemblyInvalidTokenErrorinstead of a rawZodError.console.warn; the thrown error message stays generic sowithErrorHandlercan't leak the detail to API clients.onRequestError(instrumentation) now filters expected 401-class auth failures (AssemblyMissingHeadersError,AssemblyInvalidTokenError,AssemblyTokenParseError) instead of reporting them to Sentry as unhandled errors.global-error.tsxreverted to a plainSentry.captureException+ comment — server errors are redacted in prod, so type-based filtering there is a no-op.authenticateHeadersandonRequestError.Fixes CLIENT-HOME-V3-1K (Sentry).
Testing Criteria
pnpm typecheck,pnpm lint,pnpm test(48 passing), andpnpm build(no Edge Runtime warnings) all pass.authenticateHeadersunit tests: valid internal-user and client headers return the correctUser; missingtoken/workspaceIdthrowsAssemblyMissingHeadersErrorand the diagnostic names the missing header; missing bothinternalUserIdandclientIdthrowsAssemblyInvalidTokenError.onRequestErrorunit tests: expected auth errors are skipped (not sent to Sentry); a genericErrorand a non-Errorthrown value are still forwarded to Sentry.Notes
docs:commit (engineering notes inAGENTS.md) is unrelated to the fix and included only for convenience; can be dropped if preferred.Impact & Surface Area of Change
authenticateHeaders) called by the root layout and ~10 API route controllers, plus the global instrumentation error hook. Happy-path behavior is unchanged (sameUsershape returned).ZodError, and these expected 401-class failures no longer create Sentry noise. Worth a regression glance at authenticated page loads and API routes to confirm normal auth still works.