Repository navigation
fix: Answer unhandled REST requests with a not found error - #49
Conversation
- The REST not handled error now carries a 404 status, as its MVC sibling does - Unmatched requests no longer answer with a 500 nor get reported to Sentry - Tests cover the status code and the routing of unmatched requests Fixes OMNI-LDJ-14
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches📝 Generate docstrings
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused, backward-compatible change matches established MVC behavior and has appropriate regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Adds HTTP 404 handling for unmatched REST routes, preventing false server errors and Sentry reports.
Changes:
- Adds a default/custom status code to
RESTRequestNotHandled. - Tests unmatched routing and exception status codes.
- Documents the fix.
| File | Description |
|---|---|
rest/src/rest/exceptions.py |
Adds the default 404 status code. |
rest/src/rest/test.py |
Covers unmatched routes and status codes. |
CHANGELOG.md |
Records the corrected behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Fixes OMNI-LDJ-14.
Cause
A request whose path is matched by no route of the REST service plugins raises
RESTRequestNotHandled(rest/src/rest/system.py), which, unlike its siblingsMVCRequestNotHandledand theRequestNotHandledof the colony HTTP handler, carries nostatus_code. The WSGI handler falls back to 500 for exceptions without one, so the client receives an internal error, the exception is logged with error verbosity and it reaches Sentry through the logging handler, as the client error filter of #45 only recognises the exceptions that carry a 4XX status code.The Sentry event has no request context (URL, method) because
request.beginis only notified once a route matches, which never happens for these requests.Changes
RESTRequestNotHandledtakes astatus_codedefaulting to 404, mirroringMVCRequestNotHandled, so the WSGI handler answers with a not found error, logs it with debug verbosity and Sentry ignores it as a client errorVerification
test_handle_request_not_handledroutes a request with no REST service plugin and with one whose routes do not match, asserting the error and its 404 status code, then registers a matching route to confirm that the lack of it was the causetest_rest_request_not_handledasserts the default status code andtest_rest_request_not_handled_custom_statusa provided one'RESTRequestNotHandled' object has no attribute 'status_code') and pass with the changeblack --checkclean