feature: do not raise 500's on bad request

[rustatian]

Reason for This PR

closes: roadrunner-server/roadrunner#2353

Description of Changes

• Check errors from parsers, and if unable to parse, raise a special errors, indicating invalid input.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

• All commits in this PR are signed (git commit -s).
• The reason for this PR is clearly provided (issue no. or explanation).
• The description of changes is clear and encompassing.
• Any required documentation changes (code and docs) are included in this PR.
• Any user-facing changes are mentioned in CHANGELOG.md.
• All added/changed functionality is tested.

Summary by CodeRabbit

• Bug Fixes

  • Improved error response formatting with consistent HTTP status codes and headers.
  • Fixed oversized request handling to return HTTP 413 instead of generic server errors.
  • Simplified error messages in debug mode for better troubleshooting.

• Tests

  • Added comprehensive test coverage for error handling scenarios including oversized requests and malformed input.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a5c4718-2e07-43f3-adf2-ddc1b26c0a56

📥 Commits

Reviewing files that changed from the base of the PR and between 0bac2ba and ea54514.

📒 Files selected for processing (7)

• handler/handle_error_test.go
• handler/handler.go
• handler/request.go
• handler/status_error.go
• handler/status_error_test.go
• tests/handler_test.go
• tests/http_plugin_test.go

---

📝 Walkthrough

Walkthrough

This PR refactors HTTP request error handling by introducing an error classification system that wraps errors with explicit HTTP status codes, applies it to request body parsing, simplifies error response handlers, and validates behavior with new regression tests.

Changes

Request Error Classification and Response

Layer / File(s)	Summary
Error classification infrastructure handler/status_error.go, handler/status_error_test.go	Introduces statusError type and helper functions withStatus and classifyParseErr to wrap errors with explicit HTTP status codes and promote specific parse errors (http.MaxBytesError, multipart.ErrMessageTooLarge) to HTTP 413 status.
Parse error classification in request body handling handler/request.go	Updates populateBody to use classifyParseErr for both multipart and non-multipart request body parsing, routing size/overflow errors to HTTP 413 classification.
Simplified error handlers and response formatting handler/handler.go, handler/handle_error_test.go	Simplifies handleRequestErr to use a default 400 status overridden only for *statusError, removes complex error mapping logic, and updates handleError to use http.Error with plain-text or debug-mode error messages.
Integration and regression tests tests/handler_test.go, tests/http_plugin_test.go	Adds integration tests for oversized non-multipart requests (413), multipart requests with malformed query strings (400), and updates existing test assertions to match the new error message format.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A classifying tale of errors fine,
HTTP statuses now align,
Four-one-three for bodies too immense,
Four-hundred when query makes no sense,
Handlers freed from tangled ways,
Clear responses brighten API days! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: error handling now avoids returning 500s for bad requests (client errors), redirecting them to 400s instead.
Description check	✅ Passed	The PR description is mostly complete with the linked issue, description of changes, and license acceptance; however, the description of changes is somewhat minimal and could be more detailed about the implementation approach.
Linked Issues check	✅ Passed	The PR successfully implements the requirement from issue #2353: parser/parse errors now return HTTP 400 instead of 500, including specific handling for semicolon separator errors in multipart queries.
Out of Scope Changes check	✅ Passed	All changes are focused on refactoring error handling to properly classify and return appropriate HTTP status codes for parse/input errors, which directly aligns with the linked issue objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/400-when-needed

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR updates the HTTP request parsing/error-handling flow so malformed client input returns 4xx (not 5xx), while preserving 413 for request-size overflows and keeping true internal failures on the internal-error path.

Changes:

• Introduced a statusError wrapper + classifyParseErr helper to carry explicit HTTP status codes (notably 413) through error chains.
• Adjusted request parsing to classify ParseMultipartForm / io.ReadAll overflow errors as 413 and otherwise default parsing failures to 400.
• Updated and added tests to cover regression cases (oversize non-multipart body, malformed query in multipart) and the updated internal error response behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/http_plugin_test.go	Updates expected error body for oversize request handling.
tests/handler_test.go	Adds regression tests for 413 on oversize non-multipart bodies and 400 on malformed multipart query parsing.
handler/status_error.go	Adds statusError, withStatus, and classifyParseErr to preserve explicit status codes for parse errors.
handler/status_error_test.go	Unit tests for statusError wrapping and classifyParseErr promotion behavior.
handler/request.go	Routes multipart parse errors and raw-body read errors through classifyParseErr.
handler/handler.go	Makes request-parse errors default to 400 (unless explicitly wrapped) and updates internal error response writing behavior.
handler/handle_error_test.go	Verifies handleError writes a body and expected headers in debug and non-debug modes.

---

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