From aca4b081cd75acce26ac367ea1454ebc58cde9bc Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 21 Mar 2024 15:49:19 -0700 Subject: [PATCH 1/9] feat: quick TTFB --- package-lock.json | 8 ++++---- package.json | 2 +- src/sw.ts | 45 +++++++++++++++++++++++++++++++++++++++------ 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index f463a203..b3dc99e8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "@helia/verified-fetch": "^1.2.1", + "@helia/verified-fetch": "^1.3.0", "@libp2p/logger": "^4.0.6", "@multiformats/dns": "^1.0.5", "@sgtpooki/file-type": "^1.0.1", @@ -3016,9 +3016,9 @@ "integrity": "sha512-HzdtdBwxsIkzpeXzhQ5mAhhuxcHbjEHH+JQoxt7hG/2HGFjjwyolLo7hbaexcnhoEuV4e0TNJ8kkpMjiEYY4VQ==" }, "node_modules/@helia/verified-fetch": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@helia/verified-fetch/-/verified-fetch-1.2.1.tgz", - "integrity": "sha512-l1DJT0qkbLrGv80OgwfEwg4J0H2xpROyNMBkCEyYl1z557zRLsgMsSIuoL4dXhryYlNWZuPIAstHCvxN8YdLjw==", + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@helia/verified-fetch/-/verified-fetch-1.3.0.tgz", + "integrity": "sha512-8mEmh+A6PMnEVOwY1QT+mBMpABfhBE8qd8MbwO6zC/qrsDop9UrU1D/4Lcm9G1cjqGItPxB8u4Y1d2sUoXeHPg==", "dependencies": { "@helia/block-brokers": "^2.0.2", "@helia/car": "^3.1.0", diff --git a/package.json b/package.json index 66f4bd6c..74126cb8 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ } }, "dependencies": { - "@helia/verified-fetch": "^1.2.1", + "@helia/verified-fetch": "^1.3.0", "@libp2p/logger": "^4.0.6", "@multiformats/dns": "^1.0.5", "@sgtpooki/file-type": "^1.0.1", diff --git a/src/sw.ts b/src/sw.ts index a2420b49..91eac277 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -26,8 +26,9 @@ interface AggregateError extends Error { interface FetchHandlerArg { path: string request: Request - + event: FetchEvent } + interface GetVerifiedFetchUrlOptions { protocol?: string | null id?: string | null @@ -309,7 +310,7 @@ function getCacheKey (event: FetchEvent): string { } async function fetchAndUpdateCache (event: FetchEvent, url: URL, cacheKey: string): Promise { - const response = await fetchHandler({ path: url.pathname, request: event.request }) + const response = await fetchHandler({ path: url.pathname, request: event.request, event }) // log all of the headers: response.headers.forEach((value, key) => { @@ -381,7 +382,7 @@ async function storeReponseInCache ({ response, isMutable, cacheKey }: StoreRepo await cache.put(cacheKey, respToCache) } -async function fetchHandler ({ path, request }: FetchHandlerArg): Promise { +async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise { // test and enforce origin isolation before anything else is executed const originLocation = await findOriginIsolationRedirect(new URL(request.url)) if (originLocation !== null) { @@ -407,8 +408,29 @@ async function fetchHandler ({ path, request }: FetchHandlerArg): Promise): void => { + clearTimeout(signalAbortTimeout) + if (event?.type !== 'verified-fetch-timeout') { + log.trace('helia-sw: request signal aborted') + abortController.abort('request signal aborted') + } else { + log.trace('helia-sw: timeout waiting for response from @helia/verified-fetch') + abortController.abort('timeout') + } + } + /** + * five minute delay to get the initial response. + * + * @todo reduce to 2 minutes? + */ + const signalAbortTimeout = setTimeout(() => { + abortFn({ type: 'verified-fetch-timeout' }) + }, 5 * 60 * 1000) + // if the fetch event is aborted, we need to abort the signal we give to @helia/verified-fetch + event.request.signal.addEventListener('abort', abortFn) + try { const { id, protocol } = getSubdomainParts(request.url) const verifiedFetchUrl = getVerifiedFetchUrl({ id, protocol, path }) @@ -419,7 +441,7 @@ async function fetchHandler ({ path, request }: FetchHandlerArg): Promise Date: Thu, 21 Mar 2024 17:37:59 -0700 Subject: [PATCH 2/9] tmp: clear timeout before awaiting response --- src/sw.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sw.ts b/src/sw.ts index 91eac277..4302be0e 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -441,7 +441,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise log.trace('fetchHandler: request headers: %s: %s', key, value) }) - const response = await verifiedFetch(verifiedFetchUrl, { + const response = verifiedFetch(verifiedFetchUrl, { signal, headers, // TODO redirect: 'manual', // enable when http urls are supported by verified-fetch: https://github.com/ipfs-shipyard/helia-service-worker-gateway/issues/62#issuecomment-1977661456 @@ -459,7 +459,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise * the response object, regardless of it's inner content */ clearTimeout(signalAbortTimeout) - return response + return await response } catch (err: unknown) { const errorMessages: string[] = [] if (isAggregateError(err)) { From 84cd5d9d344eaee6690df6cc5221fa15957c50b5 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 21 Mar 2024 19:26:03 -0700 Subject: [PATCH 3/9] fix: TTI avg < 10s on big-buck-bunny from new kubo node --- package-lock.json | 1 - src/sw.ts | 31 +++++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index b3dc99e8..5c09ced0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,7 +7,6 @@ "": { "name": "helia-service-worker-gateway", "version": "1.0.0", - "hasInstallScript": true, "license": "MIT", "dependencies": { "@helia/verified-fetch": "^1.3.0", diff --git a/src/sw.ts b/src/sw.ts index 4302be0e..0630fe40 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -39,6 +39,7 @@ interface StoreReponseInCacheOptions { response: Response cacheKey: string isMutable: boolean + event: FetchEvent } /** @@ -317,12 +318,10 @@ async function fetchAndUpdateCache (event: FetchEvent, url: URL, cacheKey: strin log.trace('helia-sw: response headers: %s: %s', key, value) }) - log('helia-sw: response range header value: "%s"', response.headers.get('content-range')) - log('helia-sw: response status: %s', response.status) try { - await storeReponseInCache({ response, isMutable: true, cacheKey }) + await storeReponseInCache({ response, isMutable: true, cacheKey, event }) trace('helia-ws: updated cache for %s', cacheKey) } catch (err) { error('helia-ws: failed updating response in cache for %s', cacheKey, err) @@ -357,10 +356,25 @@ async function getResponseFromCacheOrFetch (event: FetchEvent): Promise { - // 👇 only cache successful responses - if (!response.ok || invalidOkResponseCodesForCache.some(code => code === response.status)) { +function shouldCacheResponse ({ event, response }: { event: FetchEvent, response: Response }): boolean { + if (!response.ok) { + return false + } + const invalidOkResponseCodesForCache = [206] + if (invalidOkResponseCodesForCache.some(code => code === response.status)) { + log('helia-sw-cache-debug: not caching response with status %s', response.status) + return false + } + if (event.request.headers.get('pragma') === 'no-cache' || event.request.headers.get('cache-control') === 'no-cache') { + log('helia-sw-cache-debug: request indicated no-cache, not caching') + return false + } + + return true +} + +async function storeReponseInCache ({ response, isMutable, cacheKey, event }: StoreReponseInCacheOptions): Promise { + if (!shouldCacheResponse({ event, response })) { return } trace('helia-ws: updating cache for %s in the background', cacheKey) @@ -379,7 +393,8 @@ async function storeReponseInCache ({ response, isMutable, cacheKey }: StoreRepo } log('helia-ws: storing response for key %s in cache', cacheKey) - await cache.put(cacheKey, respToCache) + // do not await this.. large responses will delay [TTFB](https://web.dev/articles/ttfb) and [TTI](https://web.dev/articles/tti) + void cache.put(cacheKey, respToCache) } async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise { From 6c2ca6f5c7fe153901da34135b15bb7610035810 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 21 Mar 2024 19:42:55 -0700 Subject: [PATCH 4/9] chore: minor cleanup --- package-lock.json | 1 + src/sw.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5c09ced0..b3dc99e8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7,6 +7,7 @@ "": { "name": "helia-service-worker-gateway", "version": "1.0.0", + "hasInstallScript": true, "license": "MIT", "dependencies": { "@helia/verified-fetch": "^1.3.0", diff --git a/src/sw.ts b/src/sw.ts index 0630fe40..53e321cc 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -456,7 +456,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise log.trace('fetchHandler: request headers: %s: %s', key, value) }) - const response = verifiedFetch(verifiedFetchUrl, { + const response = await verifiedFetch(verifiedFetchUrl, { signal, headers, // TODO redirect: 'manual', // enable when http urls are supported by verified-fetch: https://github.com/ipfs-shipyard/helia-service-worker-gateway/issues/62#issuecomment-1977661456 @@ -474,7 +474,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise * the response object, regardless of it's inner content */ clearTimeout(signalAbortTimeout) - return await response + return response } catch (err: unknown) { const errorMessages: string[] = [] if (isAggregateError(err)) { From b2e2b9774c8e5a16cb6d3be1e70bd2dc3137ec7a Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Thu, 21 Mar 2024 19:43:42 -0700 Subject: [PATCH 5/9] chore: log prefix change --- src/sw.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sw.ts b/src/sw.ts index 53e321cc..87a174b4 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -362,11 +362,11 @@ function shouldCacheResponse ({ event, response }: { event: FetchEvent, response } const invalidOkResponseCodesForCache = [206] if (invalidOkResponseCodesForCache.some(code => code === response.status)) { - log('helia-sw-cache-debug: not caching response with status %s', response.status) + log('helia-sw: not caching response with status %s', response.status) return false } if (event.request.headers.get('pragma') === 'no-cache' || event.request.headers.get('cache-control') === 'no-cache') { - log('helia-sw-cache-debug: request indicated no-cache, not caching') + log('helia-sw: request indicated no-cache, not caching') return false } From 1c65cc47f9733f58060c1e36e2e5ccef524a08cb Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 22 Mar 2024 10:53:34 -0700 Subject: [PATCH 6/9] chore: PR suggestion var name Co-authored-by: Daniel Norman <1992255+2color@users.noreply.github.com> --- src/sw.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sw.ts b/src/sw.ts index 87a174b4..bd33e8fc 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -360,7 +360,7 @@ function shouldCacheResponse ({ event, response }: { event: FetchEvent, response if (!response.ok) { return false } - const invalidOkResponseCodesForCache = [206] + const statusCodesToNotCache = [206] if (invalidOkResponseCodesForCache.some(code => code === response.status)) { log('helia-sw: not caching response with status %s', response.status) return false From ea0d4b597d36bd1fee3d6c35edbaa50aed1b486b Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 22 Mar 2024 10:57:18 -0700 Subject: [PATCH 7/9] chore: use statusCodesToNotCache --- src/sw.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sw.ts b/src/sw.ts index bd33e8fc..0008dd05 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -361,7 +361,7 @@ function shouldCacheResponse ({ event, response }: { event: FetchEvent, response return false } const statusCodesToNotCache = [206] - if (invalidOkResponseCodesForCache.some(code => code === response.status)) { + if (statusCodesToNotCache.some(code => code === response.status)) { log('helia-sw: not caching response with status %s', response.status) return false } From 5a7305a4fcd71fabed8152e4c78f9c52d7a128ed Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 22 Mar 2024 10:57:39 -0700 Subject: [PATCH 8/9] chore: use non-magic string for timeoutAbortEvent type --- src/sw.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sw.ts b/src/sw.ts index 0008dd05..565445eb 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -82,6 +82,7 @@ const CURRENT_CACHES = Object.freeze({ }) let verifiedFetch: VerifiedFetch const channel = new HeliaServiceWorkerCommsChannel('SW') +const timeoutAbortEventType = 'verified-fetch-timeout' const ONE_HOUR_IN_SECONDS = 3600 const urlInterceptRegex = [new RegExp(`${self.location.origin}/ip(n|f)s/`)] const updateVerifiedFetch = async (): Promise => { @@ -427,7 +428,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise const signal = abortController.signal const abortFn = (event: Pick): void => { clearTimeout(signalAbortTimeout) - if (event?.type !== 'verified-fetch-timeout') { + if (event?.type !== timeoutAbortEventType) { log.trace('helia-sw: request signal aborted') abortController.abort('request signal aborted') } else { @@ -441,7 +442,7 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise * @todo reduce to 2 minutes? */ const signalAbortTimeout = setTimeout(() => { - abortFn({ type: 'verified-fetch-timeout' }) + abortFn({ type: timeoutAbortEventType }) }, 5 * 60 * 1000) // if the fetch event is aborted, we need to abort the signal we give to @helia/verified-fetch event.request.signal.addEventListener('abort', abortFn) From da1286589638ed41d528b3466acf0b4d4c100907 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:14:28 -0700 Subject: [PATCH 9/9] chore: abortFn check for truth first --- src/sw.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sw.ts b/src/sw.ts index 565445eb..86305a29 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -428,12 +428,12 @@ async function fetchHandler ({ path, request, event }: FetchHandlerArg): Promise const signal = abortController.signal const abortFn = (event: Pick): void => { clearTimeout(signalAbortTimeout) - if (event?.type !== timeoutAbortEventType) { - log.trace('helia-sw: request signal aborted') - abortController.abort('request signal aborted') - } else { + if (event?.type === timeoutAbortEventType) { log.trace('helia-sw: timeout waiting for response from @helia/verified-fetch') abortController.abort('timeout') + } else { + log.trace('helia-sw: request signal aborted') + abortController.abort('request signal aborted') } } /**