Skip to content

Add pluggable HTTP client behaviour with Hackney and Finch implementations#148

Closed
klacointe wants to merge 5 commits into
elixir-waffle:masterfrom
klacointe:feat/pluggable-http-client
Closed

Add pluggable HTTP client behaviour with Hackney and Finch implementations#148
klacointe wants to merge 5 commits into
elixir-waffle:masterfrom
klacointe:feat/pluggable-http-client

Conversation

@klacointe

@klacointe klacointe commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Follows up on the discussion in #147.

What changed

Extracts all network-related behaviour from Waffle.File into a pluggable
Waffle.HTTPClient behaviour, following the same pattern as Waffle.StorageBehavior.

Waffle.HTTPClient — new behaviour with a single callback:

@callback get(url :: String.t(), headers :: list(), options :: keyword()) ::
  {:ok, body()} |
  {:ok, body(), filename()} |
  {:error, :timeout | :service_unavailable | {:http_error, any()}}

A shared parse_content_disposition/1 helper is provided on the behaviour module
so custom implementations don't need to reimplement it.

Two built-in implementations:

  • Waffle.HTTPClient.Hackney — default, wraps the existing hackney API
  • Waffle.HTTPClient.Finch — opt-in alternative

Finch is an optional dependency. Add {:finch, "~> 0.18"} only if you want to use it. Hackney remains a regular dependency (unchanged from before this PR).

Retry logic stays in Waffle.File — it's client-agnostic and triggers on
:timeout, :recv_timeout, and :service_unavailable, so custom implementations
get retries for free.

Configuration

# Hackney (default) — add {:hackney, "~> 1.9"} to deps
config :waffle, :http_client, Waffle.HTTPClient.Hackney

# Finch — add {:finch, "~> 0.18"} to deps, start a pool, then:
config :waffle, :http_client, Waffle.HTTPClient.Finch
config :waffle, Waffle.HTTPClient.Finch, pool_name: MyApp.Finch

Custom implementations

Any module adopting the Waffle.HTTPClient behaviour can be plugged in:

config :waffle, :http_client, MyApp.HTTPClient

Tests

  • Unit tests for parse_content_disposition/1
  • Unit tests for both Hackney and Finch implementations (mocked transport layer)
  • Retry/dispatch integration tests in Waffle.File

@klacointe klacointe changed the title feat: introduce pluggable HTTP client behaviour Add pluggable HTTP client behaviour with Hackney and Finch implementations Jun 15, 2026
@klacointe klacointe force-pushed the feat/pluggable-http-client branch 13 times, most recently from bed2a5c to e96efb2 Compare June 15, 2026 21:00

@achempion achempion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good work 👍

left some comments, thank you for looking into this

Comment thread lib/waffle/behaviors/http_client_behaviour.ex Outdated
Comment thread lib/waffle/http_client/hackney.ex Outdated
Comment thread mix.exs Outdated
Comment thread test/actions/store_test.exs
Comment thread lib/waffle/http_client/finch.ex Outdated
Comment thread test/http_client/finch_test.exs Outdated
Comment thread lib/waffle/behaviors/http_client_behaviour.ex
Comment thread test/http_client/finch_test.exs Outdated
Comment thread lib/waffle/http_client/finch.ex Outdated
- Add Waffle.HTTPClient behaviour with a single get/3 callback and
  shared parse_content_disposition/1 helper
- Add Waffle.HTTPClient.Hackney (default) and Waffle.HTTPClient.Finch
  implementations
- Refactor Waffle.File to dispatch through the configured :http_client
  instead of calling hackney directly; retry logic stays client-agnostic
- Make hackney and finch optional dependencies — users add whichever
  they need; at least one is required for remote file downloads
- Upgrade CI from Elixir 1.13 to 1.20 (required by finch ~> 0.18)
- Bump credo to ~> 1.7 for Elixir 1.19+ compatibility
- Add tests for both implementations and retry behaviour in Waffle.File
@klacointe klacointe force-pushed the feat/pluggable-http-client branch from e96efb2 to 1f57d35 Compare June 15, 2026 21:42
- Remove hackney 4 from docs (incompatible API, no max_body_length)
- Keep hackney as a non-optional dep for backward compatibility
- Restore :recv_timeout error for hackney atom :timeout (backward compat)
- Add :recv_timeout to retryable errors in Waffle.File
- Rewrite Finch client to use stream_while/5: enforces max_body_length
  mid-stream to avoid OOM on large responses
- Include HTTP status code in unexpected_status error tuple
- Wrap Finch call in try/rescue for exception safety
- Guard finch.ex and finch_test.exs with Code.ensure_loaded?(Finch)
  so the library compiles without the optional dep
@klacointe klacointe marked this pull request as ready for review June 16, 2026 09:53
Comment thread lib/waffle/http_client/finch.ex Outdated
Comment thread lib/waffle/http_client/finch.ex
Comment thread lib/waffle/http_client/hackney.ex
- Remove connect_options from Finch request options (not a valid
  Finch request_opt; connection settings belong at pool startup)
- Document that connect_timeout and follow_redirect are unsupported
  by the Finch client
- Add comment in hackney.ex explaining the two distinct timeout shapes
  hackney returns (map for connect, atom for recv)
Finch has no built-in redirect handling. Since waffle only issues GET
requests, implement a redirect loop manually: detect 3xx, resolve the
Location header via URI.merge/2 (handles relative URLs), and re-issue
the request up to :max_redirects hops (default 5).
- Add :follow_redirect to Hackney options table (was passed to hackney
  but undocumented)
- Document that :follow_redirect (boolean) is unsupported by Finch;
  use :max_redirects instead
@klacointe klacointe force-pushed the feat/pluggable-http-client branch from 0cfdb18 to cbedd7c Compare June 16, 2026 17:47
@achempion

Copy link
Copy Markdown
Member

Thanks for the work. PR is getting too large. Can we please move Finch into a different PR, so this one only adds the adapter pattern?

@klacointe

Copy link
Copy Markdown
Contributor Author

Thanks for the work. PR is getting too large. Can we please move Finch into a different PR, so this one only adds the adapter pattern?

I've already spent a lot of time on this PR, I'll let you split the work if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants