Skip to content

fix: preserve HTTP status for non-JSON API errors#238

Open
mayankbohradev wants to merge 1 commit into
resend:mainfrom
mayankbohradev:fix/preserve-http-status-non-json-errors
Open

fix: preserve HTTP status for non-JSON API errors#238
mayankbohradev wants to merge 1 commit into
resend:mainfrom
mayankbohradev:fix/preserve-http-status-non-json-errors

Conversation

@mayankbohradev

@mayankbohradev mayankbohradev commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

What

When the API (or a proxy/CDN in front of it) returns a non-JSON body, the SDK no longer always raises a fake 500 InternalServerError.

It now uses:

  • the real HTTP status when status >= 400 (e.g. 429, 401, 503)
  • 500 only when the status looks successful but the body still is not usable JSON

Applies to both request.py and async_request.py. Error type is application_error (aligned with the Node SDK’s non-JSON error path).

Why

HTTP clients already return (content, status_code, headers), but the request layer discarded status_code and hard-coded 500 whenever:

  • Content-Type was not application/json, or
  • json.loads failed

So a real rate-limit / unauthorized / gateway HTML page became an unhelpful internal server error. Callers could not branch on e.code or use retry-after from headers correctly.

JSON error payloads that include statusCode / name / message are unchanged (perform() still handles those).

Testing

  • Added sync coverage in tests/request_test.py:
    • 429 + text/htmlcode == 429, headers preserved
    • 503 + non-JSON → code == 503
    • 200 + HTML → still code == 500
    • invalid JSON with 502code == 502
  • Added async coverage for 401 + HTML
  • Removed a vacuous emails_test case that patched make_request and never hit this path
  • Local: pytest tests/request_test.py tests/emails_test.py (482 suite tests also green via tox -e py)
  • Local: tox -e mypy green
  • Note: local tox -e lint fails on Python 3.13 (flake8<5 + importlib_metadata); CI still runs lint on 3.8–3.11

Out of scope

Does not change behavior when a JSON body parses successfully but omits statusCode while the HTTP status is 4xx/5xx. That is a separate issue.


Summary by cubic

Preserves real HTTP status codes for non‑JSON API responses in both sync and async requests. Fixes false 500s so clients can react to 4xx/5xx correctly and use headers like Retry-After.

  • Bug Fixes
    • Use actual HTTP status when the body isn’t JSON; fall back to 500 only if the status is <400 but the body is unusable.
    • Set error_type to "application_error" and preserve response headers on errors.
    • Updated resend/request.py and resend/async_request.py; added sync/async tests; removed a stale emails test.

Written for commit 8451c2d. Summary will update on new commits.

Review in cubic

When the response body is not usable JSON (CDN HTML, empty 5xx,
proxies), use the real HTTP status for 4xx/5xx instead of always
raising a fake 500. Mirror sync and async request paths.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.96%. Comparing base (7f1acd0) to head (8451c2d).
⚠️ Report is 188 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #238       +/-   ##
===========================================
+ Coverage   82.66%   95.96%   +13.29%     
===========================================
  Files           4       61       +57     
  Lines          75     3221     +3146     
===========================================
+ Hits           62     3091     +3029     
- Misses         13      130      +117     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants