Implement InvalidRequestError with field-level validation details#26
Merged
codebestia merged 2 commits intoJun 28, 2026
Conversation
Add param and field_errors parsing from API error responses and raise InvalidRequestError on HTTP 400/422 so developers can identify bad params.
Contributor
|
Hello @samueloyibodevv |
Contributor
📝 WalkthroughWalkthrough
InvalidRequestError structured validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
codebestia
approved these changes
Jun 28, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/shade/errors.py (1)
176-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the shadow
_parse_bodyredefinition.This copy silently replaces the earlier
_parse_bodyat Lines 153-160. Keeping both definitions makes future edits easy to miss and looks like leftover conflict resolution.🤖 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 `@src/shade/errors.py` around lines 176 - 183, Remove the duplicate _parse_body definition in the same module so only the original helper remains; the redefined copy near the later _parse_body function is shadowing the earlier implementation and should be deleted. Keep the canonical _parse_body used by the surrounding error-handling code in src/shade/errors.py, and make sure any callers continue to rely on that single definition.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/shade/errors.py`:
- Around line 186-205: The `_parse_error_response` helper is discarding
string-valued `error` payloads, so invalid request responses like
`{"error":"missing amount"}` lose the server message. Update
`_parse_error_response` to preserve `error` when it is a string (while still
handling dict payloads for `message`, `param`, and `field_errors`), and make
sure the returned `message` falls back to that string before using the generic
invalid-request text.
---
Nitpick comments:
In `@src/shade/errors.py`:
- Around line 176-183: Remove the duplicate _parse_body definition in the same
module so only the original helper remains; the redefined copy near the later
_parse_body function is shadowing the earlier implementation and should be
deleted. Keep the canonical _parse_body used by the surrounding error-handling
code in src/shade/errors.py, and make sure any callers continue to rely on that
single definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 897abbf3-b31f-44da-adc0-2ee2c880a2ca
📒 Files selected for processing (2)
src/shade/errors.pytests/test_errors.py
Comment on lines
+186
to
+205
| def _parse_error_response(response_body: Optional[str]) -> dict[str, Any]: | ||
| data = _parse_body(response_body) | ||
| error = data.get("error", {}) | ||
| if not isinstance(error, dict): | ||
| error = {} | ||
|
|
||
| field_errors = error.get("field_errors") | ||
| if not isinstance(field_errors, dict): | ||
| field_errors = data.get("field_errors") | ||
| if not isinstance(field_errors, dict): | ||
| field_errors = {} | ||
|
|
||
| message = error.get("message") | ||
| if not message: | ||
| message = data.get("message") | ||
|
|
||
| return { | ||
| "message": message, | ||
| "param": error.get("param"), | ||
| "field_errors": field_errors, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve string-valued error messages when parsing invalid requests.
Line 188 drops any non-dict error payload, so a 400/422 body like {"error":"missing amount"} now degrades to the generic "Invalid request" instead of surfacing the server message.
Suggested fix
def _parse_error_response(response_body: Optional[str]) -> dict[str, Any]:
data = _parse_body(response_body)
- error = data.get("error", {})
- if not isinstance(error, dict):
- error = {}
+ raw_error = data.get("error", {})
+ if isinstance(raw_error, dict):
+ error = raw_error
+ else:
+ error = {}
field_errors = error.get("field_errors")
if not isinstance(field_errors, dict):
field_errors = data.get("field_errors")
if not isinstance(field_errors, dict):
field_errors = {}
message = error.get("message")
+ if not message and isinstance(raw_error, str):
+ message = raw_error
if not message:
message = data.get("message")
return {
"message": message,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _parse_error_response(response_body: Optional[str]) -> dict[str, Any]: | |
| data = _parse_body(response_body) | |
| error = data.get("error", {}) | |
| if not isinstance(error, dict): | |
| error = {} | |
| field_errors = error.get("field_errors") | |
| if not isinstance(field_errors, dict): | |
| field_errors = data.get("field_errors") | |
| if not isinstance(field_errors, dict): | |
| field_errors = {} | |
| message = error.get("message") | |
| if not message: | |
| message = data.get("message") | |
| return { | |
| "message": message, | |
| "param": error.get("param"), | |
| "field_errors": field_errors, | |
| def _parse_error_response(response_body: Optional[str]) -> dict[str, Any]: | |
| data = _parse_body(response_body) | |
| raw_error = data.get("error", {}) | |
| if isinstance(raw_error, dict): | |
| error = raw_error | |
| else: | |
| error = {} | |
| field_errors = error.get("field_errors") | |
| if not isinstance(field_errors, dict): | |
| field_errors = data.get("field_errors") | |
| if not isinstance(field_errors, dict): | |
| field_errors = {} | |
| message = error.get("message") | |
| if not message and isinstance(raw_error, str): | |
| message = raw_error | |
| if not message: | |
| message = data.get("message") | |
| return { | |
| "message": message, | |
| "param": error.get("param"), | |
| "field_errors": field_errors, |
🤖 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 `@src/shade/errors.py` around lines 186 - 205, The `_parse_error_response`
helper is discarding string-valued `error` payloads, so invalid request
responses like `{"error":"missing amount"}` lose the server message. Update
`_parse_error_response` to preserve `error` when it is a string (while still
handling dict payloads for `message`, `param`, and `field_errors`), and make
sure the returned `message` falls back to that string before using the generic
invalid-request text.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR implements
InvalidRequestErrorin the Shade Python SDK so developers get clear, field-level feedback when the API rejects malformed or invalid request parameters. It parses the API error response format, exposes the offending field and all field errors, and raises the exception on HTTP 400/422 responses.Related Issue
Closes #17
Changes
src/shade/errors.pyInvalidRequestError(ShadeError)withparam: Optional[str]andfield_errors: dict.{"error": {"code": "invalid_param", "param": "amount", "message": "..."}}.InvalidRequestError.from_response()to construct the exception from raw 400/422 response bodies.raise_for_invalid_request()to raiseInvalidRequestErroron HTTP 400 and 422 responses.__str__()so the offending parameter name is included in the error message when available.🧪 Tests
tests/test_errors.pyparamfrom API error responses.field_errorsfrom API error responses.str(error)includes the param name.InvalidRequestError.Verification Results
InvalidRequestErrorerror.paramnames the offending field when the API provides oneerror.field_errorscontains all field-level errors from the API responsestr(error)includes the param name in the messageSummary by CodeRabbit