Improve HTTP Client Recording Performance and Reliability #36
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
## Why This Change Was Made
- Single-file recording strategy (recordings.json) had O(n) performance - every new recording required reading ALL existing recordings, modifying the list, and writing everything back
- Concurrent tests had file contention - multiple tests writing to the same file caused race conditions and test flakiness
- Large test suites accumulated hundreds of recordings in one file, making it impossible to inspect, debug, or version control individual recordings
- Users wanted granular control - commit specific fixtures, diff individual recordings, delete obsolete ones
## What Was Changed
- storage.gleam: Rewrote to use individual files with format {method}_{host}_{path}_{hash}.json
- Added build_filename() using SHA256 hash for uniqueness while keeping human-readable base
- Added sanitize_for_filename() to handle slashes, special chars, and path truncation (50 char limit)
- load_recordings() scans directory for all .json files instead of reading single file
- save_recording_immediately() writes one file directly - O(1) instead of O(n)
- Added gleam_crypto dependency for SHA256 hashing
- Updated storage API to require matching_config parameter (internal change, recorder API unchanged)
- Added 8 comprehensive tests: filename uniqueness, sanitization, overwrite behavior, fixture-based playback, etc.
- All tests now write to test/fixtures/recordings/ (committed to git for CI without mock server)
- Fixed all tests to use localhost:9876 instead of api.example.com
- Eliminated all nested cases from test suite
- Bumped version to 3.0.1
## Note to Future Engineer
- The hash is generated from the FULL matching signature, not just visible filename parts - this means GET /text and GET /text?page=2 have different hashes even though base filename looks identical
- test/fixtures/recordings/ is COMMITTED to git - this is intentional so CI can run tests in Playback mode without spinning up the mock server
- If a recording isn't loading, check the matching config - the hash includes headers/body if match_headers/match_body are enabled
- The matching_config parameter exists because filename uniqueness depends on what parts of the request we're matching on
- Yes, accumulating fixture files in git seems weird, but it's actually the entire point - proves the recording workflow works and gives you inspectable, version-controlled test data. Embrace it or go write flaky integration tests that depend on external APIs like a savage.
…t fixture collision
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR improves the HTTP client's recording and playback system by switching from a single-file strategy to individual files per recording. This change eliminates performance bottlenecks, enables concurrent test recording, and provides better organization for test fixtures.
Why This Change Was Made
The previous single-file recording strategy (
recordings.json) had several critical issues:What Changed
Core Implementation
{method}_{host}_{path}_{hash}.jsonTest Improvements
test/fixtures/recordings/and committed to gitlocalhost:9876instead ofapi.example.comAPI Changes
User-facing
recorderAPI unchanged - this is backward compatible. Internalstoragemodule functions now require amatching_configparameter, but most users don't call these directly.Test Plan
Version
Bumped to 3.0.1 (non-breaking enhancement)