lostisland/faraday

Retain additional adapter information (timing, timeout, etc)

Open

#527 opened on Sep 24, 2015

View on GitHub
 (6 comments) (0 reactions) (0 assignees)Ruby (5,861 stars) (997 forks)batch import
featurehelp wanted

Description

Some adapters (e.g. typhoeus) have detailed timing information (total_time, starttransfer_time, appconnect_time, pretransfer_time, connect_time, namelookup_time, etc) that they may have gotten from the underlying C library, so discovering timing information via middleware (e.g. start = Time.now; app.call(env).on_complete do |env| duration = Time.now - start end) has an inherent loss of fidelity.

Also, when operating in parallel, adapters don't/can't/shouldn't raise exceptions on timeouts (since we don't currently have any mechanism for handling on_complete callback exceptions when running in parallel). One hack for this is to check for response.status == 0, but it would be better to just ask response.timed_out? There may be other bugs/errors that lead to status == 0, and some adapters might stream the response and set the status to non-zero upfront but then timeout while waiting for the response to complete. So response.status == 0 can't equal timeout. We ought to preserve this information on the response.

response.timed_out? #=> true/false
response.time       #=> total time in seconds
response.time_ms    #=> total time in ms
response.times      #=> {total: t0, starttransfer: t1, appconnect: t2, ...}
response.times_ms   #=> {total: t0, starttransfer: t1, appconnect: t2, ...}

Stored on the response under the obvious keys: :timed_out and :times_ms.

I already have a PR at typhoeus/typhoeus#476 to update the typhoeus adapter with env[:typhoeus_timed_out], but it would make more sense to have a more standard mechanism that could be adopted by any adapters. If you think this is a reasonable approach (or have a different approach to suggest), I'll put together a PR for this (including updating some of other adapters).

Contributor guide