Classify serde_json errors for AppError mapping#12
Conversation
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
masterror/src/convert/serde_json.rs
Lines 47 to 52 in e0ddfe5
[P1] Avoid misclassifying deserialize I/O failures as serialization errors
The new From<serde_json::Error> implementation maps every Category::Io to AppError::serialization, but serde_json::Error::classify() does not encode whether the error happened while encoding or decoding. When serde_json::from_reader fails because the underlying Read returns an io::Error (e.g., a client disconnects mid‑request), err.classify() is also Io, so this code will report a serialization error even though the failure occurred during deserialization. Because Category::Io (and Category::Data) is used for both encode and decode paths, relying on it alone yields incorrect error kinds and misleads callers that inspect AppErrorKind. Consider preserving the previous Internal mapping or letting the caller decide the context instead of forcing all I/O errors into Serialization.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Summary
serde_json::error::CategoryTesting
cargo build --all-targets --features serde_jsoncargo clippy --all-targets --features serde_json -- -D warningscargo test --all --features serde_jsoncargo doc --no-deps --features serde_jsonhttps://chatgpt.com/codex/tasks/task_e_68c2718b6d90832bb86efea56cbf1573