fix: bubble up errors in rewrites #11136
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 2d9aefd 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 |
github-actions
bot
added
pkg: astro
ematipico
force-pushed
the
fix/bubble-up-error
branch
2 times, most recently
from
9f2729d to
2803dfe
Compare
ematipico
force-pushed
the
fix/bubble-up-error
branch
from
2803dfe to
4a8698a
Compare
matthewp
approved these changes
Jun 5, 2024
ematipico
force-pushed
the
fix/bubble-up-error
branch
from
4a8698a to
1d22289
Compare
ematipico
force-pushed
the
fix/bubble-up-error
branch
from
1d22289 to
2d9aefd
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Closes #11110
Two important changes here:
RenderContext, I removed thetry/catchinside the rewrite method. Now runtime errors emitted by that function are bubbled up and shown to the userAstro.rewrite("/404")inside static pages. It doesn't make sense to emit HTML/static pages that contain the HTML of a 404 without returning aResponsewith status code404. However, this is still a valid behaviour in SSR because we're able to return aResponsewith the correct status code. In order to align this behaviour, I added a new abstract method inside the base pipeline. In dev, we allow this behaviour only if the configuration is in SSR, so the user receives a similar DX to SSR.Testing
I updated some existing test cases. I created a new fixture to make sure that building a SSG project, and using
Astro.rewrite("/404")throws an error.Docs
N/A