Skip to content

invalid uuid check is returning a plain object instead of sending HTTP response #2734

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

Open
adityamane765 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

invalid uuid check is returning a plain object instead of sending HTTP response #2734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27799e6
invalid uuid check is returning a plain object instead of sending HTT…
adityamane765 Jun 13, 2026
3b23184
properly handle errors in config controller suggested by coderabbit
adityamane765 Jun 13, 2026
9abfffd
fix(app): error handler was ignoring AppError statusCode and always r…
adityamane765 Jun 13, 2026
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
4 changes: 2 additions & 2 deletions src/api/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ app.use('/', [yieldRoutes, config, median, perp, enriched, lsd, pools]);

function errorHandler (err, req, res, next) {
console.log(err)
res.status(500)
res.render('error', { error: err })
const statusCode = err.statusCode ?? 500
res.status(statusCode).json({ status: err.status ?? 'error', message: err.message })
Comment on lines +57 to +58

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid returning raw internal error messages for 5xx responses.

At Line 58, message: err.message will expose internal failure details on unexpected server errors. Prefer a generic message for statusCode >= 500, while keeping specific messages for 4xx AppError flows.

Suggested fix 
 function errorHandler (err, req, res, next) {
   console.log(err)
   const statusCode = err.statusCode ?? 500
-  res.status(statusCode).json({ status: err.status ?? 'error', message: err.message })
+  const message = statusCode >= 500 ? 'Internal server error' : err.message
+  res.status(statusCode).json({ status: err.status ?? 'error', message })
 }
🤖 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/api/app.js` around lines 57 - 58, The error handler currently returns
err.message for all responses which can leak internals for 5xx errors; change
the response so that after computing const statusCode = err.statusCode ?? 500
you use a conditional message (e.g., const message = statusCode >= 500 ?
'Internal server error' : err.message) and then call
res.status(statusCode).json({ status: err.status ?? 'error', message }); keep
the existing status field behavior and only substitute a generic message for
server errors.

}
Comment on lines 55 to 59

@coderabbitai coderabbitai Bot Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether error middleware already guards headersSent
rg -n -C3 'function errorHandler|headersSent|res\.status\(statusCode\)\.json' src/api/app.js

# Inspect error producers that can reach middleware
rg -n -C2 'throw new AppError|next\(err\)|Promise\.reject|throw ' src/api/controllers src/api/routes src/api

Repository: DefiLlama/yield-server

Length of output: 2117

🏁 Script executed:

#!/bin/bash
rg -n "class AppError|function AppError|statusCode|fail" src/api src/utils src -S
echo "----"
rg -n "res\.status\(400\)\.json|invalid uuid|UUID|uuid|Bad Request" src/api/controllers -S

Repository: DefiLlama/yield-server

Length of output: 50379

🏁 Script executed:

#!/bin/bash
rg -n "class AppError|function AppError|statusCode|fail|status\s*=" src/api src/utils src -S
echo "----"
rg -n "res\.status\(400\)\.json|invalid uuid|UUID|uuid|Bad Request" src/api/controllers -S

Repository: DefiLlama/yield-server

Length of output: 50379

Add a res.headersSent guard to the global Express error middleware

src/api/app.js’s errorHandler always calls res.status(...).json(...) without checking res.headersSent, so if a response has already started, Express can throw ERR_HTTP_HEADERS_SENT instead of delegating. Add an early if (res.headersSent) return next(err).

Suggested fix 
 function errorHandler (err, req, res, next) {
   console.log(err)
+  if (res.headersSent) {
+    return next(err)
+  }
   const statusCode = err.statusCode ?? 500
   res.status(statusCode).json({ status: err.status ?? 'error', message: err.message })
 }
🤖 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/api/app.js` around lines 55 - 59, The global Express error middleware
function errorHandler currently unconditionally writes a JSON response and can
cause ERR_HTTP_HEADERS_SENT if a response has already begun; add an early guard
in errorHandler that checks if res.headersSent and if so calls return next(err)
to delegate to the default/next error handler, otherwise continue to compute
statusCode and send the JSON response as before.


app.use(errorHandler)
Expand Down
8 changes: 4 additions & 4 deletions src/api/controllers/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const getDistinctID = async (req, res) => {
const response = await conn.query(query);

if (!response) {
return new AppError(`Couldn't get data`, 404);
throw new AppError(`Couldn't get data`, 404);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

res.status(200).json(response);
Expand All @@ -41,7 +41,7 @@ const getConfigPool = async (req, res) => {
const valid =
ids.map((id) => validator.isUUID(id)).reduce((a, b) => a + b, 0) ===
ids.length;
if (!valid) return { status: 'invalid uuid parameter' };
if (!valid) return res.status(400).json({ status: 'invalid uuid parameter' });

const query = `
SELECT
Expand All @@ -54,7 +54,7 @@ const getConfigPool = async (req, res) => {
const response = await conn.query(query, { configIDs: ids });

if (!response) {
return new AppError(`Couldn't get data`, 404);
throw new AppError(`Couldn't get data`, 404);
}

res.status(200).json({
Expand All @@ -79,7 +79,7 @@ const getAllPools = async (req, res) => {
const response = await conn.query(query);

if (!response) {
return new AppError(`Couldn't get data`, 404);
throw new AppError(`Couldn't get data`, 404);
}

res.status(200).json(response);
Expand Down
Loading