fix: redirect /demo to /demo/ to resolve 404 without trailing slash#842
fix: redirect /demo to /demo/ to resolve 404 without trailing slash#842SSDWGG wants to merge 1 commit into
Conversation
Accessing /demo without a trailing slash returned 404 because koa-static treats it as a file request rather than serving demo/index.html. Add a redirect from /demo to /demo/. Fixes #275
WalkthroughThe PR adds an HTTP GET route at 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e36fa79fa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // GET /demo — redirect to /demo/ so koa-static resolves demo/index.html | ||
| rootRouter.get('/demo', (ctx) => { | ||
| ctx.redirect('/demo/'); |
There was a problem hiding this comment.
Change the redirect target to keep the original query string; ctx.redirect('/demo/') rewrites /demo?... to /demo/ and drops parameters. This breaks demo deep links that rely on window.location.search (for example token and adminkey in public/demo/measurements.vue.js and public/demo/probes.vue.js), so users opening /demo?token=... no longer get the prefilled/authenticated state after this fix.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/http/server.ts (1)
65-65: ⚡ Quick winAdd type annotations for consistency.
The root route handler (line 54) includes type parameters
<object, CustomContext>, while this route omits them. Adding type annotations improves type safety and maintains consistency across route handlers.🔧 Proposed fix to add type annotations
- rootRouter.get('/demo', (ctx) => { + rootRouter.get<object, CustomContext>('/demo', (ctx) => { ctx.redirect('/demo/'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/http/server.ts` at line 65, The rootRouter.get('/demo', (ctx) => { ... }) route is missing the generic type annotations used elsewhere; update the handler to use the same type parameters (e.g., rootRouter.get<object, CustomContext>('/demo', (ctx) => { ... }) or annotate the ctx parameter as ctx: CustomContext) so it matches the other route signatures and preserves type safety (refer to rootRouter.get usage with <object, CustomContext> in the file).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/http/server.ts`:
- Around line 65-67: The redirect handler registered with
rootRouter.get('/demo', (ctx) => { ... }) currently uses ctx.redirect which
defaults to 302; change it to perform a permanent redirect by setting ctx.status
= 301 before calling ctx.redirect('/demo/') (or otherwise ensure the redirect
uses status 301) so the trailing-slash normalization is a permanent (HTTP 301)
redirect.
---
Nitpick comments:
In `@src/lib/http/server.ts`:
- Line 65: The rootRouter.get('/demo', (ctx) => { ... }) route is missing the
generic type annotations used elsewhere; update the handler to use the same type
parameters (e.g., rootRouter.get<object, CustomContext>('/demo', (ctx) => { ...
}) or annotate the ctx parameter as ctx: CustomContext) so it matches the other
route signatures and preserves type safety (refer to rootRouter.get usage with
<object, CustomContext> in the file).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b5d57f8-1053-46e4-b315-d5dddd32a5d5
📒 Files selected for processing (1)
src/lib/http/server.ts
| rootRouter.get('/demo', (ctx) => { | ||
| ctx.redirect('/demo/'); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
How to set HTTP 301 status code with ctx.redirect() in Koa?
💡 Result:
To set an HTTP 301 (Moved Permanently) status code when using ctx.redirect in Koa, you must explicitly assign the status code to ctx.status before calling the redirect method [1][2]. By default, ctx.redirect performs a 302 (Found) redirect [1][2]. To override this, set the status code manually: ctx.status = 301; ctx.redirect('/new-url'); You can perform this assignment either before or after calling ctx.redirect, as Koa allows you to alter the response status independently of the redirect method [1][2]. It is common practice to set the status first for clarity [3].
Citations:
- 1: https://github.com/koajs/koa/blob/master/docs/api/response.md
- 2: https://koajs.com/
- 3: https://akhromieiev.com/tutorials/handling-redirects-with-koa/
Use HTTP 301 for permanent trailing slash normalization.
The redirect currently uses Koa's default 302 status code (temporary redirect). For URL normalization, HTTP 301 (permanent redirect) is standard practice—it signals to search engines and browsers that this is permanent, enabling better SEO signals and more aggressive caching.
Set the status code explicitly before redirecting:
Proposed fix
rootRouter.get('/demo', (ctx) => {
+ ctx.status = 301;
ctx.redirect('/demo/');
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rootRouter.get('/demo', (ctx) => { | |
| ctx.redirect('/demo/'); | |
| }); | |
| rootRouter.get('/demo', (ctx) => { | |
| ctx.status = 301; | |
| ctx.redirect('/demo/'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/http/server.ts` around lines 65 - 67, The redirect handler registered
with rootRouter.get('/demo', (ctx) => { ... }) currently uses ctx.redirect which
defaults to 302; change it to perform a permanent redirect by setting ctx.status
= 301 before calling ctx.redirect('/demo/') (or otherwise ensure the redirect
uses status 301) so the trailing-slash normalization is a permanent (HTTP 301)
redirect.
1 participant
Summary
Fixes #275 — accessing the demo page at
/demo(without trailing slash) was returning 404 while/demo/worked correctly.Root cause
koa-staticservesdemo/index.htmlwhen accessing/demo/, but when/demois requested without the trailing slash, it's treated as a file lookup for a resource named "demo" rather than a directory, resulting in a 404.Change
Added a redirect on
rootRouterfrom/demoto/demo/so both URLs work for users.Type of Change
Checklist