elixir-tesla / elixir-tesla/tesla

Add `:timeout` option to every adapter

Open
#255 11 comments 5 reactions 0 assignees View on GitHub
idea :bulb: stale:keep
Dominant language
Elixir
Stars
2.1k
Forks
365
Avg merge
4h 14m
Merged PRs (30d)
24

Description

[copy from #240]

Timeout middleware has been causing issues for a long long time (#151, #157, #237 and more).
The two biggest problems with it are the use of `Task` which breaks `Tesla.Mock` and overriding adapter-specific timeouts.

With a well-defined adapter interface we can introduce a common `:timeout` option for adapter and promote it instead of the middleware. If adapter's underlying http library supports it (like hackney) it will be same as setting some option (`recv_timeout: opts[:timeout]`), if the library does not implement it it will have to be implemented on the adapter level (most probably in a similar fashion as current timeout middleware).

The timeout middleware should probably stay as it is, since it provides a different semantics when used with Retry middleware - the timeout set would capture the whole operation of multiple requests while adapter-level timeout only works for a single request.

To-do list:
- [ ] Add `:timeout` option to adapter spec
- [ ] Implement `:timeout` in every adapter
- [ ] Update documentation to promote `:timeout` option instead of `Tesla.Middleware.Timeout`

The issue turned out to be more complex than I first thought. For example, hackney has `recv_timeout` option, but it is not the same as `Tesla.Middleware.Timeout`. `recv_timeout` applies to single `recv` call, while `Tesla.Middleware.Timeout wraps the whole request/response exchange.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.