cloudflare / kv-asset-handler Goto Github PK

This bug is described here: https://github.com/cloudflare/wrangler/issues/928.

To summarize, the Cache API can't serve range requests to Safari because the cache key is stripping the client's request headers (namely, the Range header). Opening a PR to fix this now

I was testing my site with our 0.0.4 release and noticed that our redirect logic for /about -> /about/index.html is now broken. Example: https://blog-gabbi-test.ziggy.workers.dev/about

See more detail in my comment here: https://github.com/cloudflare/kv-asset-handler/pull/115/files#r445871787

tl;dr: now that we're including ETag headers on cached responses, and If-None-Match headers on requests, calls to cache.match(request) may return 304 Not Modified responses which have no body. More detail about how this happens is in the Cache API docs for the match method.

This (a) throws a TypeError on matches when accessing request.body.cancel in the current implementation, and (b) means we should probably revisit the shouldRevalidate logic.

You can repro by:

1. Set up a new Gatsby project
2. Follow the docs to deploy it via Workers Sites
3. Check out the tip of master of kv-asset-handler in another directory
4. Run npm link there
5. cd ~/my-gatsby-project && npm link @cloudflare/kv-asset-handler to replace the npm package with your local tip of master
6. wrangler publish again
7. Smash reload a bunch of times and observe 500s on every 2nd request, when the request has If-None-Match headers and thus triggers a cache match

The simple solution is to defend against request.body being null in the shouldRevalidate logic, but I'm not sure if we need to make further updates to that logic for the sake of correctness. We'll need to address this before releasing 0.0.11.

FYI @shagamemnon @harrishancock @SukkaW

this package assumes (generally) that it will be used in conjunction with wrangler using the [site] feature, but it would be useful to include in the readme more about the different options one can pass to the various functions, as well as the assumptions about bindings present in the worker.

It seems that this package still has the assumption of the old worker format (e.g. global environment variables, FetchEvent, etc) and does not work properly with the new ES6 module format. I would like to use Durable Objects, which requires the new ES6 module format, in a Workers Site project, and wrangler already has a patchset for Workers Site support with the new ES6 module format, and the fact that this package does not support the new format seems to be the last blocker.

getAssetFromKV() currently throws an exception when the key is not found. The documentation and auto-generated code suggest the caller should catch all exceptions and serve a 404 page in response.

I'm concerned, though, that this means other kinds of exceptions -- like transient network errors -- get converted to 404s. Returning a 404 in this case might confuse some clients, especially automated ones. For example, a web crawler knows that if it gets a 500-level error, it should try again later while continuing to index the last version it successfully received. However, if it gets a 404, it may believe the page is gone, evict the page from the index and may not try to fetch it again for some time.

Any thoughts on this?

Hello,

I have a javascript file that is being changed to mime type "text/plain". Is there a reason this is happening?

Thanks

Now that we've merged @shagamemnon 's change to support ETags in #94 , we should document the default behavior in the README.

If I have an asset with a filename test file.jpg the corresponding url is /test%20file.jpg, which is correct. However, getAssetFromKV tries to access it as test%20file.jpg which does not exist and fails.

the URL constructor converts all spaces not already encoded to %20 automatically, so a simplistic solution would be running decodeURI here after the url was already parsed with URL:

Right now, we check for a content-type for the request we are receiving. If this comes back null, we assume what the request really wants is at path/index.html, but strictly speaking this isn’t entirely true. Here’s my proposed change:

if the path ends in /

serve the asset at /index.html, if any (existing logic)

if the path does not end in /

1. Check for a mime type based on the extension (mime.getType()). If this comes back with something, use the existing path. (this is the existing behavior).
2. If there’s no extension, check the Accept header. This is a comma-separated list of content types. Most likely, 99% of the time this will start with text/html. If we call mime.getExtension() on each item in this list in turn, we can check the asset manifest for the path + the given extension. If it exists there, return the asset in question.
3. Finally, if there isn’t an asset with the exact name + extension, and it’s supposed to be an html file, try finding the resource + index.html. (this is also existing behavior) redirect to the same path plus /, as discussed in this article: How to force https, www and a trailing slash with one redirect - Daniel Morell.

It would be awesome if we could "hijack" requests and serve different assets to that request, even if that filename exists in the ASSET_MANIFEST. An immediate use-case for this would be to serve newer image formats to clients that support them, such as WebP or AVIF images, even when a JPEG is requested.

The mapRequestToAsset would be perfect for this use-case, if it allowed this function to entirely override the requestKey. The code in this file doesn't seem to support this though:

Pseudo-code example if this were possible:

// handle images rewrite to webp
if(url.pathname.match(imageRegex)){
	// request for an image. Check if we can serve webp instead
	if(event.request.headers.has('accept') && event.request.headers.get('accept').match(/image\/webp/)){
		options.mapRequestToAsset = (request) => {
			const parsedUrl = new URL(request.url);
			parsedUrl.pathname = parsedUrl.pathname.replace(imageRegex, '.webp');
			return new Request(parsedUrl.toString(), request);
		};
	}
}

Note: This actually used to work in much older versions of kv-asset-handler:

Let me know if I can provide any more info here!

It appears that the default behavior is that getAssetFromKV will parse the __STATIC_CONTENT_MANIFEST global string value into an object every time getAssetFromKV is called:

https://github.com/cloudflare/kv-asset-handler/blob/master/src/index.ts#L73-L90

This is unnecessary and has the potential to meaningfully impact TTFB for large sites. We should instead parse the manifest once on script load.

🐛 Bug Report

Environment

• operating system: OSX
• output of rustc -V: error: no input filename given
• output of node -v: v12.10.0
• output of wrangler -V: 👷 ✨ wrangler 1.4.0-rc.1

Steps to reproduce

Create a custom keyModifier that returns a single filename that exists in ./public:

return await getAssetFromKV(event, {keyModifier: (pathname) => 'shrek.txt'});

What did you expect to see?

The worker should return the contents of shrek.txt.

What did you see instead?

The worker checks KV for hrek.txt and returns a 404. It's fixed by prepending a / to the keyModifier's return value, but I wasn't expecting it to always remove the first character.

I tried to publish a Hugo site with a Japanese URL on Workers Sites, but it returned Not Found.
For example: https://example.com/日本語/ to /日本語/index.html

Since https://example.com/日本語/index.html can be can be displayed. If you specify other files, they can be displayed.

It was previously pointed out in a pull request, but it hasn't been merged:
#142

v0.0.11 added support for cache revalidation via etags. Per RFC2616, Cloudflare does not alter responses from cache that have a strongly-held etag header, so resources that are not gzipped or brotli compressed by the origin server (or kv-asset-handler) will either have their etags stripped or remain in their uncompressed state when delivered to a client.

Some requirements for implementing compression on text files:

• Must be able to serve files in both uncompressed and compressed form based on the client's Accept-Encoding request header
• Must provide gzip compression (brotli is a nice-to-have)
• Must include content-type and content-encoding response headers upon cache insertion
• Each variant of content-encoding will need to produce a corresponding etag variant
• Must detect mime type of assets to determine which files can be compressed

Compression can be handled in one of three ways:

1. Apply compression on-the-fly in Workers. This would probably require an NPM package and is the least resource-efficient option available
2. Allow Cloudflare CDN to apply compression on-the-fly. This would require modifying both the format of the etag and the comparison validation function.
3. Apply compression during upload to KV. This is probably the best solution and certainly the most resource efficient

Proposal on how to fix issues like #44

Custom Error class

I suggest we create a custom error class that looks something like

export class KVError extends Error {
  constructor(message?: string, code: number = 500) {
    super(message)
    Object.setPrototypeOf(this, new.target.prototype) // restore prototype chain
    this.name = KVError.name // stack traces display correctly now
    this.code = code
  }
  code: number // Appropriate HTTP status code
}

getAssetFromKV throws custom error

getAssetFromKV will now throw custom errors that we can identify as KVError.

Example inside getAssetFromKV

    throw new KVError(`${request.method} is not a valid request method`, 405)
    throw new KVError(`there is no ${ASSET_NAMESPACE} namespace bound to the script`, 500)

Workers sites template

now instead of having crazy try/catch blocks and assuming a 404 error in the sites template, we'd have workers-site-template look like:

try {
  let resp = await getAssetFromKV()
  return new Response(resp.message, { status: resp.code })
} catch (e) {
  if (resp instanceof KVError) {
    switch (e.code) {
      case 404:
        return notFoundResponse
        break
      case 401:
        return methodNotAllowedResponse
        break
      default:
        return new Response(resp.message, { status: resp.code })
    }
  }
}

Alternative option: getAssetFromKV returns a custom error

getAssetFromKV will now not throw generated errors, but instead optionally return KVError.

const getAssetFromKV: (event..) => Promise<KVError | Response>

Examples of returning these errors instead of throwing them:

    return new KVError(`${request.method} is not a valid request method`, 405)
    return new KVError(`there is no ${ASSET_NAMESPACE} namespace bound to the script`, 500)

Hello! @AustinCorridor ran into an issue where the HTML of his site was being delivered with a Cache-Control TTL of one year, causing it to not update properly for some clients as he changed it. Is this expected behavior of Wrangler / the kv-asset-handler? Thank you!

When using getAssetFromKV, it infers the Content-Type of the requested asset and sets it to the Content-Type header. That code is here.

The issue is that for Content-Types such as text/html and text/plain, it is often preferred to append this: "; charset=utf-8". This does not happen currently.

Of course I can do this myself, but something like this should be built-in. Maybe as a new option.

at the moment in the code we provide an option to set a custom ASSET_NAMESPACE binding, but it appears that we actually never use it.

If we want this option to work, we need to test and verify the following scenarios:

1. Proof of the bug: Define a kv namespace for both CUSTOM_NAMESPACE and __STATIC_CONTENT. Pass CUSTOM_NAMESPACE in to the function as options.ASSET_NAMESPACE. You would expect to get back the result from CUSTOM_NAMESPACE, but you will actually get back the result from __STATIC_CONTENT.

My working hypothesis is that if the global __STATIC_CONTENT is undefined, this will actually throw an undefined exception in the current code.

2. I fail to bind anything to the global variable CUSTOM_NAMESPACE, and pass it in as part of the options object under ASSET_NAMESPACE. The kv asset handler should throw an InternalError when it does the typeof check against ASSET_NAMESPACE. I believe this happens with the code right now.

3. (Control test): I bind a kv namespace to the global __STATIC_CONTENT and do not pass anything in to the options object. The kv asset handler should call .get on the static content object and return the result.

4. Proof the bug is squashed: run test #1 above. You should get back the response from the CUSTOM_NAMESPACE.

Recently, I try to publish a react application with Workers KV. In order to optimize the loading time further, I wonder how to implement the server push features with Workers KV. Thank you very much.

I just had a strange bug report on my worker (which uses v0.012 of this package):

Failed to execute 'cancel' on 'ReadableStream': parameter 1 is not of type 'Value'.

The only cancel I could find was here:
https://github.com/cloudflare/kv-asset-handler/blob/master/src/index.ts#L192

In both cases it was on a favicon (https://app.bilderbrief.de/favicon-32.png and https://app.bilderbrief.de/favicon-192.png)

According to https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/cancel the (optional) parameter of cancel should be a DOMString and not a Value as complained here.

Value seems to refer to the underlying implementation (see https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/core/streams/readable_stream.cc#L1607-L1609)

As it stands the handling of HEAD requests with the default mapRequestToAsset handler and cacheControl object results in an error response from the Cloudflare Key Value Store. This shows up in the local console as:

Uncaught (in promise)
TypeError: Cannot cache response to non-GET request.

and in the worker logs as:

"exceptions":[{"name":"TypeError","message":"Cannot cache response to non-GET request.","timestamp":1596746202312}]

The mitigant is straightforward on the caller's side but given that the library explicitly claims to handle HEAD it should probably be handled internally. I'm opening a PR to do so.

In the meantime, here's a fix for the caller (assuming that the caller still wants to rely on getAssetFromKV throwing when the http verb isn't a GET or HEAD:

  const cacheControl: Partial<CacheControl> = {
    bypassCache: method !== 'GET',
  };

https://github.com/cloudflare/kv-asset-handler#getassetfromkv

In the README example resp is undefined and NotFoundError is never imported.

import { getAssetFromKV } from '@cloudflare/kv-asset-handler'

addEventListener('fetch', event => {
  event.respondWith(handleEvent(event))
})

async function handleEvent(event) {
  if (event.request.url.includes('/docs')) {
    try {
      return await getAssetFromKV(event)
    } catch (e) {
        switch (typeof resp) {
          case NotFoundError: 
            //..
          case MethodNotAllowedError:
            // ...
          default:
            return new Response("An unexpected error occurred", { status: 500 })
        }
      }
    }
  } else return fetch(event.request)
}

Currently in getAssetFromKV the HTTP Method is checked before the call to mapRequestToAsset:

I suggest to check the HTTP method after this.

In my case, I want to send an e-mail on a webhook (which is a POST call from an external service - payment provider for instance).
I store my email template along my static site, so that it is uploaded to the KV store.
I want to retrieve the template using getAssetFromKV, (replacing the request path using mapRequestToAsset), however it fails because the method check is done before this.

I currently have an ugly hack, creating a new pseudo-event with a fresh GET request...

I don't see any drawback to moving this check after the if block (mapRequestToAsset is not supposed to have any side-effect, so calling it before failing should not be a big deal9.

I would be happy to make a PR if you think this is not a bad idea.

Asset manifest updates much faster than KV, this will make people's pages 404 for a small window while KV updates. We should probably check KV for the thing the asset manifest points to first, but if it's not there, look for the stuff that used to be in kv with like a regex or something

Looks like 0.1.3 has been released but not yet published to NPM:

https://www.npmjs.com/package/@cloudflare/kv-asset-handler

Will it be published soon? Looking to upgrade to get unblocked from this issue: #174

Everything is in the title :) Would be nice to have types. Or should types be part of @cloudflare/workers-types?

when mime can't find a type, it returns null, which is not a content-type. we should have a sane default, perhaps text/plain?

Considering a URL: https://example.com/你好/:

const url = new URL('https://example.com/你好/');
url.href;
// "https://example.com/%E4%BD%A0%E5%A5%BD/"
url.pathname;
// "/%E4%BD%A0%E5%A5%BD/"

But during wrangler publish, the file will be stored using original name in Workers KV:

Thus the url.pathname should be decodeURI before being used as rawPathKey.

This should be considered as a critical bug (It will cause every non-ascii URL path to be 404) and an emergency fix should be released.

Hi,

I'd like to have a different browser cache max-age for HTML and assets. Is this possible somehow or does it need a change to the library?

Thanks,
Agost

I have a website in Angular and it is published using workers KV.
With version 0.0.7 I use serveSinglePageApp. It works when the url is like
mysite.com/buy/item

but if the url is like
mysite.com/buy/item?id=1234
it get an "could not find buy/item/index.html in your content namespace"

I know that the best design should be buy/item/1234 but i can not manage that because I need to manage externals responses to my site so I can not change that way.

Is any way to solve this?

Thanks in advance!
Sebastián

Are there any limits to metadata associated with keys? Is 250 bytes/characters acceptable?

{
   "token":"000000000000000000000000000000000000000000000",
   "dropin":"1.6.0",
   "foobar":"4.0.9",
   "framework":"5.4.1",
   "network":"0",
   "php":"7.3.18-1+ubuntu18.04.1+deb.sury.org+1",
   "extention":"5.2.1",
   "binary":"3.1.2",
   "openssl":"7.3.18-1+ubuntu18.04.1+deb.sury.org+1"
}

In the example in the readme to strip /docs from any incoming request before looking up an asset in Cloudflare's KV:

import { getAssetFromKV, mapRequestToAsset } from '@cloudflare/kv-asset-handler'
...
const customKeyModifier = request => {
  let url = request.url
  //custom key mapping optional
  url.replace('/docs', '').replace(/^\/+/, '')
  return mapRequestToAsset(new Request(url, request))
}
let asset = await getAssetFromKV(event, { mapRequestToAsset: customKeyModifier })

The string replace method used on url returns a new string, so you need to set the result to a variable. As written, the example passes the url to the new Request object unmodified.

Here's the modification:

import { getAssetFromKV, mapRequestToAsset } from '@cloudflare/kv-asset-handler'
...
const customKeyModifier = request => {
  let url = request.url
  //custom key mapping optional
  url = url.replace('/docs', '').replace(/^\/+/, '')
  return mapRequestToAsset(new Request(url, request))
}
let asset = await getAssetFromKV(event, { mapRequestToAsset: customKeyModifier })

Right now the default fallback path for the serveSinglePageApp helper function is ${parsedUrl.origin}/index.html which works great for single page apps where there's only one entry to the site, that being index.html. However, frameworks such as NuxtJS generate multiple entry points, making the default fallback html page 200.html. It would just be nice to have a way to change the file that serveSinglePageApp is looking for.

My current handleEvent function is similar to:

let options = {
    mapRequestToAsset: (request) => {
      // First apply the default handler, which already has logic to detect
      // paths that should map to HTML files.
      request = mapRequestToAsset(request)

      const parsedUrl = new URL(request.url)

      // Detect if the default handler decided to map to
      // a HTML file in some specific directory.
      if (parsedUrl.pathname.endsWith('.html')) {
        // If expected HTML file was missing, just return the root index.html
        return new Request(`${parsedUrl.origin}/200.html`, request)
      } else {
        // The default handler decided this is not an HTML page. It's probably
        // an image, CSS, or JS file. Leave it as-is.
        return request
      }
    }
  }

  return await getAssetFromKV(event, options)

But since I'm copying the function just to change one line, what I want to do is something like:

let options = {
  mapRequestToAsset: serveSinglePageApp,
  fallback: '200.html'
}
  return await getAssetFromKV(event, options)

I'm personally not sure the best way to achieve this, but I think adding it to options and then passing it to the const requestKey = ASSET_MANIFEST[rawPathKey] ? request : options.mapRequestToAsset(request) line would work.

to better communicate the purpose of this api function.

Hi,

I'm writing a middleware for Cloudflare Workers that checks some conditions in the incoming request and rewrites the URL if those conditions are met.

I can construct a new Request with a modified URL, but I couldn't find a way to pass the modified URL to getAssetFromKV, because it takes a FetchEvent as argument and it's not possible to construct a new FetchEvent in workers or to mutate the FetchEvent.request property. So I thought a possible solution would be if I could pass a urlOverwrite option to getAssetFromKV in which case the function would use that URL instead of the event.request.url.

Happy to provide a PR for the urlOverwrite option or even more happier if you can suggest a better solution. :)

Best,
Agost

The current default mime type for files with unrecognized or missing extensions is text/plain, but the suggestion was made in #121 to change it to application/octet-stream. The MIME RFC states that the default should be text/plain, but the actual HTTP spec states that when the Content-Type header is missing, clients should treat it as application/octet-stream.

One consideration is that browsers don't attempt to render application/octet-stream; they download it to the filesystem instead.

🐛 Bug Report

Environment

• operating system: mac os
• output of rustc -V: rustc 1.26.2
• output of node -v: v10.16.3
• output of wrangler -V: 👷 ✨ wrangler 1.6.0

Steps to reproduce

I created a worker site with wrangler generate --site test-mp4 and replaced the wrangler ferris image with a looping mp4.

The looping mp4 plays correctly on all devices except for iOS. I think this might be due to the worker not handling ranged requests, and iOS requiring ranged requests for video playback (https://developer.apple.com/library/archive/documentation/AppleApplications/Reference/SafariWebContent/CreatingVideoforSafarioniPhone/CreatingVideoforSafarioniPhone.html#//apple_ref/doc/uid/TP40006514-SW6)

Try it out on your iPhone here -> https://test-mp4.tappable.workers.dev

What did you expect to see?

A video playing

What did you see instead?

A broken video indicator

For users wanting to serve single page applications with in-app routing (such as vue, react-router, etc), we should provide a handler that will treat a specific subset of paths as root, always serving the root index.html with the javascript application.

Some background:

Stack Overflow Q&A: https://stackoverflow.com/questions/58432345/cloudflare-workers-spa-with-vuejs/58439234

Cloudflare Community post (same user): https://community.cloudflare.com/t/cloudflare-workers-spa-with-vuejs/122864

Docs issue: cloudflare/workers-docs#439

0.0.8 has some important bug fixes, but on npm it only has TypeScript files. Can we get normal JavaScript files too?

Hello,

I am working on a Gatsby project that is being deployed to Cloudflare, Gatsby bundles page URL's like enterprise.html and contact-us.html to /enterprise.html/index.html and contact-us.html/index.html. When I deployed to Cloudflare I was receiving 404's as the worker was not finding the assets. I investigated further and found out that the reason is that the workers use the mapRequestToAsset function with the below cases:

  if (pathname.endsWith('/')) {
    // If path looks like a directory append index.html
    // e.g. If path is /about/ -> /about/index.html
    pathname = pathname.concat('index.html')
  } else if (!mime.getType(pathname)) {
    // If path doesn't look like valid content
    //  e.g. /about.me ->  /about.me/index.html
    pathname = pathname.concat('/index.html')
  }

The problem here is that the worker receives a request like example.com/enterprise.html which does not pass the } else if (!mime.getType(pathname)) { test case and it does not get the /index.html added so the worker cannot find the resource. Is there any way I can tell mapRequestToAsset to mandatory add the /index.html ?

I was thinking of something similar:

const mapRequestToAsset = (request: Request, forceAdd: string | undefined) => {
  const parsedUrl = new URL(request.url)
  let pathname = parsedUrl.pathname

  if (pathname.endsWith('/')) {
    // If path looks like a directory append index.html
    // e.g. If path is /about/ -> /about/index.html
    pathname = pathname.concat('index.html')
  } else if (!mime.getType(pathname)) {
    // If path doesn't look like valid content
    //  e.g. /about.me ->  /about.me/index.html
    pathname = pathname.concat('/index.html')
  } else if (forceAdd) {
    pathname = pathname.concat(forceAdd)
  }

  parsedUrl.pathname = pathname
  return new Request(parsedUrl.toString(), request)
}

I need to keep the .html pages and already asked Gatsby if they can change the way their bundle works but I don't think that is possible. I can create a PR if you are open to this idea or have any other suggestions.

I'm using the getAssetFromKV and serveSinglePageApp to serve my single page app and thats working well so far but I've noticed that all assets are being server via http/1.1. I was wondering if there was a way to serve the SPA and assets using http/2?

Browser cache gets set to edgeTTL on HITS

Today, cacheable, unchanged resources are not stored in the browser cache - regardless of the Cache-Control response headers that are applied in Workers. In other words, it's impossible to get a 304 Not Modified response (it is, however, possible to get Chrome to return the resource from memory cache).

There are two reasons for this:

1. Resources stored in Cache API do not include a Last-Modified or Etag response header
2. The cacheKey tied to each resource would strip the If-Modified-Since or If-None-Match header before querying the cache API (right now, the browser wouldn't even send these headers because of the first reason)

Solutions
Coming soon

In the source I see references to an asset manifest which comes from __STATIC_CONTENT_MANIFEST. It looks like a KV namespace but in I don't see such namespace in the KV dashboard.

I imagine that this manifest is created from the files in this config in the wrangler.toml:

[site]
bucket = "./public"

Can I create such manifest myself to map file requests to KV entries?

https://github.com/cloudflare/kv-asset-handler#cache-revalidation-and-etags

https://www.cdnplanet.com/ uses Workers Sites and no responses are served with an ETag.
My worker script is not setting bypassCache: true

Is the documentation indeed wrong and it's correct that Cloudflare does not serve ETag header on my site's responses?

TL;DR: I think this line should preserve whatever response status code cache.match() returned:

Originally reported by @willbicks here: cloudflare/wrangler-legacy#1746

For a fuller investigation, please read: https://community.cloudflare.com/t/pdfs-downloaded-from-cfworkers-wrangler-corrupted/238857/10

@shagamemnon I think you worked on this code previously -- would you mind taking a look? Is the fix as easy as I hope it is?

Test + add to tests what happens when bypassCache, browserTTL, or/and edgeTTL or set to null

Document the behavior in the readme

for both my gatsby sites, i needed to pass in a custom key modifier to allow URLs to work correctly. given a URL like /react-2019, by default, my site will link to /react-2019, which 404s with the default key modifier configuration. if i go to /react-2019/index.html, it redirects to /react-2019/, and (afaict, let me know if i'm wrong) chrome rewrites the url to /react-2019. i added a custom key config which isn't great (below) and while this works, i wonder if this is going to be something that many people will run into. maybe we should have stronger defaults and allow people to relax those rules via a custom key config?

const keyModifier = pathname => {
  if (pathname.endsWith('/')) {
    pathname = pathname.concat('index.html')
  }
  if (!pathname.endsWith('/') && !pathname.includes('.')) {
    pathname = pathname.concat('/index.html')
  }
  return pathname
}
return await getAssetFromKV(event, { keyModifier })

on this custom key modifier: the second conditional here is saying "if the path doesn't end with / and isn't some sort of asset (has a . in it), concat /index.html". it could be useful here (if possible) to have the content type or more information about the retrieved asset inside of this function. i think my logic could be cleaned up to say "if content type is HTML"...