Skip to content

Commit

Permalink
fix(retry): be more specific about when we retry
Browse files Browse the repository at this point in the history
Retries are now attempted only for specific known errors. All other will
fail fast. Additionally, requests will be retried on 404s and
rate-limiting status codes 420 (for Twitter) and 429, along with the
original 408 and 500-level error codes. All other codes will continue to
fast-path to request failure.

One big effect of this is that requests are no longer retried on
ENOTFOUND -- that is, the getaddrinfo DNS failure error. When this
happens, the problem is almost certainly that the user is currently
offline. In combination with conditional requests and the cache rules,
this means that make-fetch-happen now has effectively an "offline mode",
where the `default` cache mode will only fail on ENOTFOUND if there is
no cache entry available.

As before, if we're using a stale locally-cached request due to an error
(again, such as ENOTFOUND), a `Warning: 111` header will be added to the
response, so you can check that field to figure out what
make-fetch-happen did.

BREAKING CHANGE: Retry logic has changes.

* 404s, 420s, and 429s all retry now.
* ENOTFOUND no longer retries.
* Only ECONNRESET, ECONNREFUSED, EADDRINUSE, ETIMEDOUT, and `request-timeout` errors are retried.
  • Loading branch information
zkat committed Apr 9, 2017
1 parent 66e5e87 commit a47b782
Showing 1 changed file with 36 additions and 5 deletions.
41 changes: 36 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ const retry = require('promise-retry')
let ssri
const Stream = require('stream')

const RETRY_ERRORS = [
'ECONNRESET', // remote socket closed on us
'ECONNREFUSED', // remote host refused to open connection
'EADDRINUSE', // failed to bind to a local port (proxy?)
'ETIMEDOUT' // someone in the transaction is WAY TOO SLOW
// Known codes we do NOT retry on:
// ENOTFOUND (getaddrinfo failure. Either bad hostname, or offline)
]

const RETRY_TYPES = [
'request-timeout'
]

// https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
module.exports = cachingFetch
cachingFetch.defaults = function (_uri, _opts) {
Expand Down Expand Up @@ -196,7 +209,7 @@ function condFetch (uri, cachedRes, opts) {
if (ctrl.match(/must-revalidate/i)) {
throw err
} else {
setWarning(cachedRes, 111, `Unexpected error: ${err.message}`)
setWarning(cachedRes, 111, `${err.code}: ${err.message}`)
return cachedRes
}
})
Expand Down Expand Up @@ -280,8 +293,18 @@ function remoteFetch (uri, opts) {
return res
}
})
} else if (res.status === 408 || res.status >= 500) {
// 408 === timeout
} else if (
// Retriable + rate-limiting status codes
// When hitting an API with rate-limiting features,
// be sure to set the `retry` settings according to
// documentation for that.
res.status === 404 || // Not Found ("subsequent requests permissible")
res.status === 408 || // Request Timeout
res.status === 420 || // Enhance Your Calm (usually Twitter rate-limit)
res.status === 429 || // Too Many Requests ("standard" rate-limiting)
// Assume server errors are momentary hiccups
res.status >= 500
) {
if (req.method === 'POST') {
return res
} else if (req.body instanceof Stream) {
Expand All @@ -293,14 +316,22 @@ function remoteFetch (uri, opts) {
return res
}
}).catch(err => {
if (req.method !== 'POST') {
const code = err.code === 'EPROMISERETRY'
? err.retried.code
: err.code
if (
req.method !== 'POST' && (
RETRY_ERRORS.indexOf(code) >= 0 ||
RETRY_TYPES.indexOf(err.type) >= 0
)
) {
return retryHandler(err)
} else {
throw err
}
})
}, opts.retry === false ? { retries: 0 } : opts.retry).catch(err => {
if (err.status >= 500 || err.status === 408) {
if (err.status >= 400) {
return err
} else {
throw err
Expand Down

0 comments on commit a47b782

Please sign in to comment.