Skip to content

fix: correct HTTP adapter response handling - #254

Merged
Mattsface merged 1 commit into
release/0.8.0from
fix/http-adapter-correctness
Aug 1, 2026
Merged

fix: correct HTTP adapter response handling#254
Mattsface merged 1 commit into
release/0.8.0from
fix/http-adapter-correctness

Conversation

@Mattsface

@Mattsface Mattsface commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

Correct HTTP adapter response-handling defects before sessions, timeouts, retries, or structured exception subclasses are introduced.

Why status classification now occurs before JSON decoding

Error responses from MLB (and intermediate proxies) may return HTML or other non-JSON bodies. Decoding before checking status incorrectly raised Bad JSON in response for 4xx/5xx failures. The adapter now classifies by HTTP status first, so a JSON or HTML 404 still returns empty data, and a JSON or HTML 5xx raises the HTTP-status exception.

How the full 2xx range is handled

Successful responses are now detected with 200 <= status_code <= 299, so statuses such as 201, 204, and 299 are accepted instead of falling through to the non-2xx fallback.

How empty successful responses are handled

After a successful status check, an empty body uses response_data = {}. Non-empty successful bodies are decoded as JSON once; invalid JSON still raises TheMlbStatsApiException("Bad JSON in response") with exception chaining.

How MlbResult now protects caller-owned data

MlbResult no longer uses a mutable default dictionary. Each instance gets a new outer dictionary (dict(data) or {}), and copyright is removed only from that copy. Caller-owned dictionaries are left unchanged.

Compatibility

  • 404 responses still return MlbResult(status_code=404, message=response.reason, data={})
  • 5xx responses still raise TheMlbStatsApiException with "{status}: {reason}"
  • No sessions, retries, timeouts, or new exception subclasses were added
  • Constructor and get() signatures are unchanged, including the unused data argument

Test plan

Focused adapter/result tests:

poetry run pytest \
  tests/test_mlb_dataadapter.py \
  tests/test_mlb_result.py \
  -v

Result: 26 passed, 0 failed, 0 xfailed, 0 xpassed

Complete offline suite:

poetry run pytest tests/ --ignore=tests/external_tests

Result: 95 passed

Live adapter tests:

poetry run pytest \
  tests/external_tests/mlbdataadapter/test_mlbadapter.py \
  -v

Result: 3 passed (live MLB API was available)

Risk and impact

  • Risk: Normal — response classification and MlbResult ownership change, but public signatures and 404/5xx semantics are preserved
  • Impact if wrong: callers could mis-handle non-200 success statuses, empty bodies, or HTML error pages; covered by the new regression tests
Open in Web Open in Cursor 

Co-authored-by: Matthew Spah <spahmatthew@gmail.com>
@Mattsface
Mattsface marked this pull request as ready for review August 1, 2026 05:30
@Mattsface
Mattsface merged commit dc57263 into release/0.8.0 Aug 1, 2026
3 checks passed
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