feat: introduce pluggable HTTP client behaviour#150
Conversation
|
@achempion when you'll be ok with this one I'll propose the Finch adapter. |
9c5b76f to
df970da
Compare
- Add Waffle.HTTPClient behaviour with a single get/3 callback - Add Waffle.HTTPClient.Hackney as the default implementation - Refactor Waffle.File to dispatch through the configured :http_client instead of calling hackney directly; retry logic stays client-agnostic - Document how to implement a custom HTTP client adapter
df970da to
e6a6bd6
Compare
achempion
left a comment
There was a problem hiding this comment.
looking great, left couple of minor comments before merging
|
I am excited to see this work going into waffle, and I'm happy to help push it forward if needed. |
I'm on holidays for the 2 following weeks and will not have time to work on this before 😕 |
- Share a normalize_error/1 helper between the connect/response error
path and the read_body error path in Waffle.HTTPClient.Hackney, so a
timeout while streaming the response body now normalizes to
{:error, :timeout} / {:error, :recv_timeout} instead of being wrapped
as {:error, {:http_error, reason}}
- This makes body-read timeouts retryable, matching connection timeouts
- Replace assert called(...) with assert_called_exactly(..., 3) in the
retry tests so the asserted call count matches the inline comment
- Add coverage for both timeout error shapes during body read
Addresses review feedback on elixir-waffle#150
- Share a normalize_error/1 helper between the connect/response error
path and the read_body error path in Waffle.HTTPClient.Hackney, so a
timeout while streaming the response body now normalizes to
{:error, :timeout} / {:error, :recv_timeout} instead of being wrapped
as {:error, {:http_error, reason}}
- This makes body-read timeouts retryable, matching connection timeouts
- Inline retry_or_error/5 into request/4's case clause since it was a
5-arg helper used from a single call site
- Replace assert called(...) with assert_called_exactly(..., 3) in the
retry tests so the asserted call count matches the inline comment
- Add coverage for both timeout error shapes during body read
Addresses review feedback on elixir-waffle#150
3f1d66d to
d068c11
Compare
I finally found some time. |
|
Looking great, thanks for the PR I'll publish new minor release soon. |
Great! ☝️ 😄 |
Summary
Waffle.HTTPClientbehaviour with a singleget/3callbackWaffle.HTTPClient.Hackneyas the default implementation (existing behaviour, just moved into a dedicated module)Waffle.Fileto dispatch through the configured:http_clientinstead of calling hackney directly; retry logic stays client-agnosticUsage
Configure a custom client:
Implement the behaviour:
Error shape changes
{:error, :timeout}{:error, :timeout}— unchanged{:error, :recv_timeout}{:error, :recv_timeout}— unchanged{:error, {:waffle_hackney_error, {:ok, 503, headers, ref}}}{:error, :service_unavailable}{:error, {:waffle_hackney_error, {:ok, status, headers, ref}}}{:error, {:http_error, status}}e.g.{:error, {:http_error, 404}}The previous 503 and non-2xx shapes leaked a closed hackney
client_refinside the error tuple.Notes
#75