feat: add catcher feature to custom status handler (#31) #33
Conversation
WalkthroughThis update introduces a status-based error handling mechanism to the server framework. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HttpServer
participant Catcher
participant PythonHandler
Client->>HttpServer: Send HTTP request
HttpServer->>HttpServer: Process request, generate response
alt Response status matches registered catcher
HttpServer->>Catcher: Invoke catcher for status
Catcher->>PythonHandler: Call Python handler with request & response
PythonHandler-->>Catcher: Return modified response
Catcher-->>HttpServer: Provide new response
end
HttpServer->>Client: Send (possibly modified) response
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
examples/api/main.py (1)
127-130: Consider status code consistency in error catcherThe catcher correctly intercepts internal server errors, but returns a BAD_REQUEST (400) status instead of maintaining the original INTERNAL_SERVER_ERROR (500). While this might be intentional, consider whether this status conversion is desired UX. Usually, it's better to maintain the same status code category while providing a more user-friendly error message.
@catcher(Status.INTERNAL_SERVER_ERROR) def catch_500(req: Request, res: Response): - return {"error": res.body}, Status.BAD_REQUEST + return {"error": res.body}, Status.INTERNAL_SERVER_ERRORsrc/handling/response_handler.rs (2)
41-49: Surface catcher errors instead of silently ignoring themIf the Python catcher raises or returns an invalid object, the error is swallowed and the original response is returned. This makes debugging extremely hard.
Consider:
- Logging the Python exception via
e.print(py)or your logging façade.- Returning a
500built fromStatus::INTERNAL_SERVER_ERRORso that clients and tests can detect the failure path.Even a low‑verbosity log is better than complete silence.
53-54: Check the result ofresponse_sender.send
_ = sender.send(..)discards a potentialErr, masking back‑pressure or channel‑closure problems.
Return or log the error so that upstream can react.-let _ = process_request.response_sender.send(response).await; +if let Err(e) = process_request.response_sender.send(response).await { + eprintln!("Failed to forward response: {e}"); +}src/catcher.rs (1)
1-36: Consider adding documentation for Python users.While the implementation is solid, adding Python-facing docstrings would improve usability for Python developers consuming this API. PyO3 supports exposing Rust doc comments as Python docstrings.
#[derive(Clone)] #[pyclass] +/// A catcher for HTTP responses with specific status codes. +/// +/// Use this class to register custom handlers for specific HTTP status codes. pub struct Catcher { pub status: Status, pub handler: Arc<Py<PyAny>>, } #[pymethods] impl Catcher { #[new] + /// Create a new catcher for the given status code. pub fn new(status: PyRef<'_, Status>, py: Python<'_>) -> Self { Self { status: status.clone(), handler: Arc::new(py.None()), } } + /// Register a handler function for this catcher. + /// + /// The handler should accept a response object and return a modified response. fn __call__(&self, handler: Py<PyAny>) -> PyResult<Self> { Ok(Self { handler: Arc::new(handler), ..self.clone() }) } } #[pyfunction] +/// Create a new catcher for the given status code. +/// +/// This is a decorator function that can be used to register +/// a handler for a specific status code. pub fn catcher(status: PyRef<'_, Status>, py: Python<'_>) -> Catcher { Catcher::new(status, py) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/api/main.py(5 hunks)src/catcher.rs(1 hunks)src/handling/request_handler.rs(3 hunks)src/handling/response_handler.rs(2 hunks)src/lib.rs(12 hunks)src/routing.rs(1 hunks)src/serializer/mod.rs(1 hunks)src/status.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib.rs (1)
src/catcher.rs (2)
catcher(33-35)new(17-22)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: macos (macos-14, aarch64)
- GitHub Check: macos (macos-13, x86_64)
- GitHub Check: linux (ubuntu-22.04, x86)
- GitHub Check: linux (ubuntu-22.04, aarch64)
- GitHub Check: linux (ubuntu-22.04, s390x)
- GitHub Check: musllinux (ubuntu-22.04, armv7)
- GitHub Check: linux (ubuntu-22.04, x86_64)
- GitHub Check: windows (windows-latest, x86)
- GitHub Check: linux (ubuntu-22.04, armv7)
- GitHub Check: windows (windows-latest, x64)
- GitHub Check: musllinux (ubuntu-22.04, aarch64)
- GitHub Check: musllinux (ubuntu-22.04, x86_64)
- GitHub Check: linux (ubuntu-22.04, ppc64le)
- GitHub Check: musllinux (ubuntu-22.04, x86)
🔇 Additional comments (12)
src/status.rs (1)
8-8: Improved trait derivations allow Status to be used as HashMap keyAdding
PartialEq,Eq,Hash, andDebugderivations is essential for the new catcher feature, as it enablesStatusinstances to be used as keys in the status catcher HashMap.src/routing.rs (1)
21-21: LGTM: Good refactoring to use SelfUsing
Selfinstead of the explicit struct name improves code maintainability. If the struct name changes in the future, this code won't need updating.examples/api/main.py (3)
19-19: LGTM: Added catcher importThe import is properly added to support the new error handling functionality.
66-66: Removed try-except blocks in favor of global error handlingThe validation exception handling is now delegated to the global catcher instead of being handled with try-except blocks in each handler, which is a cleaner approach.
Also applies to: 86-86
144-144: LGTM: Registered catcher with server instanceThe catcher is properly registered with the server using the new API.
src/serializer/mod.rs (1)
145-145: Code simplification improves readabilityThe refactoring removes redundant
Ok()wrappers and unnecessary.into()calls, making the code more concise while maintaining the same functionality.Also applies to: 153-153, 156-156
src/handling/request_handler.rs (1)
23-33: Thread‑safety ofstatus_catchermust be guaranteed
status_catcheris captured by thetokio::spawnbelow.
That future must beSend, which in turn requires
Arc<HashMap<Status, Py<PyAny>>>: Send + Sync.Double‑check that
Py<PyAny>indeed implements bothSendandSync
for yourpyo3version; otherwise this will fail to compile (or, worse, compile
withunsafeguarantees violated).
IfSyncis missing, wrap the map in anRwLockand store the handlers as
GILOnceCell<Py<PyAny>>, or use adashmapguarded by the GIL.This issue has bitten the router handlers before, so please verify.
src/catcher.rs (5)
1-5: Imports look appropriate.The imports correctly include
Arcfor thread-safe reference counting, PyO3 for Python integration, and the internalStatustype. This provides the foundation needed for the rest of the implementation.
7-12: Good struct design with appropriate attribute derivation.The
Catcherstruct correctly derivesCloneand is properly marked with#[pyclass]for Python exposure. UsingArc<Py<PyAny>>for the handler ensures thread-safety when sharing Python callables across async contexts.
14-22: Constructor initialization looks good.The constructor appropriately takes a Python reference to a
Statusobject and initializes the handler to Python'sNone. This matches the pattern where the actual handler would be set later via the__call__method.
24-29: Elegant implementation of the callable pattern.The
__call__method creates a newCatcherwith the provided handler while preserving the original status. Using struct update syntax (..self.clone()) is a clean approach. The return type correctly usesPyResultto handle potential Python errors.
32-35: Good Python function export.The standalone
catcherfunction provides a convenient way to create a newCatcherinstance from Python. This follows the common PyO3 pattern of providing a function wrapper around a struct constructor for better Python ergonomics.
j03-dev
force-pushed
the
feat/catcher-31
branch
from
1e5c5a8 to
f40b06b
Compare
Summary by CodeRabbit