|
| 1 | +--- |
| 2 | +summary: Extract the body-cap subsystem and the httpx2-exception-mapper context managers out of client.py into dedicated _internal/ modules. |
| 3 | +--- |
| 4 | + |
| 5 | +# Design: Extract the body-cap subsystem into `_internal/body_cap.py` |
| 6 | + |
| 7 | +## Summary |
| 8 | + |
| 9 | +`client.py:38-212` holds ~180 lines of pure, self-contained code with zero |
| 10 | +`self`/`Client`/`AsyncClient` coupling: the `max_response_body_bytes` |
| 11 | +body-capping subsystem (`_validate_max_response_body_bytes`, |
| 12 | +`_parse_content_length`, `_CapExceeded`, `_accumulate_capped`, |
| 13 | +`_safe_extensions`, `_buffered_headers`, `_response_has_body`, |
| 14 | +`_read_capped`, `_read_capped_async`) and two httpx2-exception-mapper context |
| 15 | +managers (`_httpx2_exception_mapper`, `_httpx2_exception_mapper_sync`). This |
| 16 | +change moves the body-cap subsystem into a new `_internal/body_cap.py`, and |
| 17 | +folds the two context managers into the existing `_internal/exception_mapping.py` |
| 18 | +next to the `map_httpx2_exception` function they wrap. No behavior change. |
| 19 | + |
| 20 | +## Motivation |
| 21 | + |
| 22 | +- `client.py` is 2,154 lines, by far the largest module in the package. |
| 23 | + Everything genuinely coupled to `Client`/`AsyncClient` (`_terminal`, |
| 24 | + `_prepare_request`, `stream()`) has to stay there; ~200 lines of this file |
| 25 | + currently don't. |
| 26 | +- The package already has a home for exactly this shape of code: |
| 27 | + `_internal/status.py` holds `_raise_on_status_error` and the |
| 28 | + streaming-body predicates, `_internal/exception_mapping.py` holds |
| 29 | + `map_httpx2_exception` — both pure, self-contained, shared by both clients. |
| 30 | + The body-cap subsystem and the exception-mapper context managers are the |
| 31 | + same shape and never received the same treatment. |
| 32 | +- Same-day precedent: `2026-06-23.01-retry-policy-extraction.md` and |
| 33 | + `2026-06-23.02-decoder-resolver-extraction.md` pulled comparable |
| 34 | + self-contained logic out of their hosting files into dedicated modules. |
| 35 | + `2026-06-23.03-response-body-cap.md` landed the body-cap subsystem directly |
| 36 | + in `client.py` the same day and never got the follow-up move. |
| 37 | +- **Depth:** the body-cap subsystem's own tests (`tests/test_capped_read.py`, |
| 38 | + `tests/test_capped_read_props.py`) already exercise it as a standalone |
| 39 | + unit, importing the functions directly rather than through `Client`. The |
| 40 | + seam already exists in the tests; the source hasn't caught up. |
| 41 | +- **Deletion test:** delete `_internal/body_cap.py` after this change and the |
| 42 | + ~150 lines of cap-accumulation/header-rebuild logic reappear somewhere — |
| 43 | + it's real, load-bearing complexity, not a pass-through. It earns the move. |
| 44 | + |
| 45 | +## Design |
| 46 | + |
| 47 | +### 1. `_internal/body_cap.py` — new module |
| 48 | + |
| 49 | +Moves verbatim (no logic change): |
| 50 | + |
| 51 | +- `_MAX_RESPONSE_BODY_BYTES_INVALID`, `_validate_max_response_body_bytes` |
| 52 | +- `_parse_content_length` |
| 53 | +- `_CapExceeded`, `_accumulate_capped` |
| 54 | +- `_safe_extensions` |
| 55 | +- `_WIRE_BODY_HEADERS`, `_BODILESS_STATUS`, `_buffered_headers` |
| 56 | +- `_response_has_body` |
| 57 | +- `_read_capped`, `_read_capped_async` |
| 58 | + |
| 59 | +Needs its own imports: `httpx2`, `HTTPStatus` (from `http`), `Mapping` (from |
| 60 | +`collections.abc`), and `ResponseTooLargeError` (from `httpware.errors`) — |
| 61 | +all currently imported by `client.py` for this code's sake and dropped from |
| 62 | +`client.py`'s import block once it moves (`Mapping` and |
| 63 | +`ResponseTooLargeError` are used nowhere else in `client.py`; `HTTPStatus` |
| 64 | +stays imported in `client.py` because `stream()` uses it independently at |
| 65 | +`client.py:1152` and `:2146`). |
| 66 | + |
| 67 | +`_build_default_decoders` (`client.py:172-190`) stays in `client.py`. |
| 68 | +`architecture/decoders.md` documents its location explicitly ("`decoders=None` |
| 69 | +resolves via `client.py:_build_default_decoders()`"); it is the same shape of |
| 70 | +pure, self-contained code but moving it isn't part of this change's motivation |
| 71 | +and would mean editing a Seam B contract that this change has no reason to |
| 72 | +touch. |
| 73 | + |
| 74 | +### 2. `_internal/exception_mapping.py` — gains two context managers |
| 75 | + |
| 76 | +`_httpx2_exception_mapper` (async) and `_httpx2_exception_mapper_sync` (sync) |
| 77 | +move in next to `map_httpx2_exception`, which they already wrap. Same |
| 78 | +rationale as body-cap — pure, self-contained, sibling concern already has a |
| 79 | +module — bundled into this change rather than a second near-identical PR. |
| 80 | + |
| 81 | +### 3. `client.py` shrinks to three imported names |
| 82 | + |
| 83 | +Only `_validate_max_response_body_bytes` (called once per `__init__`), |
| 84 | +`_read_capped`, and `_read_capped_async` (called from `_terminal` and |
| 85 | +`stream()`, both worlds) are actually referenced from inside `Client`/ |
| 86 | +`AsyncClient`. Every other moved name (`_parse_content_length`, |
| 87 | +`_CapExceeded`, `_accumulate_capped`, `_safe_extensions`, `_buffered_headers`, |
| 88 | +`_response_has_body`) is internal to `body_cap.py` and never imported by |
| 89 | +`client.py`. `_httpx2_exception_mapper`/`_httpx2_exception_mapper_sync` are |
| 90 | +imported by name, same call sites as today (`client.py:283`, `:1151`, |
| 91 | +`:1255`, `:2145`). |
| 92 | + |
| 93 | +```python |
| 94 | +from httpware._internal.body_cap import _read_capped, _read_capped_async, _validate_max_response_body_bytes |
| 95 | +from httpware._internal.exception_mapping import _httpx2_exception_mapper, _httpx2_exception_mapper_sync, map_httpx2_exception |
| 96 | +``` |
| 97 | + |
| 98 | +Nine functions/one class collapse behind a three-name import in `client.py` |
| 99 | +for body-cap, plus the two context managers for exception mapping. |
| 100 | + |
| 101 | +### 4. `architecture/overview.md` — module-layout table |
| 102 | + |
| 103 | +Add a `body_cap.py` row under `_internal/`; update `exception_mapping.py`'s |
| 104 | +description to mention the context managers: |
| 105 | + |
| 106 | +```text |
| 107 | +└── _internal/ |
| 108 | + ├── body_cap.py # max_response_body_bytes: validate, read-capped (sync+async) |
| 109 | + ├── exception_mapping.py # map_httpx2_exception + context-manager wrappers (shared) |
| 110 | + ├── import_checker.py # is_*_installed flags |
| 111 | + ├── observability.py # _emit_event |
| 112 | + └── status.py # _raise_on_status_error, _is_streaming_body_*, STREAMING_BODY_MARKER |
| 113 | +``` |
| 114 | + |
| 115 | +## Non-goals |
| 116 | + |
| 117 | +- No behavior change. Cap validation, accumulation, header rebuild, |
| 118 | + `ResponseTooLargeError` reasons, and exception-mapping order stay |
| 119 | + byte-identical. |
| 120 | +- Not moving `_build_default_decoders` (see Design §1). |
| 121 | +- Not touching the public `max_response_body_bytes` parameter, its |
| 122 | + validation error message, or `ResponseTooLargeError`'s shape. |
| 123 | +- Not addressing the larger `Client`/`AsyncClient` sync/async duplication |
| 124 | + (the per-verb methods, `_prepare_request`, `_request_with_body`) — a |
| 125 | + separate, much larger candidate surfaced in the same architecture review, |
| 126 | + deliberately deferred pending its own design call. |
| 127 | + |
| 128 | +## Testing |
| 129 | + |
| 130 | +- **Parity net:** rename `tests/test_capped_read.py` → |
| 131 | + `tests/test_body_cap.py` and `tests/test_capped_read_props.py` → |
| 132 | + `tests/test_body_cap_props.py`, updating their imports from |
| 133 | + `httpware.client` to `httpware._internal.body_cap`. All existing |
| 134 | + assertions stay unchanged — byte-identical behavior is the bar. |
| 135 | +- Move `test_parse_content_length` (currently `tests/test_client_stream.py:446-447`, |
| 136 | + testing a `body_cap.py` function directly rather than `stream()` behavior) |
| 137 | + into `tests/test_body_cap.py`, updating its import. |
| 138 | +- No new tests — this is a pure move with an existing, adequate net |
| 139 | + (`test_body_cap.py`, `test_body_cap_props.py`, `test_client_body_cap.py` |
| 140 | + for cap wiring through the client, `test_error_mapping_terminal.py` for |
| 141 | + exception-mapping-at-terminal behavior). |
| 142 | +- `just lint && just test` green; 100% coverage maintained. |
| 143 | + |
| 144 | +## Risk |
| 145 | + |
| 146 | +- **Import cycle** (unlikely × medium): `_internal/body_cap.py` imports |
| 147 | + `httpware.errors.ResponseTooLargeError`. *Mitigation:* `_internal/status.py` |
| 148 | + already imports from `httpware.errors` today with no cycle — same |
| 149 | + direction, same precedent. |
| 150 | +- **Missed call site during the move** (unlikely × low): a stale import left |
| 151 | + in `client.py`, or a name imported but now unused. *Mitigation:* `just |
| 152 | + lint` catches unused imports; `just test` exercises every moved function |
| 153 | + through its existing suite. |
| 154 | +- **Test-file rename loses git history readability** (certain × low): |
| 155 | + `git mv` preserves blame across the rename; low impact either way. |
0 commit comments