From f7aa408f3e83bcba832319219460bf70ae685c5b Mon Sep 17 00:00:00 2001 From: bcoll Date: Wed, 7 Feb 2024 10:46:19 +0000 Subject: [PATCH 1/5] fix: don't report invalid `playground-preview-worker` upload input --- .../playground-preview-worker/src/index.ts | 7 ++ .../playground-preview-worker/src/sentry.ts | 2 +- .../playground-preview-worker/src/user.do.ts | 88 +++++++++----- .../tests/index.test.ts | 108 +++++++++++++++++- 4 files changed, 171 insertions(+), 34 deletions(-) diff --git a/packages/playground-preview-worker/src/index.ts b/packages/playground-preview-worker/src/index.ts index 454bcffa3cca..d099841befef 100644 --- a/packages/playground-preview-worker/src/index.ts +++ b/packages/playground-preview-worker/src/index.ts @@ -133,6 +133,13 @@ class PreviewRequestForbidden extends HttpError { } } +export class BadUpload extends HttpError { + name = "BadUpload"; + constructor(message = "Invalid upload") { + super(message, 400, false); + } +} + /** * Given a preview token, this endpoint allows for raw http calls to be inspected * diff --git a/packages/playground-preview-worker/src/sentry.ts b/packages/playground-preview-worker/src/sentry.ts index 2c792048187c..f7cc37604fb9 100644 --- a/packages/playground-preview-worker/src/sentry.ts +++ b/packages/playground-preview-worker/src/sentry.ts @@ -1,5 +1,5 @@ import { Toucan } from "toucan-js"; -import { z, ZodError } from "zod"; +import { ZodError } from "zod"; import { HttpError, ZodSchemaError } from "."; export function handleException(e: unknown, sentry: Toucan): Response { diff --git a/packages/playground-preview-worker/src/user.do.ts b/packages/playground-preview-worker/src/user.do.ts index 4c220455b20b..9ab9cbaae83e 100644 --- a/packages/playground-preview-worker/src/user.do.ts +++ b/packages/playground-preview-worker/src/user.do.ts @@ -1,5 +1,6 @@ import assert from "node:assert"; import { Buffer } from "node:buffer"; +import z from "zod"; import { constructMiddleware } from "./inject-middleware"; import { doUpload, @@ -8,7 +9,7 @@ import { UploadResult, } from "./realish"; import { handleException, setupSentry } from "./sentry"; -import { ServiceWorkerNotSupported, WorkerTimeout } from "."; +import { BadUpload, ServiceWorkerNotSupported, WorkerTimeout } from "."; const encoder = new TextEncoder(); @@ -26,6 +27,15 @@ function switchRemote(url: URL, remote: string) { return workerUrl; } +const UploadedMetadata = z.object({ + body_part: z.ostring(), + main_module: z.ostring(), + compatibility_date: z.ostring(), + compatibility_flags: z.array(z.string()).optional(), +}); + +type UploadedMetadata = z.infer; + /** * This Durable object coordinates operations for a specific user session. It's purpose is to * communicate with the Realish preview service on behalf of a user, without leaking more info @@ -114,17 +124,9 @@ export class UserSession { await this.state.storage.put("inspectorUrl", this.inspectorUrl); } - async fetch(request: Request) { + async handleRequest(request: Request) { const url = new URL(request.url); - // We need to construct a new Sentry instance here because throwing - // errors across a DO boundary will wipe stack information etc... - const sentry = setupSentry( - request, - undefined, - this.env.SENTRY_DSN, - this.env.SENTRY_ACCESS_CLIENT_ID, - this.env.SENTRY_ACCESS_CLIENT_SECRET - ); + // This is an inspector request. Forward to the correct inspector URL if (request.headers.get("Upgrade") && url.pathname === "/api/inspector") { assert(this.inspectorUrl !== undefined); @@ -158,17 +160,33 @@ export class UserSession { } return workerResponse; } + const userSession = this.state.id.toString(); - const worker = await request.formData(); - const m = worker.get("metadata"); + let worker: FormData; + try { + worker = await request.formData(); + } catch (e) { + throw new BadUpload(`Expected valid form data`); + } - assert(m instanceof File); + const m = worker.get("metadata"); + if (!(m instanceof File)) { + throw new BadUpload("Expected metadata file to be defined"); + } - const uploadedMetadata = JSON.parse(await m.text()); + let uploadedMetadata: UploadedMetadata; + try { + uploadedMetadata = UploadedMetadata.parse(JSON.parse(await m.text())); + } catch { + throw new BadUpload("Expected metadata file to be valid"); + } - if ("body_part" in uploadedMetadata) { - return new ServiceWorkerNotSupported().toResponse(); + if ( + uploadedMetadata.body_part !== undefined || + uploadedMetadata.main_module === undefined + ) { + throw new ServiceWorkerNotSupported(); } const today = new Date(); @@ -206,19 +224,33 @@ export class UserSession { }) ); - try { - await this.uploadWorker(this.workerName, worker); + await this.uploadWorker(this.workerName, worker); - assert(this.inspectorUrl !== undefined); + assert(this.inspectorUrl !== undefined); + + return Response.json({ + // Include a hash of the inspector URL so as to ensure the client will reconnect + // when the inspector URL has changed (because of an updated preview session) + inspector: `/api/inspector?user=${userSession}&h=${await hash( + this.inspectorUrl + )}`, + preview: userSession, + }); + } - return Response.json({ - // Include a hash of the inspector URL so as to ensure the client will reconnect - // when the inspector URL has changed (because of an updated preview session) - inspector: `/api/inspector?user=${userSession}&h=${await hash( - this.inspectorUrl - )}`, - preview: userSession, - }); + async fetch(request: Request) { + // We need to construct a new Sentry instance here because throwing + // errors across a DO boundary will wipe stack information etc... + const sentry = setupSentry( + request, + undefined, + this.env.SENTRY_DSN, + this.env.SENTRY_ACCESS_CLIENT_ID, + this.env.SENTRY_ACCESS_CLIENT_SECRET + ); + + try { + return await this.handleRequest(request); } catch (e) { return handleException(e, sentry); } diff --git a/packages/playground-preview-worker/tests/index.test.ts b/packages/playground-preview-worker/tests/index.test.ts index f027e6fb4f00..91e6619c874c 100644 --- a/packages/playground-preview-worker/tests/index.test.ts +++ b/packages/playground-preview-worker/tests/index.test.ts @@ -5,9 +5,9 @@ const REMOTE = "https://playground-testing.devprod.cloudflare.dev"; const PREVIEW_REMOTE = "https://random-data.playground-testing.devprod.cloudflare.dev"; -const TEST_WORKER_CONTENT_TYPE = - "multipart/form-data; boundary=----WebKitFormBoundaryqJEYLXuUiiZQHgvf"; -const TEST_WORKER = `------WebKitFormBoundaryqJEYLXuUiiZQHgvf +const TEST_WORKER_BOUNDARY = "----WebKitFormBoundaryqJEYLXuUiiZQHgvf"; +const TEST_WORKER_CONTENT_TYPE = `multipart/form-data; boundary=${TEST_WORKER_BOUNDARY}`; +const TEST_WORKER = `--${TEST_WORKER_BOUNDARY} Content-Disposition: form-data; name="index.js"; filename="index.js" Content-Type: application/javascript+module @@ -39,12 +39,12 @@ export default { } } -------WebKitFormBoundaryqJEYLXuUiiZQHgvf +--${TEST_WORKER_BOUNDARY} Content-Disposition: form-data; name="metadata"; filename="blob" Content-Type: application/json {"compatibility_date":"2023-05-04","main_module":"index.js"} -------WebKitFormBoundaryqJEYLXuUiiZQHgvf--`; +--${TEST_WORKER_BOUNDARY}--`; async function fetchUserToken() { return fetch(REMOTE).then( @@ -318,6 +318,104 @@ describe("Upload Worker", () => { '"{\\"error\\":\\"UploadFailed\\",\\"message\\":\\"Valid token not provided\\",\\"data\\":{}}"' ); }); + it("should reject invalid form data", async () => { + const w = await fetch(`${REMOTE}/api/worker`, { + method: "POST", + headers: { + cookie: `user=${defaultUserToken}`, + "Content-Type": "text/plain", + }, + body: "not a form", + }); + expect(w.status).toBe(400); + expect(await w.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"' + ); + }); + it("should reject missing metadata", async () => { + const w = await fetch(`${REMOTE}/api/worker`, { + method: "POST", + headers: { + cookie: `user=${defaultUserToken}`, + "Content-Type": "text/plain", + }, + body: `--${TEST_WORKER_BOUNDARY} +Content-Disposition: form-data; name="index.js"; filename="index.js" +Content-Type: application/javascript+module + +export default { + fetch(request) { return new Response("body"); } +} + +--${TEST_WORKER_BOUNDARY}--`, + }); + expect(w.status).toBe(400); + expect(await w.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"' + ); + }); + it("should reject invalid metadata json", async () => { + const w = await fetch(`${REMOTE}/api/worker`, { + method: "POST", + headers: { + cookie: `user=${defaultUserToken}`, + "Content-Type": TEST_WORKER_CONTENT_TYPE, + }, + body: `--${TEST_WORKER_BOUNDARY} +Content-Disposition: form-data; name="metadata"; filename="blob" +Content-Type: application/json + +{"compatibility_date":"2023-05-04", +--${TEST_WORKER_BOUNDARY}--`, + }); + expect(w.status).toBe(400); + expect(await w.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected metadata file to be valid\\",\\"data\\":{}}"' + ); + }); + it("should reject invalid metadata", async () => { + const w = await fetch(`${REMOTE}/api/worker`, { + method: "POST", + headers: { + cookie: `user=${defaultUserToken}`, + "Content-Type": TEST_WORKER_CONTENT_TYPE, + }, + body: `--${TEST_WORKER_BOUNDARY} +Content-Disposition: form-data; name="metadata"; filename="blob" +Content-Type: application/json + +{"compatibility_date":42,"main_module":"index.js"} +--${TEST_WORKER_BOUNDARY}--`, + }); + expect(w.status).toBe(400); + expect(await w.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected metadata file to be valid\\",\\"data\\":{}}"' + ); + }); + it("should reject service worker", async () => { + const w = await fetch(`${REMOTE}/api/worker`, { + method: "POST", + headers: { + cookie: `user=${defaultUserToken}`, + "Content-Type": TEST_WORKER_CONTENT_TYPE, + }, + body: `--${TEST_WORKER_BOUNDARY} +Content-Disposition: form-data; name="index.js"; filename="index.js" +Content-Type: application/javascript + +addEventListener("fetch", (event) => event.respondWith(new Response("body"))); +--${TEST_WORKER_BOUNDARY} +Content-Disposition: form-data; name="metadata"; filename="blob" +Content-Type: application/json + +{"compatibility_date":"2023-05-04","body_part":"index.js"} +--${TEST_WORKER_BOUNDARY}--`, + }); + expect(w.status).toBe(400); + expect(await w.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"ServiceWorkerNotSupported\\",\\"message\\":\\"Service Workers are not supported in the Workers Playground\\",\\"data\\":{}}"' + ); + }); }); describe("Raw HTTP preview", () => { From 1e29ffa7e2714b2dd94dc00069c511b86f44ee1a Mon Sep 17 00:00:00 2001 From: bcoll Date: Wed, 7 Feb 2024 11:37:36 +0000 Subject: [PATCH 2/5] fix: bump `hono` to address cooking parsing issues honojs/hono#1428 improved cookie parsing and addresses the `Cannot read properties of undefined (reading 'split')` issues we're seeing. --- packages/playground-preview-worker/package.json | 2 +- .../playground-preview-worker/tests/index.test.ts | 11 ++++++++++- pnpm-lock.yaml | 9 +++++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/playground-preview-worker/package.json b/packages/playground-preview-worker/package.json index 34ab5a459976..7abe1f3c265c 100644 --- a/packages/playground-preview-worker/package.json +++ b/packages/playground-preview-worker/package.json @@ -12,7 +12,7 @@ "test:e2e": "vitest run" }, "dependencies": { - "hono": "^3.3.2", + "hono": "^3.12.11", "zod": "^3.22.3" }, "devDependencies": { diff --git a/packages/playground-preview-worker/tests/index.test.ts b/packages/playground-preview-worker/tests/index.test.ts index 91e6619c874c..684503425d61 100644 --- a/packages/playground-preview-worker/tests/index.test.ts +++ b/packages/playground-preview-worker/tests/index.test.ts @@ -85,7 +85,7 @@ describe("Preview Worker", () => { '"/hello?world"' ); expect(resp.headers.get("set-cookie") ?? "").toMatchInlineSnapshot( - `"token=${defaultUserToken}; Domain=random-data.playground-testing.devprod.cloudflare.dev; HttpOnly; Secure; SameSite=None"` + `"token=${defaultUserToken}; Domain=random-data.playground-testing.devprod.cloudflare.dev; Path=/; HttpOnly; Secure; SameSite=None"` ); }); it("shouldn't be redirected with no token", async () => { @@ -197,6 +197,15 @@ describe("Preview Worker", () => { '"{\\"error\\":\\"PreviewRequestFailed\\",\\"message\\":\\"Valid token not found\\",\\"data\\":{}}"' ); }); + it("should reject invalid cookie header", async () => { + const resp = await fetch(PREVIEW_REMOTE, { + headers: { + cookie: "token", + }, + }); + expect(resp.status).toBe(400); + expect(await resp.text()).toMatchInlineSnapshot('"{\\"error\\":\\"PreviewRequestFailed\\",\\"message\\":\\"Valid token not found\\",\\"data\\":{}}"'); + }); it("should reject invalid token", async () => { const resp = await fetch(PREVIEW_REMOTE, { headers: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 07c25f28787b..b5211f7dfb21 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1034,8 +1034,8 @@ importers: packages/playground-preview-worker: dependencies: hono: - specifier: ^3.3.2 - version: 3.5.6 + specifier: ^3.12.11 + version: 3.12.11 zod: specifier: ^3.22.3 version: 3.22.3 @@ -12491,6 +12491,11 @@ packages: resolution: {integrity: sha512-Yu+q/XWr2fFQ11tHxPq4p4EiNkb2y+lAacJNhAdRXVfRIcDH6gi7htWFnnlIzvqHMHoWeIsfXlNAjZInpAOJDA==} dev: true + /hono@3.12.11: + resolution: {integrity: sha512-LSpxVgIMR3UzyFiXZaPvqBUGqyOKG0LMZqgMn2RXz9f+YAdkHSfFQQX0dtU72fPm5GnEMh5AYXs0ek5NYgMOmA==} + engines: {node: '>=16.0.0'} + dev: false + /hono@3.5.6: resolution: {integrity: sha512-ycTOpIZJ6yLbjzoE+ojsesC7G7ZXfGSoCIDyvqmzlHc5Mk4Aj48Ed9R5g7gw3v7rOkS81pjcYIvWef/karq1iA==} engines: {node: '>=16.0.0'} From bcfb5ff2684d28394310d30c7495d5a7b18af607 Mon Sep 17 00:00:00 2001 From: bcoll Date: Wed, 7 Feb 2024 11:48:57 +0000 Subject: [PATCH 3/5] chore: add standard linting rules to `playground-preview-worker` --- .../playground-preview-worker/.eslintrc.js | 4 ++++ .../playground-preview-worker/package.json | 2 ++ .../playground-preview-worker/src/index.ts | 8 ++++---- .../playground-preview-worker/src/user.do.ts | 20 ++++++++----------- .../tests/index.test.ts | 4 +++- pnpm-lock.yaml | 3 +++ 6 files changed, 24 insertions(+), 17 deletions(-) create mode 100644 packages/playground-preview-worker/.eslintrc.js diff --git a/packages/playground-preview-worker/.eslintrc.js b/packages/playground-preview-worker/.eslintrc.js new file mode 100644 index 000000000000..a478f67f57c2 --- /dev/null +++ b/packages/playground-preview-worker/.eslintrc.js @@ -0,0 +1,4 @@ +module.exports = { + root: true, + extends: ["@cloudflare/eslint-config-worker"], +}; diff --git a/packages/playground-preview-worker/package.json b/packages/playground-preview-worker/package.json index 7abe1f3c265c..6ff5ba3d21ae 100644 --- a/packages/playground-preview-worker/package.json +++ b/packages/playground-preview-worker/package.json @@ -6,6 +6,7 @@ "build-middleware": "pnpm run build-middleware:common && pnpm run build-middleware:loader", "build-middleware:common": "pnpm dlx esbuild ../wrangler/templates/middleware/common.ts --outfile=src/middleware/common.module.template", "build-middleware:loader": "pnpm dlx esbuild ../wrangler/templates/middleware/loader-modules.ts --outfile=src/middleware/loader.module.template", + "check:lint": "eslint .", "deploy": "wrangler -j deploy", "deploy:testing": "wrangler -j deploy -e testing", "start": "wrangler -j dev", @@ -16,6 +17,7 @@ "zod": "^3.22.3" }, "devDependencies": { + "@cloudflare/eslint-config-worker": "workspace:*", "@cloudflare/workers-types": "^4.20230321.0", "@types/cookie": "^0.5.1", "cookie": "^0.5.0", diff --git a/packages/playground-preview-worker/src/index.ts b/packages/playground-preview-worker/src/index.ts index d099841befef..07b897469980 100644 --- a/packages/playground-preview-worker/src/index.ts +++ b/packages/playground-preview-worker/src/index.ts @@ -1,10 +1,10 @@ import { Hono } from "hono"; import { getCookie, setCookie } from "hono/cookie"; import prom from "promjs"; -import { Toucan } from "toucan-js"; -import { ZodIssue } from "zod"; import { handleException, setupSentry } from "./sentry"; import type { RegistryType } from "promjs"; +import type { Toucan } from "toucan-js"; +import type { ZodIssue } from "zod"; const app = new Hono<{ Bindings: Env; @@ -261,7 +261,7 @@ app.get(`${rootDomain}/`, async (c) => { }); app.post(`${rootDomain}/api/worker`, async (c) => { - let userId = getCookie(c, "user"); + const userId = getCookie(c, "user"); if (!userId) { throw new UploadFailed(); } @@ -282,7 +282,7 @@ app.post(`${rootDomain}/api/worker`, async (c) => { app.get(`${rootDomain}/api/inspector`, async (c) => { const url = new URL(c.req.url); - let userId = url.searchParams.get("user"); + const userId = url.searchParams.get("user"); if (!userId) { throw new PreviewRequestFailed("", false); } diff --git a/packages/playground-preview-worker/src/user.do.ts b/packages/playground-preview-worker/src/user.do.ts index 9ab9cbaae83e..79ad42dee243 100644 --- a/packages/playground-preview-worker/src/user.do.ts +++ b/packages/playground-preview-worker/src/user.do.ts @@ -2,20 +2,16 @@ import assert from "node:assert"; import { Buffer } from "node:buffer"; import z from "zod"; import { constructMiddleware } from "./inject-middleware"; -import { - doUpload, - RealishPreviewConfig, - setupTokens, - UploadResult, -} from "./realish"; +import { doUpload, setupTokens } from "./realish"; import { handleException, setupSentry } from "./sentry"; import { BadUpload, ServiceWorkerNotSupported, WorkerTimeout } from "."; +import type { RealishPreviewConfig, UploadResult } from "./realish"; const encoder = new TextEncoder(); async function hash(text: string) { - const hash = await crypto.subtle.digest("SHA-256", encoder.encode(text)); - return Buffer.from(hash).toString("hex"); + const digest = await crypto.subtle.digest("SHA-256", encoder.encode(text)); + return Buffer.from(digest).toString("hex"); } function switchRemote(url: URL, remote: string) { @@ -51,7 +47,7 @@ export class UserSession { inspectorUrl: string | undefined; workerName!: string; constructor(private state: DurableObjectState, private env: Env) { - this.state.blockConcurrencyWhile(async () => { + void this.state.blockConcurrencyWhile(async () => { this.config = await this.state.storage.get( "config" ); @@ -212,9 +208,9 @@ export class UserSession { metadata.main_module = entrypoint; - for (const [path, m] of additionalModules.entries()) { - assert(m instanceof File); - worker.set(path, m); + for (const [path, additionalModule] of additionalModules.entries()) { + assert(additionalModule instanceof File); + worker.set(path, additionalModule); } worker.set( diff --git a/packages/playground-preview-worker/tests/index.test.ts b/packages/playground-preview-worker/tests/index.test.ts index 684503425d61..34e4f0d4d939 100644 --- a/packages/playground-preview-worker/tests/index.test.ts +++ b/packages/playground-preview-worker/tests/index.test.ts @@ -204,7 +204,9 @@ describe("Preview Worker", () => { }, }); expect(resp.status).toBe(400); - expect(await resp.text()).toMatchInlineSnapshot('"{\\"error\\":\\"PreviewRequestFailed\\",\\"message\\":\\"Valid token not found\\",\\"data\\":{}}"'); + expect(await resp.text()).toMatchInlineSnapshot( + '"{\\"error\\":\\"PreviewRequestFailed\\",\\"message\\":\\"Valid token not found\\",\\"data\\":{}}"' + ); }); it("should reject invalid token", async () => { const resp = await fetch(PREVIEW_REMOTE, { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b5211f7dfb21..9c2af37c3c2e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1040,6 +1040,9 @@ importers: specifier: ^3.22.3 version: 3.22.3 devDependencies: + '@cloudflare/eslint-config-worker': + specifier: workspace:* + version: link:../eslint-config-worker '@cloudflare/workers-types': specifier: ^4.20230321.0 version: 4.20230821.0 From f169ac7db4cb12962e317e14e7b6b306bf7e5e99 Mon Sep 17 00:00:00 2001 From: bcoll Date: Wed, 7 Feb 2024 11:55:20 +0000 Subject: [PATCH 4/5] fix: remove playground worker circular dependency on `index.ts` --- .../playground-preview-worker/src/errors.ts | 121 +++++++++++++++++ .../playground-preview-worker/src/index.ts | 128 ++---------------- .../playground-preview-worker/src/realish.ts | 2 +- .../playground-preview-worker/src/sentry.ts | 2 +- .../playground-preview-worker/src/user.do.ts | 2 +- 5 files changed, 132 insertions(+), 123 deletions(-) create mode 100644 packages/playground-preview-worker/src/errors.ts diff --git a/packages/playground-preview-worker/src/errors.ts b/packages/playground-preview-worker/src/errors.ts new file mode 100644 index 000000000000..13549c6563c7 --- /dev/null +++ b/packages/playground-preview-worker/src/errors.ts @@ -0,0 +1,121 @@ +import type { ZodIssue } from "zod"; + +export class HttpError extends Error { + constructor( + message: string, + readonly status: number, + // Only report errors to sentry when they represent actionable errors + readonly reportable: boolean + ) { + super(message); + Object.setPrototypeOf(this, new.target.prototype); + } + toResponse() { + return Response.json( + { + error: this.name, + message: this.message, + data: this.data, + }, + { + status: this.status, + headers: { + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Methods": "GET,PUT,POST", + }, + } + ); + } + + get data(): Record { + return {}; + } +} + +export class WorkerTimeout extends HttpError { + name = "WorkerTimeout"; + constructor() { + super("Worker timed out", 400, false); + } + + toResponse(): Response { + return new Response("Worker timed out"); + } +} + +export class ServiceWorkerNotSupported extends HttpError { + name = "ServiceWorkerNotSupported"; + constructor() { + super( + "Service Workers are not supported in the Workers Playground", + 400, + false + ); + } +} +export class ZodSchemaError extends HttpError { + name = "ZodSchemaError"; + constructor(private issues: ZodIssue[]) { + super("Something went wrong", 500, true); + } + + get data(): { issues: string } { + return { issues: JSON.stringify(this.issues) }; + } +} + +export class PreviewError extends HttpError { + name = "PreviewError"; + constructor(private error: string) { + super(error, 400, false); + } + + get data(): { error: string } { + return { error: this.error }; + } +} + +export class TokenUpdateFailed extends HttpError { + name = "TokenUpdateFailed"; + constructor() { + super("Provide valid token", 400, false); + } +} + +export class RawHttpFailed extends HttpError { + name = "RawHttpFailed"; + constructor() { + super("Provide valid token", 400, false); + } +} + +export class PreviewRequestFailed extends HttpError { + name = "PreviewRequestFailed"; + constructor(private tokenId: string | undefined, reportable: boolean) { + super("Valid token not found", 400, reportable); + } + get data(): { tokenId: string | undefined } { + return { tokenId: this.tokenId }; + } +} + +export class UploadFailed extends HttpError { + name = "UploadFailed"; + constructor() { + super("Valid token not provided", 401, false); + } +} + +export class PreviewRequestForbidden extends HttpError { + name = "PreviewRequestForbidden"; + constructor() { + super("Preview request forbidden", 403, false); + } +} + +export class BadUpload extends HttpError { + name = "BadUpload"; + constructor(message = "Invalid upload") { + super(message, 400, false); + } +} diff --git a/packages/playground-preview-worker/src/index.ts b/packages/playground-preview-worker/src/index.ts index 07b897469980..fcc435a71fb7 100644 --- a/packages/playground-preview-worker/src/index.ts +++ b/packages/playground-preview-worker/src/index.ts @@ -1,10 +1,17 @@ import { Hono } from "hono"; import { getCookie, setCookie } from "hono/cookie"; import prom from "promjs"; +import { + HttpError, + PreviewRequestFailed, + PreviewRequestForbidden, + RawHttpFailed, + TokenUpdateFailed, + UploadFailed, +} from "./errors"; import { handleException, setupSentry } from "./sentry"; import type { RegistryType } from "promjs"; import type { Toucan } from "toucan-js"; -import type { ZodIssue } from "zod"; const app = new Hono<{ Bindings: Env; @@ -20,125 +27,6 @@ const app = new Hono<{ const rootDomain = ROOT; const previewDomain = PREVIEW; -export class HttpError extends Error { - constructor( - message: string, - readonly status: number, - // Only report errors to sentry when they represent actionable errors - readonly reportable: boolean - ) { - super(message); - Object.setPrototypeOf(this, new.target.prototype); - } - toResponse() { - return Response.json( - { - error: this.name, - message: this.message, - data: this.data, - }, - { - status: this.status, - headers: { - "Access-Control-Allow-Origin": "*", - "Access-Control-Allow-Methods": "GET,PUT,POST", - }, - } - ); - } - - get data(): Record { - return {}; - } -} - -export class WorkerTimeout extends HttpError { - name = "WorkerTimeout"; - constructor() { - super("Worker timed out", 400, false); - } - - toResponse(): Response { - return new Response("Worker timed out"); - } -} - -export class ServiceWorkerNotSupported extends HttpError { - name = "ServiceWorkerNotSupported"; - constructor() { - super( - "Service Workers are not supported in the Workers Playground", - 400, - false - ); - } -} -export class ZodSchemaError extends HttpError { - name = "ZodSchemaError"; - constructor(private issues: ZodIssue[]) { - super("Something went wrong", 500, true); - } - - get data(): { issues: string } { - return { issues: JSON.stringify(this.issues) }; - } -} - -export class PreviewError extends HttpError { - name = "PreviewError"; - constructor(private error: string) { - super(error, 400, false); - } - - get data(): { error: string } { - return { error: this.error }; - } -} - -class TokenUpdateFailed extends HttpError { - name = "TokenUpdateFailed"; - constructor() { - super("Provide valid token", 400, false); - } -} - -class RawHttpFailed extends HttpError { - name = "RawHttpFailed"; - constructor() { - super("Provide valid token", 400, false); - } -} - -class PreviewRequestFailed extends HttpError { - name = "PreviewRequestFailed"; - constructor(private tokenId: string | undefined, reportable: boolean) { - super("Valid token not found", 400, reportable); - } - get data(): { tokenId: string | undefined } { - return { tokenId: this.tokenId }; - } -} - -class UploadFailed extends HttpError { - name = "UploadFailed"; - constructor() { - super("Valid token not provided", 401, false); - } -} - -class PreviewRequestForbidden extends HttpError { - name = "PreviewRequestForbidden"; - constructor() { - super("Preview request forbidden", 403, false); - } -} - -export class BadUpload extends HttpError { - name = "BadUpload"; - constructor(message = "Invalid upload") { - super(message, 400, false); - } -} /** * Given a preview token, this endpoint allows for raw http calls to be inspected diff --git a/packages/playground-preview-worker/src/realish.ts b/packages/playground-preview-worker/src/realish.ts index ad794e7db5c1..7ec1f795a2a6 100644 --- a/packages/playground-preview-worker/src/realish.ts +++ b/packages/playground-preview-worker/src/realish.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import { PreviewError } from "."; +import { PreviewError } from "./errors"; const APIResponse = (resultSchema: T) => z.union([ diff --git a/packages/playground-preview-worker/src/sentry.ts b/packages/playground-preview-worker/src/sentry.ts index f7cc37604fb9..58a85c2c5f62 100644 --- a/packages/playground-preview-worker/src/sentry.ts +++ b/packages/playground-preview-worker/src/sentry.ts @@ -1,6 +1,6 @@ import { Toucan } from "toucan-js"; import { ZodError } from "zod"; -import { HttpError, ZodSchemaError } from "."; +import { HttpError, ZodSchemaError } from "./errors"; export function handleException(e: unknown, sentry: Toucan): Response { console.error(e); diff --git a/packages/playground-preview-worker/src/user.do.ts b/packages/playground-preview-worker/src/user.do.ts index 79ad42dee243..2549ca107b4d 100644 --- a/packages/playground-preview-worker/src/user.do.ts +++ b/packages/playground-preview-worker/src/user.do.ts @@ -1,10 +1,10 @@ import assert from "node:assert"; import { Buffer } from "node:buffer"; import z from "zod"; +import { BadUpload, ServiceWorkerNotSupported, WorkerTimeout } from "./errors"; import { constructMiddleware } from "./inject-middleware"; import { doUpload, setupTokens } from "./realish"; import { handleException, setupSentry } from "./sentry"; -import { BadUpload, ServiceWorkerNotSupported, WorkerTimeout } from "."; import type { RealishPreviewConfig, UploadResult } from "./realish"; const encoder = new TextEncoder(); From d07ec1b3611585d2041230e2808fb28f50fb423c Mon Sep 17 00:00:00 2001 From: bcoll Date: Wed, 7 Feb 2024 12:00:29 +0000 Subject: [PATCH 5/5] fix: include form data parsing errors in user facing message --- packages/playground-preview-worker/src/errors.ts | 5 ++++- packages/playground-preview-worker/src/user.do.ts | 2 +- packages/playground-preview-worker/tests/index.test.ts | 6 +++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/playground-preview-worker/src/errors.ts b/packages/playground-preview-worker/src/errors.ts index 13549c6563c7..edec30d65883 100644 --- a/packages/playground-preview-worker/src/errors.ts +++ b/packages/playground-preview-worker/src/errors.ts @@ -115,7 +115,10 @@ export class PreviewRequestForbidden extends HttpError { export class BadUpload extends HttpError { name = "BadUpload"; - constructor(message = "Invalid upload") { + constructor(message = "Invalid upload", private readonly error?: string) { super(message, 400, false); } + get data() { + return { error: this.error }; + } } diff --git a/packages/playground-preview-worker/src/user.do.ts b/packages/playground-preview-worker/src/user.do.ts index 2549ca107b4d..2a80f4c89f64 100644 --- a/packages/playground-preview-worker/src/user.do.ts +++ b/packages/playground-preview-worker/src/user.do.ts @@ -163,7 +163,7 @@ export class UserSession { try { worker = await request.formData(); } catch (e) { - throw new BadUpload(`Expected valid form data`); + throw new BadUpload(`Expected valid form data`, String(e)); } const m = worker.get("metadata"); diff --git a/packages/playground-preview-worker/tests/index.test.ts b/packages/playground-preview-worker/tests/index.test.ts index 34e4f0d4d939..45e043036efb 100644 --- a/packages/playground-preview-worker/tests/index.test.ts +++ b/packages/playground-preview-worker/tests/index.test.ts @@ -340,7 +340,7 @@ describe("Upload Worker", () => { }); expect(w.status).toBe(400); expect(await w.text()).toMatchInlineSnapshot( - '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"' + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{\\"error\\":\\"TypeError: Unrecognized Content-Type header value. FormData can only parse the following MIME types: multipart/form-data, application/x-www-form-urlencoded\\"}}"' ); }); it("should reject missing metadata", async () => { @@ -348,7 +348,7 @@ describe("Upload Worker", () => { method: "POST", headers: { cookie: `user=${defaultUserToken}`, - "Content-Type": "text/plain", + "Content-Type": TEST_WORKER_CONTENT_TYPE, }, body: `--${TEST_WORKER_BOUNDARY} Content-Disposition: form-data; name="index.js"; filename="index.js" @@ -362,7 +362,7 @@ export default { }); expect(w.status).toBe(400); expect(await w.text()).toMatchInlineSnapshot( - '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected valid form data\\",\\"data\\":{}}"' + '"{\\"error\\":\\"BadUpload\\",\\"message\\":\\"Expected metadata file to be defined\\",\\"data\\":{}}"' ); }); it("should reject invalid metadata json", async () => {