From 34e339b5e478fb6abed0bdb3c49deb6fa26545e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samy=20Pess=C3=A9?= Date: Fri, 9 Feb 2024 19:24:36 +0100 Subject: [PATCH] Correctly handle errors from getPublishedContentByUrl (#142) * Correctly handle errors from getPublishedContentByUrl * Format * Improve error message --- bun.lockb | Bin 469128 -> 469128 bytes package.json | 2 +- src/lib/api.ts | 92 +++++++++++++++++++++++++++++----------------- src/middleware.ts | 86 +++++++++++++++++++++++++++++++------------ 4 files changed, 123 insertions(+), 57 deletions(-) diff --git a/bun.lockb b/bun.lockb index 07a0b9c1f85bf8f7e424f6dcca28a959a9c3ff38..8853af2f482c317c52c21d84832bb08dad603119 100755 GIT binary patch delta 156 zcmV;N0Av4%(Hw};9FQ&_HzSvkJPJ?~NsZ_`>Isv<)o-K-t@j3$oG^#puDoxtO{b=l zbbK^}D5tk5rvWXNKvQp!j|H18!1NnU!ASVa_O(vpJTjV=jMWAUB4iLx0>N*jNL|I_ z_`450(1h=Q(1s+!c KG`Ge!2Ey@px8CL+9~dBC`$Ps5<`SBti|ur_m(3xuO{b;- zA(P=sE0Z8g2!kl6w0rd5BL}5u-sX3M{nx!Kwf;Tj4 z!PcK7gliPE1+|B5U^q`3oCw(client: GitBookAPI, fn: () => Promise): Promise return apiSyncStorage.run(client, fn); } -export type PublishedContentWithCache = PublishedContentLookup & { - cacheMaxAge?: number; - cacheTags?: string[]; -}; +export type PublishedContentWithCache = + | (PublishedContentLookup & { + cacheMaxAge?: number; + cacheTags?: string[]; + }) + | { + error: { + code: number; + message: string; + }; + }; /** * Get a user by its ID. @@ -102,38 +109,57 @@ export const getPublishedContentByUrl = cache( // We call it as this logic is wrapped in an asynchronous cache that is not tied to the signal. signal?.throwIfAborted(); - const response = await api().request({ - method: 'GET', - path: '/urls/published', - query: { - url, - visitorAuthToken, - }, - secure: false, - format: 'json', - signal: signal, - ...noCacheFetchOptions, - }); + try { + const response = await api().request({ + method: 'GET', + path: '/urls/published', + query: { + url, + visitorAuthToken, + }, + secure: false, + format: 'json', + signal: signal, + ...noCacheFetchOptions, + }); - const parsed = parseCacheResponse(response); + const parsed = parseCacheResponse(response); - const tags = [ - ...parsed.tags, - ...('space' in response.data - ? [getAPICacheTag({ tag: 'space', space: response.data.space })] - : []), - ]; + const tags = [ + ...parsed.tags, + ...('space' in response.data + ? [getAPICacheTag({ tag: 'space', space: response.data.space })] + : []), + ]; - const data: PublishedContentWithCache = { - ...response.data, - cacheMaxAge: parsed.ttl, - cacheTags: tags, - }; - return { - tags, - ttl: parsed.ttl, - data, - }; + const data: PublishedContentWithCache = { + ...response.data, + cacheMaxAge: parsed.ttl, + cacheTags: tags, + }; + return { + tags, + ttl: parsed.ttl, + data, + }; + } catch (error) { + const httpError = error as GitBookAPIError; + if (httpError.code < 500) { + return { + data: { + error: { + code: httpError.code, + message: httpError.errorMessage || httpError.message, + }, + } as PublishedContentWithCache, + // Cache errors for max 10 minutes in case the user is making changes to its content configuration + ttl: 60 * 10, + tags: [], + }; + } + + throw error; + } }, { // Do not pass the options for the cache key diff --git a/src/middleware.ts b/src/middleware.ts index 6c9a45c0c..9fb1ecde5 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -99,9 +99,9 @@ export async function middleware(request: NextRequest) { }), () => lookupSpaceForURL(mode, inputURL, visitorAuthToken), ); - if (!resolved) { - return new NextResponse(`Content not found`, { - status: 404, + if ('error' in resolved) { + return new NextResponse(resolved.error.message, { + status: resolved.error.code, headers: { 'x-gitbook-version': buildVersion(), }, @@ -237,7 +237,7 @@ async function lookupSpaceForURL( mode: URLLookupMode, url: URL, visitorAuthToken: string | undefined, -): Promise { +): Promise { switch (mode) { case 'single': { return await lookupSpaceInSingleMode(url); @@ -260,7 +260,7 @@ async function lookupSpaceForURL( * GITBOOK_MODE=single * When serving a single space, configured using GITBOOK_SPACE_ID and GITBOOK_TOKEN. */ -async function lookupSpaceInSingleMode(url: URL): Promise { +async function lookupSpaceInSingleMode(url: URL): Promise { const spaceId = process.env.GITBOOK_SPACE_ID; if (!spaceId) { throw new Error( @@ -290,7 +290,7 @@ async function lookupSpaceInSingleMode(url: URL): Promise { async function lookupSpaceInMultiMode( url: URL, visitorAuthToken: string | undefined, -): Promise { +): Promise { return lookupSpaceByAPI(url, visitorAuthToken); } @@ -298,24 +298,36 @@ async function lookupSpaceInMultiMode( * GITBOOK_MODE=multi-id * When serving multi spaces with the ID passed in the path. */ -async function lookupSpaceInMultiIdMode(url: URL): Promise { +async function lookupSpaceInMultiIdMode(url: URL): Promise { // Extract the iD from the path const pathSegments = url.pathname.slice(1).split('/'); if (pathSegments[0] !== '~space') { - throw new Error(`Invalid path, expected ~space`); - return null; + return { + error: { + code: 400, + message: `Missing space ID in the path`, + }, + }; } const spaceId = pathSegments[1]; if (!spaceId) { - throw new Error(`Missing space ID in the path`); - return null; + return { + error: { + code: 400, + message: `Missing space ID in the path`, + }, + }; } // Get the auth token from the URL query const apiToken = url.searchParams.get('token'); if (!apiToken) { - throw new Error(`Missing token query parameter`); - return null; + return { + error: { + code: 400, + message: `Missing token query parameter`, + }, + }; } const apiEndpoint = url.searchParams.get('api') ?? api().endpoint; @@ -346,28 +358,45 @@ async function lookupSpaceInMultiIdMode(url: URL): Promise async function lookupSpaceInMultiPathMode( url: URL, visitorAuthToken: string | undefined, -): Promise { +): Promise { // Skip useless requests if ( url.pathname === '/favicon.ico' || url.pathname === '/robots.txt' || - url.pathname === '/sitemap.xml' || - // Match something that starts with a domain like - !url.pathname.match(/^.+\..+/) + url.pathname === '/sitemap.xml' ) { - return null; + return { + error: { + code: 404, + message: `favicon.ico, robots.txt, sitemap.xml should be accessed under a content`, + }, + }; + } + // Only match something that starts with a domain like + if (!url.pathname.match(/^.+\..+/)) { + return { + error: { + code: 404, + message: `Invalid URL in the path, should start with a domain`, + }, + }; } const targetStr = `https://${url.pathname}`; if (!URL.canParse(targetStr)) { - throw new Error(`Invalid URL in the path`); + return { + error: { + code: 404, + message: `Invalid URL in the path`, + }, + }; } const target = new URL(targetStr); const lookup = await lookupSpaceByAPI(target, visitorAuthToken); - if (!lookup) { - return null; + if ('error' in lookup) { + return lookup; } if ('redirect' in lookup) { @@ -400,7 +429,7 @@ async function lookupSpaceInMultiPathMode( async function lookupSpaceByAPI( url: URL, visitorAuthToken: string | undefined, -): Promise { +): Promise { const lookupAlternatives = getURLLookupAlternatives(stripURLSearch(url)); console.log( @@ -418,6 +447,10 @@ async function lookupSpaceByAPI( signal: abort.signal, }); + if ('error' in data) { + return data; + } + if ('redirect' in data) { if (alternative.url === url.toString()) { return data; @@ -449,7 +482,14 @@ async function lookupSpaceByAPI( ); console.log(`lookup took ${Date.now() - startTime}ms`); - return matches.find((match) => match !== null) ?? null; + return ( + matches.find((match) => match !== null) ?? { + error: { + code: 404, + message: `No content found`, + }, + } + ); } function joinPath(...parts: string[]): string {