From 8906bd8ffa085c76103a56f23a22e67dd8b5ded9 Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 3 Jul 2024 11:48:34 +0100 Subject: [PATCH] Demo server bookmark unfurl endpoint (#4062) This adds the HTMLRewriter-based bookmark unfurler to the demo server. It moves the unfurler into worker-shared, and adds some better shared error handling across our workers. I removed the fallback bookmark fetcher where we try and fetch websites locally. This will almost never work, as it requires sites to set public CORS. ### Change type - [x] `other` --- apps/bemo-worker/src/worker.ts | 32 ++++---- apps/dotcom-asset-upload/src/worker.ts | 25 +++---- apps/dotcom-worker/src/worker.ts | 54 ++++++-------- apps/dotcom/src/utils/createAssetFromUrl.ts | 58 ++++----------- packages/tlschema/api-report.md | 62 ++++++++-------- packages/validate/api-report.md | 5 +- packages/validate/src/lib/validation.ts | 7 +- packages/worker-shared/package.json | 2 + .../worker-shared/src/getUrlMetadata.ts | 27 +++++-- packages/worker-shared/src/handleRequest.ts | 73 +++++++++++++++++++ packages/worker-shared/src/index.ts | 8 ++ packages/worker-shared/src/sentry.ts | 2 +- packages/worker-shared/tsconfig.json | 3 + yarn.lock | 2 + 14 files changed, 213 insertions(+), 147 deletions(-) rename apps/dotcom-worker/src/utils/unfurl.ts => packages/worker-shared/src/getUrlMetadata.ts (84%) create mode 100644 packages/worker-shared/src/handleRequest.ts diff --git a/apps/bemo-worker/src/worker.ts b/apps/bemo-worker/src/worker.ts index 6682448f7..9bbae4f20 100644 --- a/apps/bemo-worker/src/worker.ts +++ b/apps/bemo-worker/src/worker.ts @@ -2,10 +2,13 @@ /// import { - createSentry, + getUrlMetadata, + handleApiRequest, handleUserAssetGet, handleUserAssetUpload, notFound, + parseRequestQuery, + urlMetadataQueryValidator, } from '@tldraw/worker-shared' import { WorkerEntrypoint } from 'cloudflare:workers' import { Router, createCors } from 'itty-router' @@ -18,7 +21,7 @@ const cors = createCors({ origins: ['*'] }) export default class Worker extends WorkerEntrypoint { private readonly router = Router() .all('*', cors.preflight) - .get('/v1/uploads/:objectName', (request) => { + .get('/uploads/:objectName', (request) => { return handleUserAssetGet({ request, bucket: this.env.BEMO_BUCKET, @@ -26,7 +29,7 @@ export default class Worker extends WorkerEntrypoint { context: this.ctx, }) }) - .post('/v1/uploads/:objectName', async (request) => { + .post('/uploads/:objectName', async (request) => { return handleUserAssetUpload({ request, bucket: this.env.BEMO_BUCKET, @@ -34,6 +37,10 @@ export default class Worker extends WorkerEntrypoint { context: this.ctx, }) }) + .get('/bookmarks/unfurl', async (request) => { + const query = parseRequestQuery(request, urlMetadataQueryValidator) + return Response.json(await getUrlMetadata(query)) + }) .get('/do', async (request) => { const bemo = this.env.BEMO_DO.get(this.env.BEMO_DO.idFromName('bemo-do')) const message = await (await bemo.fetch(request)).json() @@ -42,17 +49,12 @@ export default class Worker extends WorkerEntrypoint { .all('*', notFound) override async fetch(request: Request): Promise { - try { - return await this.router.handle(request).then(cors.corsify) - } catch (error) { - const sentry = createSentry(this.ctx, this.env, request) - console.error(error) - // eslint-disable-next-line deprecation/deprecation - sentry?.captureException(error) - return new Response('Something went wrong', { - status: 500, - statusText: 'Internal Server Error', - }) - } + return handleApiRequest({ + router: this.router, + request, + env: this.env, + ctx: this.ctx, + after: cors.corsify, + }) } } diff --git a/apps/dotcom-asset-upload/src/worker.ts b/apps/dotcom-asset-upload/src/worker.ts index fc1111809..bd0b37103 100644 --- a/apps/dotcom-asset-upload/src/worker.ts +++ b/apps/dotcom-asset-upload/src/worker.ts @@ -2,20 +2,20 @@ /// import { - createSentry, + createRouter, + handleApiRequest, handleUserAssetGet, handleUserAssetUpload, notFound, } from '@tldraw/worker-shared' import { WorkerEntrypoint } from 'cloudflare:workers' import { createCors } from 'itty-cors' -import { Router } from 'itty-router' import { Environment } from './types' const { preflight, corsify } = createCors({ origins: ['*'] }) export default class Worker extends WorkerEntrypoint { - readonly router = Router() + readonly router = createRouter() .all('*', preflight) .get('/uploads/:objectName', async (request) => { return handleUserAssetGet({ @@ -36,17 +36,12 @@ export default class Worker extends WorkerEntrypoint { .all('*', notFound) override async fetch(request: Request) { - try { - return await this.router.handle(request, this.env, this.ctx).then(corsify) - } catch (error) { - const sentry = createSentry(this.ctx, this.env, request) - console.error(error) - // eslint-disable-next-line deprecation/deprecation - sentry?.captureException(error) - return new Response('Something went wrong', { - status: 500, - statusText: 'Internal Server Error', - }) - } + return handleApiRequest({ + router: this.router, + request, + env: this.env, + ctx: this.ctx, + after: corsify, + }) } } diff --git a/apps/dotcom-worker/src/worker.ts b/apps/dotcom-worker/src/worker.ts index ab8f09bd4..c7780d190 100644 --- a/apps/dotcom-worker/src/worker.ts +++ b/apps/dotcom-worker/src/worker.ts @@ -6,9 +6,16 @@ import { ROOM_OPEN_MODE, ROOM_PREFIX, } from '@tldraw/dotcom-shared' -import { T } from '@tldraw/validate' -import { createSentry, notFound } from '@tldraw/worker-shared' -import { Router, createCors, json } from 'itty-router' +import { + createRouter, + getUrlMetadata, + handleApiRequest, + notFound, + parseRequestQuery, + urlMetadataQueryValidator, +} from '@tldraw/worker-shared' +import { WorkerEntrypoint } from 'cloudflare:workers' +import { createCors, json } from 'itty-router' import { createRoom } from './routes/createRoom' import { createRoomSnapshot } from './routes/createRoomSnapshot' import { forwardRoomRequest } from './routes/forwardRoomRequest' @@ -18,14 +25,13 @@ import { getRoomHistorySnapshot } from './routes/getRoomHistorySnapshot' import { getRoomSnapshot } from './routes/getRoomSnapshot' import { joinExistingRoom } from './routes/joinExistingRoom' import { Environment } from './types' -import { unfurl } from './utils/unfurl' export { TLDrawDurableObject } from './TLDrawDurableObject' const { preflight, corsify } = createCors({ origins: Object.assign([], { includes: (origin: string) => isAllowedOrigin(origin) }), }) -const router = Router() +const router = createRouter() .all('*', preflight) .all('*', blockUnknownOrigins) .post('/new-room', createRoom) @@ -44,31 +50,20 @@ const router = Router() .get(`/${ROOM_PREFIX}/:roomId/history/:timestamp`, getRoomHistorySnapshot) .get('/readonly-slug/:roomId', getReadonlySlug) .get('/unfurl', async (req) => { - if (typeof req.query.url !== 'string' || !T.httpUrl.isValid(req.query.url)) { - return new Response('url query param is required', { status: 400 }) - } - return json(await unfurl(req.query.url)) + const query = parseRequestQuery(req, urlMetadataQueryValidator) + return json(await getUrlMetadata(query)) }) .post(`/${ROOM_PREFIX}/:roomId/restore`, forwardRoomRequest) .all('*', notFound) -const Worker = { - fetch(request: Request, env: Environment, context: ExecutionContext) { - const sentry = createSentry(context, env, request) - - return router - .handle(request, env, context) - .catch((err) => { - console.error(err) - // eslint-disable-next-line deprecation/deprecation - sentry?.captureException(err) - - return new Response('Something went wrong', { - status: 500, - statusText: 'Internal Server Error', - }) - }) - .then((response) => { +export default class Worker extends WorkerEntrypoint { + override async fetch(request: Request): Promise { + return await handleApiRequest({ + router, + request, + env: this.env, + ctx: this.ctx, + after: (response) => { const setCookies = response.headers.getAll('set-cookie') // unfortunately corsify mishandles the set-cookie header, so // we need to manually add it back in @@ -83,8 +78,9 @@ const Worker = { newResponse.headers.append('set-cookie', cookie) } return newResponse - }) - }, + }, + }) + } } export function isAllowedOrigin(origin: string) { @@ -117,5 +113,3 @@ async function blockUnknownOrigins(request: Request, env: Environment) { // origin doesn't match, so we can continue return undefined } - -export default Worker diff --git a/apps/dotcom/src/utils/createAssetFromUrl.ts b/apps/dotcom/src/utils/createAssetFromUrl.ts index 403a33d0b..9cbd2604f 100644 --- a/apps/dotcom/src/utils/createAssetFromUrl.ts +++ b/apps/dotcom/src/utils/createAssetFromUrl.ts @@ -9,6 +9,7 @@ interface ResponseBody { } export async function createAssetFromUrl({ url }: { type: 'url'; url: string }): Promise { + const urlHash = getHashForString(url) try { // First, try to get the meta data from our endpoint const fetchUrl = @@ -18,65 +19,34 @@ export async function createAssetFromUrl({ url }: { type: 'url'; url: string }): url, }).toString() - const meta = (await (await fetch(fetchUrl)).json()) as ResponseBody + const meta = (await (await fetch(fetchUrl)).json()) as ResponseBody | null return { - id: AssetRecordType.createId(getHashForString(url)), + id: AssetRecordType.createId(urlHash), typeName: 'asset', type: 'bookmark', props: { src: url, - description: meta.description ?? '', - image: meta.image ?? '', - favicon: meta.favicon ?? '', - title: meta.title ?? '', + description: meta?.description ?? '', + image: meta?.image ?? '', + favicon: meta?.favicon ?? '', + title: meta?.title ?? '', }, meta: {}, } } catch (error) { - // Otherwise, fallback to fetching data from the url - - let meta: { image: string; favicon: string; title: string; description: string } - - try { - const resp = await fetch(url, { - method: 'GET', - mode: 'no-cors', - }) - const html = await resp.text() - const doc = new DOMParser().parseFromString(html, 'text/html') - meta = { - image: doc.head.querySelector('meta[property="og:image"]')?.getAttribute('content') ?? '', - favicon: - doc.head.querySelector('link[rel="apple-touch-icon"]')?.getAttribute('href') ?? - doc.head.querySelector('link[rel="icon"]')?.getAttribute('href') ?? - '', - title: doc.head.querySelector('meta[property="og:title"]')?.getAttribute('content') ?? '', - description: - doc.head.querySelector('meta[property="og:description"]')?.getAttribute('content') ?? '', - } - if (!meta.image.startsWith('http')) { - meta.image = new URL(meta.image, url).href - } - if (!meta.favicon.startsWith('http')) { - meta.favicon = new URL(meta.favicon, url).href - } - } catch (error) { - console.error(error) - meta = { image: '', favicon: '', title: '', description: '' } - } - - // Create the bookmark asset from the meta + // Otherwise, fallback to a blank bookmark + console.error(error) return { - id: AssetRecordType.createId(getHashForString(url)), + id: AssetRecordType.createId(urlHash), typeName: 'asset', type: 'bookmark', props: { src: url, - image: meta.image, - favicon: meta.favicon, - title: meta.title, - description: meta.description, + description: '', + image: '', + favicon: '', + title: '', }, meta: {}, } diff --git a/packages/tlschema/api-report.md b/packages/tlschema/api-report.md index 428b4f2cb..69d2d65e4 100644 --- a/packages/tlschema/api-report.md +++ b/packages/tlschema/api-report.md @@ -104,31 +104,31 @@ export const CameraRecordType: RecordType; export const canvasUiColorTypeValidator: T.Validator<"accent" | "black" | "laser" | "muted-1" | "selection-fill" | "selection-stroke" | "white">; // @public -export function createAssetValidator(type: Type, props: T.Validator): T.ObjectValidator<{ [P in T.ExtractRequiredKeys<{ - id: TLAssetId; - meta: JsonObject; - props: Props; - type: Type; - typeName: 'asset'; - }>]: { - id: TLAssetId; - meta: JsonObject; - props: Props; - type: Type; - typeName: 'asset'; - }[P]; } & { [P_1 in T.ExtractOptionalKeys<{ - id: TLAssetId; - meta: JsonObject; - props: Props; - type: Type; - typeName: 'asset'; - }>]?: { - id: TLAssetId; - meta: JsonObject; - props: Props; - type: Type; - typeName: 'asset'; - }[P_1] | undefined; }>; +export function createAssetValidator(type: Type, props: T.Validator): T.ObjectValidator]: { +id: TLAssetId; +meta: JsonObject; +props: Props; +type: Type; +typeName: 'asset'; +}[P]; } & { [P_1 in T.ExtractOptionalKeys<{ +id: TLAssetId; +meta: JsonObject; +props: Props; +type: Type; +typeName: 'asset'; +}>]?: { +id: TLAssetId; +meta: JsonObject; +props: Props; +type: Type; +typeName: 'asset'; +}[P_1] | undefined; }>>; // @public (undocumented) export function createBindingId(id?: string): TLBindingId; @@ -146,7 +146,7 @@ export function createBindingValidator; }, meta?: { [K in keyof Meta]: T.Validatable; -}): T.ObjectValidator<{ [P in T.ExtractRequiredKeys>]: TLBaseBinding[P]; } & { [P_1 in T.ExtractOptionalKeys>]?: TLBaseBinding[P_1] | undefined; }>; +}): T.ObjectValidator>]: TLBaseBinding[P]; } & { [P_1 in T.ExtractOptionalKeys>]?: TLBaseBinding[P_1] | undefined; }>>; // @public export const createPresenceStateDerivation: ($user: Signal<{ @@ -171,7 +171,7 @@ export function createShapeValidator; }, meta?: { [K in keyof Meta]: T.Validatable; -}): T.ObjectValidator<{ [P in T.ExtractRequiredKeys>]: TLBaseShape[P]; } & { [P_1 in T.ExtractOptionalKeys>]?: TLBaseShape[P_1] | undefined; }>; +}): T.ObjectValidator>]: TLBaseShape[P]; } & { [P_1 in T.ExtractOptionalKeys>]?: TLBaseShape[P_1] | undefined; }>>; // @public export function createTLSchema({ shapes, bindings, migrations, }?: { @@ -588,7 +588,7 @@ export function idValidator>(prefix: Id['__ty export const ImageShapeCrop: T.ObjectValidator<{ bottomRight: VecModel; topLeft: VecModel; -} & {}>; +}>; // @public (undocumented) export const imageShapeMigrations: TLPropsMigrations; @@ -596,10 +596,10 @@ export const imageShapeMigrations: TLPropsMigrations; // @public (undocumented) export const imageShapeProps: { assetId: T.Validator; - crop: T.Validator<({ + crop: T.Validator<{ bottomRight: VecModel; topLeft: VecModel; - } & {}) | null>; + } | null>; h: T.Validator; playing: T.Validator; url: T.Validator; @@ -753,7 +753,7 @@ export const lineShapeProps: { index: IndexKey; x: number; y: number; - } & {}>; + }>; scale: T.Validator; size: EnumStyleProp<"l" | "m" | "s" | "xl">; spline: EnumStyleProp<"cubic" | "line">; diff --git a/packages/validate/api-report.md b/packages/validate/api-report.md index 742b748b8..ec014c051 100644 --- a/packages/validate/api-report.md +++ b/packages/validate/api-report.md @@ -4,6 +4,7 @@ ```ts +import { Expand } from '@tldraw/utils'; import { IndexKey } from '@tldraw/utils'; import { JsonValue } from '@tldraw/utils'; @@ -102,11 +103,11 @@ function numberUnion(config: { readonly [K in keyof Shape]: Validatable; -}): ObjectValidator<{ +}): ObjectValidator]: Shape[P]; } & { [P in ExtractOptionalKeys]?: Shape[P]; -}>; +}>>; // @public (undocumented) export class ObjectValidator extends Validator { diff --git a/packages/validate/src/lib/validation.ts b/packages/validate/src/lib/validation.ts index 5ac3f0800..ab4860b6c 100644 --- a/packages/validate/src/lib/validation.ts +++ b/packages/validate/src/lib/validation.ts @@ -1,4 +1,5 @@ import { + Expand, IndexKey, JsonValue, STRUCTURED_CLONE_OBJECT_PROTOTYPE, @@ -696,7 +697,11 @@ export type ExtractOptionalKeys = { export function object(config: { readonly [K in keyof Shape]: Validatable }): ObjectValidator< - { [P in ExtractRequiredKeys]: Shape[P] } & { [P in ExtractOptionalKeys]?: Shape[P] } + Expand< + { [P in ExtractRequiredKeys]: Shape[P] } & { + [P in ExtractOptionalKeys]?: Shape[P] + } + > > { return new ObjectValidator(config) as any } diff --git a/packages/worker-shared/package.json b/packages/worker-shared/package.json index 5433aba98..b2d7d2f8c 100644 --- a/packages/worker-shared/package.json +++ b/packages/worker-shared/package.json @@ -10,6 +10,8 @@ "dependencies": { "@cloudflare/workers-types": "^4.20240620.0", "@tldraw/utils": "workspace:*", + "@tldraw/validate": "workspace:*", + "itty-router": "^4.0.13", "lazyrepo": "0.0.0-alpha.27", "toucan-js": "^3.4.0", "typescript": "^5.3.3" diff --git a/apps/dotcom-worker/src/utils/unfurl.ts b/packages/worker-shared/src/getUrlMetadata.ts similarity index 84% rename from apps/dotcom-worker/src/utils/unfurl.ts rename to packages/worker-shared/src/getUrlMetadata.ts index dad78ee8f..d5c3cbe78 100644 --- a/apps/dotcom-worker/src/utils/unfurl.ts +++ b/packages/worker-shared/src/getUrlMetadata.ts @@ -1,3 +1,9 @@ +import { T } from '@tldraw/validate' + +export const urlMetadataQueryValidator = T.object({ + url: T.httpUrl, +}) + class TextExtractor { string = '' text({ text }: any) { @@ -38,18 +44,23 @@ class IconExtractor { } } -export async function unfurl(url: string) { +export async function getUrlMetadata({ url }: { url: string }) { const meta$ = new MetaExtractor() const title$ = new TextExtractor() const icon$ = new IconExtractor() - // we use cloudflare's special html parser https://developers.cloudflare.com/workers/runtime-apis/html-rewriter/ - await new HTMLRewriter() - .on('meta', meta$) - .on('title', title$) - .on('link', icon$) - .transform((await fetch(url)) as any) - .blob?.() + try { + await new HTMLRewriter() + .on('meta', meta$) + .on('title', title$) + .on('link', icon$) + .transform((await fetch(url)) as any) + .blob() + } catch { + return null + } + + // we use cloudflare's special html parser https://developers.cloudflare.com/workers/runtime-apis/html-rewriter/ const { og, twitter } = meta$ const title = og['og:title'] ?? twitter['twitter:title'] ?? title$.string ?? undefined const description = diff --git a/packages/worker-shared/src/handleRequest.ts b/packages/worker-shared/src/handleRequest.ts new file mode 100644 index 000000000..1bb2d2a0e --- /dev/null +++ b/packages/worker-shared/src/handleRequest.ts @@ -0,0 +1,73 @@ +import { T } from '@tldraw/validate' +import { IRequest, RouteHandler, Router, RouterType, StatusError } from 'itty-router' +import { SentryEnvironment, createSentry } from './sentry' + +export type ApiRoute = ( + path: string, + ...handlers: RouteHandler[] +) => RouterType, [env: Env, ctx: Ctx]> + +export type ApiRouter = RouterType< + ApiRoute, + [env: Env, ctx: Ctx] +> + +export function createRouter< + Env extends SentryEnvironment, + Ctx extends ExecutionContext = ExecutionContext, +>() { + const router: ApiRouter = Router() + return router +} + +export async function handleApiRequest< + Env extends SentryEnvironment, + Ctx extends ExecutionContext, +>({ + router, + request, + env, + ctx, + after, +}: { + router: ApiRouter + request: Request + env: Env + ctx: Ctx + after: (response: Response) => Response | Promise +}) { + let response + try { + response = await router.handle(request, env, ctx) + } catch (error: any) { + if (error instanceof StatusError) { + console.error(`${error.status}: ${error.stack}`) + response = Response.json({ error: error.message }, { status: error.status }) + } else { + response = Response.json({ error: 'Internal server error' }, { status: 500 }) + console.error(error.stack ?? error) + // eslint-disable-next-line deprecation/deprecation + createSentry(ctx, env, request)?.captureException(error) + } + } + + try { + return await after(response) + } catch (error: any) { + console.error(error.stack ?? error) + // eslint-disable-next-line deprecation/deprecation + createSentry(ctx, env, request)?.captureException(error) + return Response.json({ error: 'Internal server error' }, { status: 500 }) + } +} + +export function parseRequestQuery(request: IRequest, validator: T.Validator) { + try { + return validator.validate(request.query) + } catch (err) { + if (err instanceof T.ValidationError) { + throw new StatusError(400, `Query parameters: ${err.message}`) + } + throw err + } +} diff --git a/packages/worker-shared/src/index.ts b/packages/worker-shared/src/index.ts index ef56b93c4..ee54611f4 100644 --- a/packages/worker-shared/src/index.ts +++ b/packages/worker-shared/src/index.ts @@ -2,5 +2,13 @@ /// export { notFound } from './errors' +export { getUrlMetadata, urlMetadataQueryValidator } from './getUrlMetadata' +export { + createRouter, + handleApiRequest, + parseRequestQuery, + type ApiRoute, + type ApiRouter, +} from './handleRequest' export { createSentry } from './sentry' export { handleUserAssetGet, handleUserAssetUpload } from './userAssetUploads' diff --git a/packages/worker-shared/src/sentry.ts b/packages/worker-shared/src/sentry.ts index 6cfe63792..c51712b4d 100644 --- a/packages/worker-shared/src/sentry.ts +++ b/packages/worker-shared/src/sentry.ts @@ -7,7 +7,7 @@ interface Context { request?: Request } -interface SentryEnvironment { +export interface SentryEnvironment { readonly SENTRY_DSN: string | undefined readonly TLDRAW_ENV?: string | undefined readonly WORKER_NAME: string | undefined diff --git a/packages/worker-shared/tsconfig.json b/packages/worker-shared/tsconfig.json index 8ea0a3fbf..daf5f6861 100644 --- a/packages/worker-shared/tsconfig.json +++ b/packages/worker-shared/tsconfig.json @@ -9,6 +9,9 @@ "references": [ { "path": "../utils" + }, + { + "path": "../validate" } ] } diff --git a/yarn.lock b/yarn.lock index fbac052ca..aec71702e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6357,6 +6357,8 @@ __metadata: dependencies: "@cloudflare/workers-types": "npm:^4.20240620.0" "@tldraw/utils": "workspace:*" + "@tldraw/validate": "workspace:*" + itty-router: "npm:^4.0.13" lazyrepo: "npm:0.0.0-alpha.27" toucan-js: "npm:^3.4.0" typescript: "npm:^5.3.3"