Skip to content

refactor: catch body-parser JSON syntax errors and improve global error logs #113

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
KGFCH2 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: catch body-parser JSON syntax errors and improve global error logs #113

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
13 changes: 12 additions & 1 deletion server/middlewares/errorHandler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,19 @@ const errorHandler = (err, req, res, next) => {
message: message
};

// Log 500 internal errors to the server console for debuggability
if (!err.statusCode || err.statusCode === 500) {
console.error(`[Error] Internal Server Error on ${req.method} ${req.originalUrl}:`, err);
}
Comment on lines +11 to +14

@coderabbitai coderabbitai Bot May 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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Logging logic incorrectly flags client errors as internal server errors.

The condition !err.statusCode will match errors that don't have a statusCode property, including the newly handled SyntaxError (which has err.status), Mongoose errors, and JWT errors. These will be logged as "Internal Server Error" even though they ultimately return 400/401/409 status codes.

Move the logging after the error-type determination so only genuine 500-level errors are logged.

🔧 Proposed fix 
  let errorResponse = {
    success: false,
    message: message
  };

- // Log 500 internal errors to the server console for debuggability
- if (!err.statusCode || err.statusCode === 500) {
-   console.error(`[Error] Internal Server Error on ${req.method} ${req.originalUrl}:`, err);
- }
-
  // Handle express malformed JSON parsing error
  if (err instanceof SyntaxError && err.status === 400 && 'body' in err) {
    statusCode = 400;
    message = "Invalid JSON syntax in request body.";
    errorResponse.message = message;
  }
  // Handle ApiError
  else if (err instanceof ApiError) {
    statusCode = err.statusCode;
    message = err.message;
    errorResponse.message = message;
  }
  // Handle Mongoose ValidationError
  else if (err.name === "ValidationError") {
    statusCode = 400;
    const messages = Object.values(err.errors).map(val => val.message);
    message = `Validation Error: ${messages.join(", ")}`;
    errorResponse.message = message;
  }
  // Handle Mongoose CastError (invalid ObjectId)
  else if (err.name === "CastError") {
    statusCode = 400;
    message = `Invalid ${err.path}: ${err.value}`;
    errorResponse.message = message;
  }
  // Handle Mongoose duplicate key error
  else if (err.code === 11000) {
    statusCode = 409;
    const field = Object.keys(err.keyValue)[0];
    message = `${field} already exists.`;
    errorResponse.message = message;
  }
  // Handle JWT errors
  else if (err.name === "JsonWebTokenError") {
    statusCode = 401;
    message = "Invalid token.";
    errorResponse.message = message;
  }
  else if (err.name === "TokenExpiredError") {
    statusCode = 401;
    message = "Token expired.";
    errorResponse.message = message;
  }
  // Handle other errors
  else if (err.message) {
    message = err.message;
    errorResponse.message = message;
  }

+ // Log 500 internal errors to the server console for debuggability
+ if (statusCode === 500) {
+   console.error(`[Error] Internal Server Error on ${req.method} ${req.originalUrl}:`, err);
+ }
+
  // Include stack trace in development mode
🤖 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 `@server/middlewares/errorHandler.js` around lines 11 - 14, The current logging
block using if (!err.statusCode || err.statusCode === 500) wrongly treats errors
without statusCode as 500; move the console.error call so it runs only after you
determine the effective response status for the error (i.e., after the
error-type checks that map SyntaxError, Mongoose, JWT, etc. to 400/401/409), and
log only when the resolved status is 500; update references to err, req.method
and req.originalUrl (and the errorHandler function) so the message still
includes the original request context and the full error object for genuine
internal server errors.


// Handle express malformed JSON parsing error
if (err instanceof SyntaxError && err.status === 400 && 'body' in err) {
statusCode = 400;
message = "Invalid JSON syntax in request body.";
errorResponse.message = message;
}
// Handle ApiError
if (err instanceof ApiError) {
else if (err instanceof ApiError) {
statusCode = err.statusCode;
message = err.message;
errorResponse.message = message;
Expand Down
Loading