Skip to content

Fix redirects being ignored when landing page is enabled#11

Merged
vpetersson merged 4 commits intomasterfrom
fix-404
Jan 19, 2026
Merged

Fix redirects being ignored when landing page is enabled#11
vpetersson merged 4 commits intomasterfrom
fix-404

Conversation

@vpetersson
Copy link
Owner

Previously, ServeDir handled requests first and used the redirect router as a fallback. This caused redirects like /gh to return 404 because ServeDir's fallback wasn't triggering correctly.

The fix inverts the lookup order: check redirect rules first, then fall back to static file serving. This ensures redirects take priority while still serving static assets (CSS, fonts, etc.) for the landing page.

Adds integration test covering redirects with static_dir enabled.

Previously, ServeDir handled requests first and used the redirect router
as a fallback. This caused redirects like /gh to return 404 because
ServeDir's fallback wasn't triggering correctly.

The fix inverts the lookup order: check redirect rules first, then fall
back to static file serving. This ensures redirects take priority while
still serving static assets (CSS, fonts, etc.) for the landing page.

Adds integration test covering redirects with static_dir enabled.
@vpetersson vpetersson requested a review from Copilot January 19, 2026 10:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an issue where redirect rules were being ignored when the landing page (static directory serving) was enabled. Previously, ServeDir handled requests first and used redirects as a fallback, but this caused redirects to return 404. The fix inverts the lookup order to check redirect rules before serving static files.

Changes:

  • Reordered request handling to check redirect rules before static file serving
  • Added ServiceExt import to support the new service composition pattern
  • Added comprehensive integration test to verify redirects work correctly alongside static file serving

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Apply Copilot's suggestion to use proper error handling when the static
file service fails, returning 500 instead of crashing.
@vpetersson vpetersson requested a review from Copilot January 19, 2026 10:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Log static file serving errors before returning 500
- Extract redirect matching logic into helper closure
@vpetersson vpetersson requested a review from Copilot January 19, 2026 10:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Move redirect_if_matches closure outside async block for better performance
- Include request path in error message for easier debugging
@vpetersson vpetersson requested a review from Copilot January 19, 2026 10:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vpetersson vpetersson merged commit 6a2439d into master Jan 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant