fresh icon indicating copy to clipboard operation
fresh copied to clipboard

[[email protected]] _error.tsx and _404.tsx bypass middlewares defined in _middleware.tsx

Open dahlia opened this issue 9 months ago • 5 comments

I'm using Fresh v2.0.0-alpha.29, and when a request is handled by _error.tsx and _404.tsx, middlewares defined in _middleware.tsx seem ignored.

Personal context

I use middlewares in order to initialize i18n stuffs, and I want to localize messages in 404/500 error pages, but currently they are not localized… So IMHO this is an unexpected behavior!

dahlia avatar Mar 14 '25 03:03 dahlia

Same as #2573?

frou avatar Mar 14 '25 09:03 frou

I just encountered the same problem. I'm not certain about _500, but in Fresh v1 at least the _404 page would run _middleware before. I needed similar things, parsed cookies in ctx.state, and some other state to correctly render the _app shell for the _error page.

csvn avatar May 15 '25 08:05 csvn

A workaround I found for _middleware that does not do anything with the Response I found is to call _middleware manually in _error. This way if there's any state set from the incoming Request, at least ctx.state can be correctly updated.

//_error.tsx
import { handler as _middleware } from './_middleware.ts';

export const handler = define.handlers((ctx) => {
  // Any code after `const res = await ctx.next();` won't work correctly in `_middleware` (it will have no effect)
  _middleware({ ...ctx, next: () => Promise.resolve(new Response()) });

  return page();
});

export default define.page(function ErrorPage(ctx) {
  // Error page now has populated `ctx.state`
}

csvn avatar May 15 '25 09:05 csvn

I need help on finding a reproduction for this issue. I tried a few things but I'm not able to reproduce it. The _middleware is included for me.

Example, this test passes:

  const server = await createServer<{ value: string }>({
    "routes/foo.tsx": {
      handlers: () => {
        console.log("/foo");
        throw new Error("fail");
      },
    },
    "routes/_middleware.tsx": {
      handlers: async (ctx) => {
        console.log("/middleware");
        ctx.state.value = "ok";
        const res = await ctx.next();
        console.log("after /middleware");
        return res;
      },
    },
    "routes/_error.tsx": {
      handlers: () => {
        console.log("/_error");
        return page();
      },
      default: (ctx) => {
        return <h1>{ctx.state.value}</h1>;
      },
    },
  });

  const res = await server.post("/foo");
  expect(await res.text()).toContain("ok");

Output:

/middleware
/foo
/_error
after /middleware

marvinhagemeister avatar May 15 '25 20:05 marvinhagemeister

I need help on finding a reproduction for this issue. I tried a few things but I'm not able to reproduce it. The _middleware is included for me.

I've created a repo with reproduction: https://github.com/csvn/fresh-v2-404-middleware-repro

Did not manage to boil it down to a test-case yet. But when I tried it seems only 404 is a problem for me right now (at least since I changed from _404 + _500 to _error).

Example

This is the result from three different kinds of pages in my reproduction:

index

middleware/app/layout/index

err

middleware/app/layout/500

foo (missing)

app/layout/404

csvn avatar May 16 '25 19:05 csvn

I'm not sure how best to solve this. On the one hand you want middlewares to run for error and 404 pages. But what happens when one of the middlewares throw? If we call the error page with middlewares again we have an infinite loop.

marvinhagemeister avatar Jul 10 '25 07:07 marvinhagemeister

Whilst working on https://github.com/denoland/fresh/pull/3086 I've took another look at this. Effectively, there can't be any guarantees that middleware's have been run when an error occurs. This is because middlewares themselves can throw. Whenever possible we'll try to include the middlewares but again, there is no guarantee that this is the case when a middleware throws.

Marking as resolved as there are no further steps to take.

marvinhagemeister avatar Jul 14 '25 17:07 marvinhagemeister