-
Notifications
You must be signed in to change notification settings - Fork 2
Make HTTP client recordings easier and safer #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9cc26f3 to
4e7cb53
Compare
## Why This Change Was Made - Recording/playback needed a flexible matching strategy so real apps can key requests deterministically (and scrub secrets) without sacrificing lookup performance. - Fixtures can contain secrets in both requests and responses, so scrubbing must support both directions. - Documentation and examples needed to be representative of runnable, tested code so users don’t copy/paste drift. ## What Was Changed - Redesigned `dream_http_client` recording/matching around a key function (`RecordedRequest -> String`) plus transformers: - `request_transformer` scrubs/normalizes requests before keying and persistence - `response_transformer` scrubs responses before persistence (Record mode only) - Introduced a builder-based recorder workflow (`recorder.new() |> ... |> start()`) and updated docs/snippets to use qualified `module.new()` with unqualified chained builder functions. - Expanded README + release notes and added runnable snippet/tests covering request/response scrubbing, ambiguous-match errors, and filename format. ## Note to Future Engineer - Downstream modules pinned to older major versions of `dream_http_client` must keep using their compatible API (yes, this is as fun as it sounds). - If you see "Ambiguous recording match", your key/transformer is too coarse—tighten it instead of “fixing” the library. - The response transformer is record-only on purpose: fewer surprises, more explicit fixtures. Congrats on having to think. BREAKING CHANGE: `dream_http_client` recorder/matching APIs were redesigned (new builder start flow, key-function matching, request/response transformers, and `client.new()` is now a function in the http_client module).
4e7cb53 to
4e65b95
Compare
Contributor
Author
|
Restored |
## Why This Change Was Made - Recording/playback and transformer support require persisted response metadata (status + headers) to make fixtures faithful, debuggable, and safe to share. - Streaming recordings previously failed to reliably capture `stream_start` headers, which undermined both fixture correctness and response scrubbing. - We needed high-confidence tests that prove these behaviors end-to-end, plus a tested snippet for cancellation (a core advertised feature). ## What Was Changed - Updated the httpc shim + client recording paths to reliably capture and persist response metadata: - Blocking recordings persist response status code + headers. - Streaming recordings reliably capture `stream_start` headers (and preserve them even when trailing headers are empty). - Strengthened integration tests to prove: - status/header persistence, - request/response transformers’ effects on persisted fixtures, - streaming header capture. - Added a tested `stream_cancel` snippet and linked it from the README. - Refined docs and release notes to reflect the above behavior and current API. ## Note to Future Engineer - Some streaming header behavior depends on `httpc` message ordering; the shim intentionally blocks `fetch_start_headers` until `stream_start` arrives so recordings don’t silently miss headers. - The cancellation snippet intentionally uses a short-lived stream and polls for process exit to avoid flaky timing. - If you’re wondering why we’re so defensive about headers: yes, it’s because computers are very polite and will happily hand you an empty list with a straight face.
ee9b825 to
2a0e855
Compare
## Why This Change Was Made - The Erlang `httpc` request tuple for entity-body requests takes Content-Type separately from headers; we were hardcoding `application/json`, which could override the caller’s intent and produce duplicate/conflicting Content-Type values. - The streaming receive loop used a hard-coded 30s timeout, ignoring the request’s configured timeout and causing surprising stream timeouts. ## What Was Changed - Resolve the request timeout once and thread it through the streaming receive loop so `selector_receive` uses the configured timeout. - Update the `httpc` shim to extract a case-insensitive `Content-Type` header, strip it from the outgoing headers list, and pass it as the tuple Content-Type (defaulting to `application/octet-stream` when none is provided). - Add `POST /content-type` to the mock server to echo the request Content-Type for verification. - Add a regression test ensuring `send()` respects an explicit `Content-Type` when a request has a body. ## Note to Future Engineer - `httpc` treats Content-Type as a separate tuple element for body requests; leaving a `content-type` header in `Headers` while also providing `ContentType` is an easy way to accidentally send duplicates. Because of course it is. - When sending a body without an explicit `Content-Type`, we default to `application/octet-stream`; if you expected something more specific, that’s on past-you.
## Why This Change Was Made - To align dream_http_client’s dependency constraints with the compatibility analysis results and avoid committing maintainer-only rangefinder outputs. ## What Was Changed - Updated `modules/http_client/gleam.toml` dependency version ranges based on compatibility testing. - Regenerated `modules/http_client/manifest.toml` to reflect the updated constraints. - Added `.gitignore` rules so only `modules/**/COMPATIBILITY.md` is tracked while `COMPATIBILITY_REPORT.md` and `CONSTRAINT_ANALYSIS.md` are ignored. - Added `modules/http_client/COMPATIBILITY.md` for the current compatibility matrix. ## Note to Future Engineer - If you rerun rangefinder, it will rewrite the report files; only keep `COMPATIBILITY.md` under version control unless you intentionally want the maintainer reports. - Yes, we’re ignoring the big reports on purpose—future you doesn’t need 300 lines of “I told you so” in every diff.
## Why This Change Was Made - Git does not version-control `.git/hooks`, so relying on a local pre-commit hook creates inconsistent behavior across clones. - We need a standard, repeatable way for contributors to install the project’s intended hooks. ## What Was Changed - Added a tracked pre-commit hook at `.githooks/pre-commit`. - Added `make install-hooks` to copy `.githooks/*` into `.git/hooks/` and ensure hook scripts are executable. ## Note to Future Engineer - If your commits aren’t running hooks, you probably forgot `make install-hooks` after cloning. Yes, Git still refuses to just track `.git/hooks` like a normal tool in the year 2025.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
documentation
Improvements or additions to documentation
enhancement
New feature or request
module
Change to a dream module
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
What changed (high level)
Breaking:
dream_http_clientrecording/matching redesign (4.0.0)recorder.new() |> ... |> start().MatchKey = fn(RecordedRequest) -> String.RequestTransformer = fn(RecordedRequest) -> RecordedRequestruns before keying and before persistence.recorder.request_transformer(...)ResponseTransformer = fn(RecordedRequest, RecordedResponse) -> RecordedResponseruns before persistence (Record mode only).recorder.response_transformer(...)client.newindream_http_clientis nowclient.new()(function) for consistency with other builders.Docs/snippets are now representative and tested
modules/http_client/README.mdso recording docs cover:record/playback/passthrough)modules/http_client/test/snippets/(including response scrubbing) and ensured the README points at those snippets.module.new()qualified (e.g.client.new(),recorder.new()), chained builder functions unqualified viaimport module.{...}.Storage behavior
{method}_{host}_{path}_{key_hash}_{content_hash}.jsonCross-module compatibility note (important)
dream_http_client.dream_opensearchpinsdream_http_client >= 2.1.0 and < 3.0.0, so it must useclient.new(value) rather thanclient.new().dream_http_clientitself.How it works now (recording flow)
request_transformerruns on the incoming request.response_transformerruns using(transformed_request, response).Test plan
modules/http_client:make format && make check && make docsmodules/opensearch:make checkMigration notes for users upgrading
dream_http_clientrequest_transformerto normalize/scrub requests before keying/persistence.response_transformerto scrub recorded responses before persistence (Record mode only).