From d512deac84aecb8ec2e4978599ce31a3185f5350 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 20 Jun 2026 11:37:01 -0700 Subject: [PATCH] [codex] Structure preview URL failures (#3275) Co-authored-by: codex --- apps/server/src/preview/Manager.test.ts | 26 ++++++++++++++ apps/server/src/preview/Manager.ts | 28 +++++++++------ packages/contracts/src/preview.ts | 11 +++--- packages/shared/src/preview.test.ts | 46 ++++++++++++++++++++++--- packages/shared/src/preview.ts | 46 +++++++++++++++++-------- 5 files changed, 122 insertions(+), 35 deletions(-) diff --git a/apps/server/src/preview/Manager.test.ts b/apps/server/src/preview/Manager.test.ts index a910e27470d2bab44bad93794f7110a3c8ea26aa..acdfe54301eb24e42fe81d208f1a02089677bf19 100644 --- a/apps/server/src/preview/Manager.test.ts +++ b/apps/server/src/preview/Manager.test.ts @@ -1,5 +1,6 @@ import { it } from "@effect/vitest"; import { type PreviewEvent, ThreadId } from "@t3tools/contracts"; +import { PreviewUrlNormalizationError } from "@t3tools/shared/preview"; import { Effect, PubSub } from "effect"; import { expect } from "vite-plus/test"; @@ -83,6 +84,31 @@ it.layer(PreviewManager.layer)("PreviewManager", (it) => { const manager = yield* PreviewManager.PreviewManager; const error = yield* Effect.flip(manager.open({ threadId, url: " " })); expect(error._tag).toBe("PreviewInvalidUrlError"); + expect(error).toMatchObject({ inputLength: 3, reason: "empty" }); + expect(error).not.toHaveProperty("rawUrl"); + expect(error.cause).toBeInstanceOf(PreviewUrlNormalizationError); + expect((error.cause as PreviewUrlNormalizationError).reason).toBe("empty"); + }), + ); + + it.effect("preserves URL parser failures as the invalid URL cause chain", () => + Effect.gen(function* () { + const threadId = freshThreadId(); + const manager = yield* PreviewManager.PreviewManager; + const rawUrl = "https://user:password@example.com:bad/path?access_token=secret#fragment"; + const error = yield* Effect.flip(manager.open({ threadId, url: rawUrl })); + + expect(error).toMatchObject({ + inputLength: rawUrl.length, + reason: "parse", + protocol: "https:", + }); + expect(error).not.toHaveProperty("rawUrl"); + expect(error.cause).toBeInstanceOf(PreviewUrlNormalizationError); + const normalizationError = error.cause as PreviewUrlNormalizationError; + expect(normalizationError.cause).toBeInstanceOf(Error); + expect(error.message).not.toContain((normalizationError.cause as Error).message); + expect(error.message).not.toMatch(/user|password|access_token|secret|fragment/); }), ); diff --git a/apps/server/src/preview/Manager.ts b/apps/server/src/preview/Manager.ts index 159932c4bdc8047eac091c5f63f06b7636bdb98a..fe3557c157f91ab4b757d8ad26cb0c9292219e9c 100644 --- a/apps/server/src/preview/Manager.ts +++ b/apps/server/src/preview/Manager.ts @@ -24,9 +24,9 @@ import { type PreviewSessionSnapshot, } from "@t3tools/contracts"; import { + isPreviewUrlNormalizationError, newPreviewTabId, normalizePreviewUrl, - PreviewUrlNormalizationError, } from "@t3tools/shared/preview"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; @@ -82,16 +82,22 @@ const sessionsForThread = ( const normalizeUrl = (rawUrl: string): Effect.Effect => Effect.try({ try: () => normalizePreviewUrl(rawUrl), - catch: (cause) => - new PreviewInvalidUrlError({ - rawUrl, - detail: - cause instanceof PreviewUrlNormalizationError - ? cause.detail - : cause instanceof Error - ? cause.message - : String(cause), - }), + catch: (cause) => { + if (isPreviewUrlNormalizationError(cause)) { + return new PreviewInvalidUrlError({ + inputLength: cause.inputLength, + reason: cause.reason, + protocol: cause.protocol, + cause, + }); + } + + return new PreviewInvalidUrlError({ + inputLength: rawUrl.length, + reason: "unexpected", + cause, + }); + }, }); const currentIsoTimestamp = DateTime.now.pipe(Effect.map(DateTime.formatIso)); diff --git a/packages/contracts/src/preview.ts b/packages/contracts/src/preview.ts index 044b8fbbd07d1adc3226a7f0b369a2396a9df14c..457e66ee07f889ae16ed698a4968d5d34cd6d4ac 100644 --- a/packages/contracts/src/preview.ts +++ b/packages/contracts/src/preview.ts @@ -172,14 +172,15 @@ export class PreviewSessionLookupError extends Schema.TaggedErrorClass()( "PreviewInvalidUrlError", { - rawUrl: Schema.String, - detail: Schema.optional(Schema.String), + inputLength: Schema.Number, + reason: Schema.Literals(["empty", "parse", "unsupported-protocol", "unexpected"]), + protocol: Schema.optional(Schema.String), + cause: Schema.Defect(), }, ) { override get message() { - return this.detail - ? `Invalid preview URL: ${this.rawUrl} (${this.detail})` - : `Invalid preview URL: ${this.rawUrl}`; + const protocol = this.protocol === undefined ? "" : `: ${this.protocol}`; + return `Invalid preview URL (${this.reason}${protocol}; input length ${this.inputLength}).`; } } diff --git a/packages/shared/src/preview.test.ts b/packages/shared/src/preview.test.ts index 6030686d3ed90b5f905b8985464ce17179bb6517..fec4203c5334c09c0ed8239e8b23eaaa0beedded 100644 --- a/packages/shared/src/preview.test.ts +++ b/packages/shared/src/preview.test.ts @@ -61,15 +61,51 @@ describe("normalizePreviewUrl", () => { }); it("rejects empty input", () => { - expect(() => normalizePreviewUrl(" ")).toThrow(PreviewUrlNormalizationError); + try { + normalizePreviewUrl(" "); + expect.unreachable("expected URL normalization to fail"); + } catch (error) { + expect(error).toBeInstanceOf(PreviewUrlNormalizationError); + expect(error).toMatchObject({ inputLength: 3, reason: "empty" }); + expect(error).not.toHaveProperty("rawUrl"); + expect("cause" in (error as object)).toBe(false); + } }); it("rejects unsupported protocols", () => { - expect(() => normalizePreviewUrl("ftp://example.com")).toThrow(PreviewUrlNormalizationError); - expect(() => normalizePreviewUrl("file:///etc/passwd")).toThrow(PreviewUrlNormalizationError); + try { + normalizePreviewUrl("ftp://example.com"); + expect.unreachable("expected URL normalization to fail"); + } catch (error) { + expect(error).toBeInstanceOf(PreviewUrlNormalizationError); + expect(error).toMatchObject({ + inputLength: "ftp://example.com".length, + reason: "unsupported-protocol", + protocol: "ftp:", + }); + } }); - it("rejects unparseable junk", () => { - expect(() => normalizePreviewUrl("http://")).toThrow(PreviewUrlNormalizationError); + it("rejects unparseable input without retaining credentials or tokens", () => { + const rawUrl = "https://user:password@example.com:bad/path?access_token=secret#fragment"; + try { + normalizePreviewUrl(rawUrl); + expect.unreachable("expected URL normalization to fail"); + } catch (error) { + expect(error).toBeInstanceOf(PreviewUrlNormalizationError); + expect(error).toMatchObject({ + inputLength: rawUrl.length, + reason: "parse", + protocol: "https:", + }); + expect(error).not.toHaveProperty("rawUrl"); + expect((error as PreviewUrlNormalizationError).cause).toBeInstanceOf(Error); + expect((error as PreviewUrlNormalizationError).message).not.toContain( + ((error as PreviewUrlNormalizationError).cause as Error).message, + ); + expect((error as PreviewUrlNormalizationError).message).not.toMatch( + /user|password|access_token|secret|fragment/, + ); + } }); }); diff --git a/packages/shared/src/preview.ts b/packages/shared/src/preview.ts index cc5a765ddcb7249db5ef42128d39ac0a5dfa1805..926b30966e52a20271ff76c879e300b45ec741b0 100644 --- a/packages/shared/src/preview.ts +++ b/packages/shared/src/preview.ts @@ -4,6 +4,8 @@ * on what counts as "loopback" and how to normalise a free-form URL string. */ +import * as Schema from "effect/Schema"; + const TAB_ID_PREFIX = "tab_"; let nextPreviewTabSequence = 0; @@ -45,17 +47,27 @@ export function isPreviewableUrl(rawUrl: string): boolean { } } -export class PreviewUrlNormalizationError extends Error { - readonly rawUrl: string; - readonly detail: string; - constructor(rawUrl: string, detail: string) { - super(`Invalid preview URL: ${rawUrl} (${detail})`); - this.name = "PreviewUrlNormalizationError"; - this.rawUrl = rawUrl; - this.detail = detail; +export class PreviewUrlNormalizationError extends Schema.TaggedErrorClass()( + "PreviewUrlNormalizationError", + { + inputLength: Schema.Number, + reason: Schema.Literals(["empty", "parse", "unsupported-protocol"]), + protocol: Schema.optional(Schema.String), + cause: Schema.optional(Schema.Defect()), + }, +) { + override get message(): string { + const protocol = this.protocol === undefined ? "" : `: ${this.protocol}`; + return `Invalid preview URL (${this.reason}${protocol}; input length ${this.inputLength}).`; } } +export const isPreviewUrlNormalizationError = Schema.is(PreviewUrlNormalizationError); + +function previewUrlProtocol(rawUrl: string): string | undefined { + return /^([A-Za-z][A-Za-z\d+.-]*):/.exec(rawUrl)?.[1]?.toLowerCase().concat(":"); +} + /** * Normalise a free-form URL string into a fully-qualified `http(s)://` URL. * @@ -69,7 +81,7 @@ export class PreviewUrlNormalizationError extends Error { export function normalizePreviewUrl(rawUrl: string): string { const trimmed = rawUrl.trim(); if (trimmed.length === 0) { - throw new PreviewUrlNormalizationError(rawUrl, "empty"); + throw new PreviewUrlNormalizationError({ inputLength: rawUrl.length, reason: "empty" }); } const useHttp = LOOPBACK_PREFIX_PATTERN.test(trimmed); const candidate = trimmed.includes("://") @@ -79,13 +91,19 @@ export function normalizePreviewUrl(rawUrl: string): string { try { parsed = new URL(candidate); } catch (cause) { - throw new PreviewUrlNormalizationError( - rawUrl, - cause instanceof Error ? cause.message : "unparseable", - ); + throw new PreviewUrlNormalizationError({ + inputLength: rawUrl.length, + reason: "parse", + protocol: previewUrlProtocol(candidate), + cause, + }); } if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { - throw new PreviewUrlNormalizationError(rawUrl, `unsupported protocol ${parsed.protocol}`); + throw new PreviewUrlNormalizationError({ + inputLength: rawUrl.length, + reason: "unsupported-protocol", + protocol: parsed.protocol, + }); } return parsed.href; } -- 2.54.0