[codex] Structure browser target resolution errors - #3327
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved This PR refactors error handling to use structured Effect TaggedErrorClass types instead of plain Error objects. The error conditions and messages remain identical; only the structure is enhanced for better typing and diagnostics. Low-risk mechanical change with comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
a18ef81 to
c7db911
Compare
Dismissing prior approval to re-evaluate c7db911
c5bc774
into
codex/redact-dpop-request-target
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Summary
Verification
vp test apps/web/src/browser/browserTargetResolver.test.tsvp check(passes with 20 pre-existing warnings)vp run typecheckNote
Low Risk
Localized resolver and shared URL-redaction helpers; behavior is unchanged aside from error shape and safer diagnostics, with no auth or data-path changes in this diff.
Overview
resolveBrowserNavigationTargetnow throws three EffectSchema.TaggedErrorClasstypes instead of genericErrors: disconnected environment, invalid environment HTTP base URL (with native parsecause), and non–private-network host (withenvironmentIdandhostname).Invalid-base-URL failures use new
getUrlDiagnostics(@t3tools/shared/urlDiagnostics) so errors expose only input length and optional protocol/hostname—tests assert messages and shapes never leak credentials or full URLs.Shared
redactDpopRequestTargetstrips userinfo, query, and fragment from URLs for safe logging (invalid input →"<invalid-url>"), with tests only in this PR.Breaking for callers: anything that matched generic error strings must handle tagged errors by
_tag/ type instead.Reviewed by Cursor Bugbot for commit c7db911. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace generic errors with structured tagged error classes in
resolveBrowserNavigationTargetbrowserTargetResolver.ts:BrowserTargetEnvironmentDisconnectedError,BrowserTargetEnvironmentUrlInvalidError, andBrowserTargetPrivateNetworkRequiredError, each with structured properties and computed messages.BrowserTargetEnvironmentUrlInvalidErroruses a newgetUrlDiagnosticshelper (inpackages/shared/src/urlDiagnostics.ts) to expose only non-sensitive diagnostics (input length, protocol, hostname) when a URL cannot be parsed.redactDpopRequestTargettopackages/shared/src/dpop.tsto produce redacted DPoP request targets without leaking user info, query, or fragment.resolveBrowserNavigationTargetwill now receive typed tagged errors instead of genericErrorinstances.Macroscope summarized c7db911.