Skip to content

Add hello world route #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
motatoes wants to merge 1 commit into from
Closed

Add hello world route #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var logger = require('morgan');

var indexRouter = require('./routes/index');
var usersRouter = require('./routes/users');
var helloRouter = require('./routes/hello');

var app = express();

Expand All @@ -20,6 +21,7 @@ app.use(express.static(path.join(__dirname, 'public')));

app.use('/', indexRouter);
app.use('/users', usersRouter);
app.use('/hello', helloRouter);
Comment on lines 22 to +24

@codeant-ai codeant-ai Bot Jan 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Because the catch‑all index router is mounted on '/' and defines a router.all('*', ...) handler that sends a response without calling next(), registering the new /hello router after it means /hello requests will be handled by the index router and never reach the hello router, so the intended JSON endpoint will not work; mounting the hello router before the index router ensures /hello is actually served by the new handler. [logic error]

Severity Level: Critical 🚨 
- ❌ /hello returns HTML from routes/index.js instead of JSON.
- ❌ JSON hello endpoint unreachable by normal GET /hello requests.
- ⚠️ Breaks new API route defined in routes/hello.js.
- ⚠️ User-facing endpoint behavior differs from PR description.
Suggested change
app.use('/', indexRouter);
app.use('/users', usersRouter);
app.use('/hello', helloRouter);
app.use('/hello', helloRouter);
app.use('/', indexRouter);
app.use('/users', usersRouter);
Steps of Reproduction ✅ 
1. Start the Express app (app.js). The route mounting order is in app.js lines 22-24 where
indexRouter is mounted at '/' before the hello router.

2. Send a GET request to /hello (e.g., curl http://localhost:3000/hello). The request
first matches the indexRouter mounted at '/' (app.js:22).

3. The index router's handler at routes/index.js:5 is router.all('*', function(req, res,
next) { ... }); which executes for all paths and immediately calls res.render('index',
...) at routes/index.js:14-21 without calling next().

4. Because indexRouter's handler sends a response and does not call next(), the request is
completed by routes/index.js and never reaches routes/hello.js where router.get('/', ...)
is defined at routes/hello.js:5-7; therefore the JSON response ({ message: 'Hello World!'
}) is not returned.

5. Confirmed by reading routes/index.js (lines shown): router.all('*', ...) renders the
index view and terminates the request flow; hello.js's GET handler is never reached when
indexRouter is mounted first.
Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** app.js
**Line:** 22:24
**Comment:**
	*Logic Error: Because the catch‑all index router is mounted on `'/'` and defines a `router.all('*', ...)` handler that sends a response without calling `next()`, registering the new `/hello` router after it means `/hello` requests will be handled by the index router and never reach the hello router, so the intended JSON endpoint will not work; mounting the hello router before the index router ensures `/hello` is actually served by the new handler.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.


// catch 404 and forward to error handler
app.use(function(req, res, next) {
Expand Down
9 changes: 9 additions & 0 deletions routes/hello.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
var express = require('express');
var router = express.Router();

/* GET hello world. */
router.get('/', function(req, res, next) {
res.json({ message: 'Hello World!' });
});

module.exports = router;
Loading