Skip to content

chores: Implement unified response parser#32

Open
KodeSage wants to merge 1 commit into
ShadeProtocol:mainfrom
KodeSage:feat/unified_parser
Open

chores: Implement unified response parser#32
KodeSage wants to merge 1 commit into
ShadeProtocol:mainfrom
KodeSage:feat/unified_parser

Conversation

@KodeSage

@KodeSage KodeSage commented Jun 27, 2026

Copy link
Copy Markdown

CLOSES #12

Description

This change introduces a single _parse_response function that every resource method can route HTTP responses through. It centralizes JSON decoding, success detection, and the mapping of HTTP status codes to the SDK's typed exception hierarchy, preventing per-resource error handling from drifting over time.

Previously, response handling logic lived alongside the retry-aware _raise_for_status helper and operated on raw (status, headers, body) tuples. _parse_response provides a clean, transport-agnostic entry point that takes an httpx.Response and either returns the decoded body or raises the correct typed
exception with the raw body and status attached.

Behavior implemented:

  • 2xx → decode JSON and return the dict.
  • 401 / 403AuthenticationError.
  • 400 / 422InvalidRequestError, carrying field-level errors when the
    response body provides them.
  • 404NotFoundError.
  • 429RateLimitError (parses the Retry-After header).
  • 5xxNetworkError (subject to retry by callers).
  • Any other non-2xxHTTPError, so nothing escapes the funnel unhandled.
  • JSON decode failure on a 2xx body, a non-dict 2xx body, or a 2xx body that
    carries a truthy error key → ShadeError("Invalid response from API") /
    the extracted error message.

Every raised exception exposes both status_code and the raw response_body. Non-JSON error bodies still map to their typed exception with the raw body attached, and because all typed errors subclass ShadeError, a malformed body never surfaces a raw JSONDecodeError.

To support field-level validation errors, InvalidRequestError now accepts and stores an optional field_errors attribute (backward-compatible — existing construction calls are unaffected).

Note on file naming: the proposed work referenced http_client.py, but the actual HTTP module in this project is src/shade/http.py (which already imports httpx as an optional dependency). The function was added there to avoid
fragmenting the HTTP layer.

Dependencies: no new runtime dependencies. httpx is already an optional dependency of the project; the type hint is declared as a string so the module continues to import cleanly when httpx is absent.

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

A new test module, tests/test_parse_response.py, exercises every acceptance
criterion against real httpx.Response objects. The full existing suite was
also run to confirm no regressions.

Reproduce locally:

# install test/runtime deps (stellar-sdk and httpx are required by the suite)
pip install stellar_sdk httpx

# run only the new tests
python -m pytest tests/test_parse_response.py -q

# run the full suite
python -m pytest -q

Result: 91 passed (61 existing + 30 new).

  • Success path — 2xx returns the decoded dict; empty body returns {};
    non-dict JSON and error-key bodies on 2xx are rejected.
  • Status → exception mapping — parametrized coverage for 401/403, 400/422,
    404, 429, 500/502/503/504, and other 4xx (e.g. 418).
  • Field-level errors — extracted from both nested (error.fields) and
    top-level (errors) shapes; None when absent.
  • Decode failure — non-JSON 2xx body raises ShadeError rather than a raw
    JSONDecodeError; non-JSON error bodies still map to the typed exception.
  • Exception context — raw response_body and status_code present on every
    raised exception; message falls back to a sensible default when the body has none.

Test configuration: Python 3.11, pytest, httpx 0.28.1.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Improved API response handling to return clearer, typed errors for common failure cases.
    • Added support for field-level validation details in invalid request errors.
  • Bug Fixes

    • Better handling of empty, non-JSON, or malformed responses without exposing low-level parsing errors.
    • More consistent error messages and status details across authentication, rate limit, not found, and server error responses.

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b250a705-8127-4643-b2d5-bd18cdad542f

📥 Commits

Reviewing files that changed from the base of the PR and between 3ad7931 and 37d8c7e.

📒 Files selected for processing (3)
  • src/shade/errors.py
  • src/shade/http.py
  • tests/test_parse_response.py

📝 Walkthrough

Walkthrough

Adds a _parse_response function to src/shade/http.py that centralizes JSON decoding and maps HTTP status codes to typed SDK exceptions. Extends InvalidRequestError with an optional field_errors attribute. A new test module validates all response paths.

Unified Response Parser

Layer / File(s) Summary
InvalidRequestError field_errors contract
src/shade/errors.py
Adds __init__ accepting optional status_code, response_body, and field_errors; stores field_errors on the instance and expands the docstring.
_parse_response and helper extractors
src/shade/http.py
Imports ShadeError, adds _error_message and _field_errors helpers, and implements _parse_response that decodes JSON, validates 2xx bodies, and maps 401/403, 400/422, 404, 429, 5xx, and other non-2xx statuses to typed exceptions.
test_parse_response suite
tests/test_parse_response.py
Covers success paths (200, 204), decode failures, truthy/falsy error key detection, all status-code-to-exception mappings, field error extraction, Retry-After parsing, and status_code/response_body presence on every exception.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • ShadeProtocol/shade-python#30: Modifies the same src/shade/http.py error/exception mapping logic with overlapping status-code handling for the same typed SDK exceptions.

Suggested reviewers

  • codebestia

🐇 A parser hops in, JSON in paw,
Maps every status by protocol law.
Four-oh-one? Auth error, begone!
Field errors nest where they belong.
The response flows clean, no raw crash to see —
Unified at last, as parsing should be! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.23% 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 is concise and accurately describes the main change: a unified response parser.
Description check ✅ Passed The description closely matches the template and includes summary, issue reference, testing, and checklist.
Linked Issues check ✅ Passed The changes implement the unified parser, error mapping, field errors, and non-JSON handling required by #12.
Out of Scope Changes check ✅ Passed The parser logic, error attribute, and tests all support the stated objective; no unrelated changes stand out.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@codebestia

Copy link
Copy Markdown
Contributor

Hello @KodeSage
Please fix the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement unified response parser

2 participants