Skip to content

fix: Preserve HTTP errors for malformed response bodies - #114

Open
emmy-devop wants to merge 1 commit into
mainfrom
em/http-errors
Open

emmy-devop wants to merge 1 commit into
mainfrom
em/http-errors

Conversation

@emmy-devop

@emmy-devop emmy-devop commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by @drafty

🔴 Review tier: HIGH — human review required

  • Contract: changes SDK error translation for malformed/unstructured HTTP failures; preserves structured application error codes and machine-readable validation details.
  • Release: merging to main triggers version/release automation and PyPI publication. This PR does not merge, tag, publish, or change workflows.

What / why

A failed request can return a JSON string, plain text, HTML, or malformed JSON instead of the expected error object. Dictionary access (or deserialization before error translation) then raises AttributeError/JSONDecodeError, hiding the real HTTP failure.

  • Check for a mapping before accessing error fields. Fall back to the actual HTTP status × 1000 (HTTP 429 → RateLimitExceededError, code 429000), rather than trusting a body status_code or defaulting to 500.
  • Preserve integer application codes in the error-code range 400000..599999, including 429001 concurrency errors and unknown subcodes. Reject strings, booleans, floats, containers, nulls, and out-of-range codes without coercion. Valid application codes still take precedence, while the original transport status remains on error.response.status_code.
  • Preserve response body and headers, including both forms of Retry-After, and retain the original HTTP exception context.
  • Use a static HTTP status phrase for unstructured bodies instead of echoing arbitrary proxy diagnostics/HTML. Structured string details are limited to 1,024 characters with nonprintable characters replaced. Existing structured 422 validation dict/list details remain unchanged.
  • Tolerate malformed JSON/invalid text encoding only for HTTP failure responses; successful-response decoding remains strict. Covers sync/async requests, including failures encountered on streaming requests.

No limiter, retry policy, consumer dependency, workflow, version, changelog, or feature-document changes.

How tested

Python 3.11.2 with the repository's frozen development dependencies (uv sync --frozen). Task runner unavailable locally; executed the Taskfile's test/coverage steps directly.

  • coverage run -m pytest --junitxml=report.xml: 405 passed, including 273 new regression cases.
  • coverage report, coverage xml, coverage html -d coverage-report: passed; total coverage 92%, errors.py 100%.
  • GitHub push and PR CI: all 10 checks passed, covering Python 3.8–3.12.
  • ruff check asknews_sdk tests/test_errors.py: passed.
  • ruff format --check tests/test_errors.py: passed.
  • mypy asknews_sdk/errors.py asknews_sdk/response.py: passed.
  • uv build --wheel: passed; build only, no upload.
  • git diff --check and gitlint --commits HEAD^..HEAD: passed.
  • Baseline proof: the eight plain-text/malformed-JSON HTTP 429 cases (sync/async × streaming/non-streaming) fail against the unmodified base implementations with AttributeError / JSONDecodeError, and pass with this change.

Risk / follow-up / rollback

  • Shared SDK error handling affects every endpoint. Malformed code fields and body-only HTTP status overrides now deliberately fall back to the real HTTP status. Structured text details longer than 1,024 characters or containing controls are normalized; validation collections preserve their existing contract.
  • Human review/merge is required. Keep the fix: subject for the squash/merge so release automation selects a patch release.
  • After the SDK release, consumers must update/pin their resolved SDK dependency and redeploy through their normal human-owned release process. Existing installations do not receive this fix merely because this PR exists. Consumer tool-error presentation and retry/backoff behavior are separate follow-up work; preserving Retry-After does not add automatic retries.
  • The underlying HTTP rate limit remains in force; this only prevents the SDK from masking it. No production changes performed.
  • Rollback is a reviewed revert plus a new release; consumers can pin the preceding version if needed.

This branch has not been deployed

No deployments
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.

1 participant