fix(rerouting): attempt without middleware

[lilnasy]

Changes

• Fixes 500.astro content is not showing when throwing an error from a middleware #8797

Testing

Added a case to middleware.test.js.

Docs

Does not affect usage.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4242c68

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[natemoo-re]

Makes sense! Thanks for the fix

[ematipico]

I believe this change should be part of the App pipeline because it's in charge of running the middleware, so we should only expose the functions to or not run it.

Also, App is used by other adapters, so we should bump all those adapters that use App:

• node
• vercel
• cloudflare
• netlify

[lilnasy]

I was under the impression that the changes made here were all to internal APIs. I'll take another look later.

[ematipico]

I was under the impression that the changes made here were all to internal APIs. I'll take another look later.

Unfortunately no. https://docs.astro.build/en/reference/adapter-reference/#astroapp

Search for import { App } from 'astro/app';, and you'll find that most of our adapters will inherit this fix.

[lilnasy]

but I have made no changes to the public interface of app

[ematipico]

but I have made no changes to the public interface of app

True, although I think that all the consumers of this fix will benefit from it, so we should bump a patch for them too.

[lilnasy]

In its current state, the fix would apply to adapters and all their versions retroactively; a bump souldn't be necessary. But maybe I am misunderstanding.

Is the idea is to allow the adapter to say whether middleware should run?

app.renderWithOptions(request, { runMiddleware: false })

It would expand the scope of this PR from a bugfix to a feature.

FWIW, it is already planned for the reroute API, and you can see the implementation in the exploration branch.

[ematipico]

In its current state, the fix would apply to adapters and all their versions retroactively; a bump souldn't be necessary. But maybe I am misunderstanding.

Yeah you're right, I misunderstood things. Sorry for bikeshedding this. A patch for core should be enough.

[lilnasy]

No worries. I'll get to the suggestions in a few hours.

[ematipico]

Looks good. Left some feedback and question.