kwhitley / itty-router

A little router.
MIT License
1.76k stars 78 forks source link

Nesting route cannot handle request for static assets #248

Closed wqcstrong closed 3 months ago

wqcstrong commented 4 months ago

Describe the Issue

I'm following the Worker Sites - Start from worker guide to intergate the KV namespace for static assets, after that my project looks like this:

// src/index.ts
import assetsRequest from './assests';

const router = AutoRouter();

router.all('/public/*', assetsRequest);

export default {
  ...router
}
// src/assets.ts

import { text } from 'itty-router';
import { getAssetFromKV, mapRequestToAsset, NotFoundError, MethodNotAllowedError } from '@cloudflare/kv-asset-handler';
import manifestJSON from '__STATIC_CONTENT_MANIFEST';

const assetManifest = JSON.parse(manifestJSON);

export defaut (request: Request, env: Env, ctx: any) => {
  try {
      return await getAssetFromKV(
          {
              request,
              waitUntil: ctx.waitUntil.bind(ctx),
          },
          {
              ASSET_NAMESPACE: env.__STATIC_CONTENT,
              ASSET_MANIFEST: assetManifest,
              mapRequestToAsset: () => {
                  const removePrefix = url.pathname.replace(/^\/public/, '');
                  const newPath = `${url.origin}${removePrefix}`;
                  return mapRequestToAsset(new Request(newPath, request));
              },
          }
      );
  } catch (e: any) {
      const { pathname } = new URL(request.url);
      if (e instanceof NotFoundError) {
          return text(`"${pathname}" not found`, {
              status: 404,
          });
      } else if (e instanceof MethodNotAllowedError) {
          return text(`Method not allowed`, {
              status: 400,
          });
      } else {
          return text(e.message, { status: 500 });
      }
  }
}
#:schema node_modules/wrangler/config-schema.json
name = "pagespy"
main = "src/index.ts"
compatibility_date = "2024-04-05"
compatibility_flags = ["nodejs_compat"]

[site]
bucket = "./public"

Above is all code, then I test in the local and I got the error:

image

Expected Behavior

The curl http://localhost:8787/public/img/logo.svg return a normal success response

Actual Behavior

It throw an error: Incorrect type for the 'fetcher' field on 'RequestInitializerDict': the provided value is not of type 'Fetcher'.

Environment (please complete the following information):

C1ARKGABLE commented 3 months ago

I'm having the same problem as well

wqcstrong commented 3 months ago

@kwhitley Any advice to help to fix the issue?

kwhitley commented 3 months ago

Hey guys - I'm looking into this, but that's def not an itty error (which emits no useful errors because those cost bytes). I'm guessing CF introduced some sort of Fetcher interface check, so I just need to figure what the signature is, and where this code doesn't align :)

C1ARKGABLE commented 3 months ago

@kwhitley Thanks for looking into this! I found that Cloudflare's itty-router-openapi supports their weird fetcher problem.

@wqcstrong In the short term, I've switched to using Cloudflare's itty-router-openapi. It looks like:

import { OpenAPIRouter } from '@cloudflare/itty-router-openapi'
import { getAssetFromKV } from '@cloudflare/kv-asset-handler';
import manifestJSON from '__STATIC_CONTENT_MANIFEST';
const assetManifest = JSON.parse(manifestJSON);

const router = OpenAPIRouter()
router.get("*", serverStaticAsset)

export default {
    fetch: router.handle,
}

async function serverStaticAsset(request, env, ctx) {
    try {
        return await getAssetFromKV({
            request,
            waitUntil: ctx.waitUntil.bind(ctx),
        }, {
            ASSET_NAMESPACE: env.__STATIC_CONTENT,
            ASSET_MANIFEST: assetManifest,
        });
    } catch (e) {
        return new Response("Resource not found", {
            status: 404,
            headers: { 'Content-Type': 'text/html' },
        })
    }
}
wqcstrong commented 3 months ago

@C1ARKGABLE Thanks for your advice, now my solution like this:

export defaut {
    async fetch(request: Request, env: Env, ctx: any) {
        const url = new URL(request.url);
        if (url.pathname.startsWith('/public')) {
            try {
                return await getAssetFromKV(
                    {
                        request,
                        waitUntil: ctx.waitUntil.bind(ctx),
                    },
                    {
                        ASSET_NAMESPACE: env.__STATIC_CONTENT,
                        ASSET_MANIFEST: assetManifest,
                        mapRequestToAsset: () => {
                            const removePrefix = url.pathname.replace(/^\/public/, '');
                            const newPath = `${url.origin}${removePrefix}`;
                            return mapRequestToAsset(new Request(newPath, request));
                        },
                    }
                );
            } catch (e: any) {
                const { pathname } = new URL(request.url);
                if (e instanceof NotFoundError) {
                    return text(`"${pathname}" not found`, {
                        status: 404,
                    });
                } else if (e instanceof MethodNotAllowedError) {
                    return text(`Method not allowed`, {
                        status: 400,
                    });
                } else {
                    return text(`内部异常: ${e.message}`, { status: 500 });
                }
            }
        } else {
            return router.fetch(request, env, ctx);
        }
    },
}
realByg commented 3 months ago

cloudflare changed the request.fecth property after 2024-04-03, so if you set the compatibility_date in wrangler.toml prior to that date, it works fine. the work around is to just create a new request object to pass to to getAssetFromKV

return await getAssetFromKV(
        {
            request: new Request(request.url, { ...request }),
            waitUntil: ctx.waitUntil.bind(ctx),
        },
        {
            ASSET_NAMESPACE: env.__STATIC_CONTENT,
            ASSET_MANIFEST: assetManifest,
        },
    )
realByg commented 3 months ago

As for the nested route problem, you just have to replace the path

const router = AutoRouter()

const kvAssetsMiddleware = (basePath: string) => {
    const middleware: RequestHandler = async (request, env, ctx) => {
        const url = new URL(request.url)
        url.pathname = url.pathname.replace(new RegExp(String.raw`^${basePath}`), '')
        return await getAssetFromKV(
            {
                request: new Request(url, { ...request }),
                waitUntil: ctx.waitUntil.bind(ctx),
            },
            {
                ASSET_NAMESPACE: env.__STATIC_CONTENT,
                ASSET_MANIFEST: assetManifest,
            },
        )
    }
    return middleware
}

router.all('*', kvAssetsMiddleware('/dist'))