Add `:timeout` option to every adapter

[teamon]

[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.

[RyanSiu1995]

The new interface for request function will be request(method, url, headers, body, timeout, opts) My plan is to use Task.await/1 to handle the timeout. If the user specify adapter specific timeout opt, like recv_timeout in hackney, this :timeout option will be overrided and ignored. Does it sound reasonable for you?

[mroach]

I'm not sure if this is the place to ask, but what do you think about being able to set the timeout on a per-request basis? For example if you have requests that must reply quickly or fail, and endpoints that are allowed to take a longer time, currently the only option would be separate modules for each endpoint. My hacky patch to try it out was simply adding this in call/3 in the middleware:

timeout = Keyword.get(env.opts, :timeout) || Keyword.get(opts, :timeout) || @default_timeout

Then I can do:

get("http://server.tld/rest/must_be_fast", opts: [timeout: 20])

[teamon]

@mroach Currently the easiest option would be to use Tesla.client/2 function to create a custom client for cases when you need something specific (and you can use that with any middleware).

Having said that, it makes sense to have per-request middleware options customization, but it would need to work consistently for all middlewares (i.e., no special case for timeout).

[mroach]

@teamon Just to be sure I understand, the idea would be something like specifying options in a way that doesn't conflict between middleware and is consistent for middleware to access?

Something like:

get("http://server/api/data", opts: [Tesla.Middleware.Timeout: [timeout: 100]])

A bit verbose, but something along those lines to namespace the configuration and make it easy for middleware to detect?

[bmarkons]

Hi, I found this issue while I was trying to figure out how to provide timeout for single Tesla request. I managed to do it by modifying the client pre field and injecting Tesla.Middleware.Timeout into it. However, I would expect to provide timeout through option like @mroach suggested but it seems it is not possible yet.

So my question is, is there any other easier way?

[yordis]

@teamon would the ideal scenario here be to reserve a key in options or something that we would use in the adapters?

Related to #151 also