fix: handle 204 No Content in Client.request

[henriquebastos]

Why

Client.request always called response.json(), which raises JSONDecodeError on a 204 No Content (empty body) — common for successful PUT/DELETE. Closes debt D-008.

What

return response.json() if response.content else None — returns None when the response has no body. Adds tests/test_client.py coverage for the empty-body case.

Rebased onto current main (originally branched pre-Ariad). Folds the Ariad doc coherence: roadmap → Done, debt D-008 → Paid.

Validation

• pytest: 45 passed locally; CI runs lint + the py3.10–3.13 matrix.
• pre-commit (ruff 0.11.9) clean.

[claude]

Claude finished @henriquebastos's task —— View job

---

PR Review Complete ✅

Todo List

• Read repository guidelines (CLAUDE.md)
• Review core fix in src/requestspro/client.py
• Analyze new tests in tests/test_client.py
• Check documentation updates
• Evaluate code quality, bugs, performance, security
• Provide comprehensive feedback

---

Review Feedback

✅ Overall Assessment: EXCELLENT

This is a well-crafted fix that elegantly resolves a real issue with proper testing and documentation. The implementation follows best practices and integrates seamlessly with the existing codebase.

🔍 Code Quality Analysis

Core Fix (src/requestspro/client.py:15)

return response.json() if response.content else None

Strengths:

• Elegant and minimal: Single-line fix that perfectly addresses the issue
• Intuitive behavior: Returning None for empty bodies is the expected behavior
• Maintains backward compatibility: Existing code with JSON responses continues to work unchanged
• Follows Python idioms: Uses truthiness of response.content appropriately

🧪 Test Coverage Analysis

New Test (tests/test_client.py:43-53)

def test_highlevel_request_returns_none_on_204_no_content(self, responses):
    responses.add("PUT", "https://h/first", status=204, body="")
    # ...
    assert result is None

Strengths:

• Comprehensive coverage: Tests the exact bug scenario (204 No Content)
• Clear naming: Test method name explicitly describes the behavior
• Good test structure: Follows given/when/then pattern consistently
• Realistic scenario: Uses PUT method, which commonly returns 204

Suggestions for enhancement:

• Consider testing other methods that commonly return empty bodies (DELETE, HEAD)
• Could test with different empty body scenarios (empty string vs None content)

🔒 Security Considerations

• No security concerns: The change doesn't introduce any security risks
• Error handling preserved: response.raise_for_status() still validates HTTP status codes
• No data exposure: Fix doesn't log or expose sensitive information

⚡ Performance Considerations

• Performance improvement: Avoids unnecessary JSON parsing on empty responses
• Minimal overhead: response.content check is very lightweight
• No regression risk: Change only affects the error path

📚 Documentation Updates

The debt ledger and roadmap updates are thorough and professional:

• D-008 properly marked as Paid: Clear closure of the technical debt item
• Roadmap updated correctly: Moving item from Active to Done with proper description

🏗️ Architecture & Design

• Maintains API contract: Method signature unchanged, return type appropriately updated
• Follows existing patterns: Consistent with the codebase's error handling approach
• Clean separation: Business logic change isolated to the appropriate layer

🚨 Potential Issues

Minor consideration:

• The fix uses response.content (bytes) for truthiness check. This works correctly but using response.text (str) might be more semantically clear for JSON APIs. However, response.content is the right choice here since it avoids potential encoding issues.

🎯 Final Recommendations

1. Merge with confidence - This is production-ready code
2. Consider expanding tests - Add DELETE and HEAD method test cases for completeness
3. Documentation - The existing docstring could mention the None return behavior for empty responses

This PR exemplifies excellent software engineering: identifying a real problem, implementing a minimal fix, adding proper test coverage, and maintaining clear documentation. The fix is elegant, safe, and addresses the exact issue described in D-008.

---