Giter Site home page Giter Site logo

node-fetch-retry's Introduction

Version License Travis

node-fetch-retry

Node Module for performing retries for HTTP requests.

It is a wrapper around node-fetch library. It has default retry logic built in as described below, as well as configurable parameters. It also has built-in support for Apache OpenWhisk actions, adjusting the timeout to reflect the action timeout.

Installation

npm install @adobe/node-fetch-retry

Usage

This library works the same as the normal fetch api, but with some added features.

Default Behavior

Without configuring any parameters, the retry behavior will be as follows:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url);
}

This example uses only custom headers and will use default retry settings:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        headers: {
            'custom-header': '<<put custom header value here>>'
        }
    });
}

Optional Custom Parameters

All the retry options are configurable and can be set in retryOptions in the options object passed to fetch.

Parameter Format Description Environment variable Default Value
retryMaxDuration Number time in milliseconds to retry until throwing an error NODE_FETCH_RETRY_MAX_RETRY 60000 ms
retryInitialDelay Number time in milliseconds to wait between retries NODE_FETCH_RETRY_INITIAL_WAIT 100 ms
retryBackoff Number backoff factor for wait time between retries NODE_FETCH_RETRY_BACKOFF 2.0
retryOnHttpResponse Function a function determining whether to retry given the HTTP response. Can be asynchronous none retry on all 5xx errors
retryOnHttpError Function a function determining whether to retry given the HTTP error exception thrown. Can be asynchronous none retry on all FetchError's of type system
socketTimeout Number time until socket timeout in milliseconds. Note: if socketTimeout is >= retryMaxDuration, it will automatically adjust the socket timeout to be exactly half of the retryMaxDuration. To disable this feature, see forceSocketTimeout below NODE_FETCH_RETRY_SOCKET_TIMEOUT 30000 ms
forceSocketTimeout Boolean If true, socket timeout will be forced to use socketTimeout property declared regardless of the retryMaxDuration. Note: this feature was designed to help with unit testing and is not intended to be used in practice NODE_FETCH_RETRY_FORCE_TIMEOUT false

Note: the environment variables override the default values if the corresponding parameter is not set. These are designed to help with unit testing. Passed in parameters will still override the environment variables

Custom Parameter Examples

This example decreases the retryMaxDuration and makes the retry delay a static 500ms. This will do no more than 4 retries.

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: {
            retryMaxDuration: 2000,  // 30s retry max duration
            retryInitialDelay: 500,
            retryBackoff: 1.0 // no backoff
        }
    });
}

This example shows how to configure retries on specific HTTP responses:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: {
            retryOnHttpResponse: function (response) {
                if ( (response.status >= 500) || response.status >= 400) { // retry on all 5xx and all 4xx errors
                    return true;
                }
            }
        }
    });
}

This example uses custom socketTimeout values:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: {
            retryMaxDuration: 300000, // 5min retry duration
            socketTimeout: 60000, //  60s socket timeout
        }
    });
}

This example uses custom socketTimeout values and custom headers:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: {
            retryMaxDuration: 300000, // 5min retry duration
            socketTimeout: 60000, //  60s socket timeout
        },
        headers: {
            'custom-header': '<<put custom header value here>>'
        }
    });
}

This example shows how to retry on all HTTP errors thrown as an exception:

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: {
            retryOnHttpError: function (error) {
                return true;
            }
        }
    });
}

Disable Retry

You can disable all retry behavior by setting retryOptions to false.

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: false
    });
}

Disabling retry behavior will not prevent the usage of other options set on the options object.

Additional notes on retry duration

If the fetch is unsuccessful, the retry logic determines how long it will wait before the next attempt. If the time remaining will exceed the total time allowed by retryMaxDuration then another attempt will not be made. There are examples of how this works in the testing code.

Apache OpenWhisk Action Support

If you are running this in the context of an OpenWhisk action, it will take into account the action timeout deadline when setting the retryMaxDuration. It uses the __OW_ACTION_DEADLINE environment variable to determine if there is an action running.

Behavior: If retryMaxDuration is greater than the time till the action will timeout, it will adjust the retryMaxDuration to be equal to the time till action timeout.

Contributing

Contributions are welcomed! Read the Contributing Guide for more information.

Licensing

This project is licensed under the Apache V2 License. See LICENSE for more information.

node-fetch-retry's People

Contributors

alexkli avatar badvision avatar dependabot[bot] avatar drgirlfriend avatar dscho avatar jdelbick avatar mjroeleveld avatar moonthug avatar niewview avatar salim-runsafe avatar semantic-release-bot avatar tmathern avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

node-fetch-retry's Issues

How to use node-fetch-retry with @adobe/helix-fetch

I'm working on a project with @alexkli and we would like to use @stefan-guggisberg's https://github.com/adobe/helix-fetch but also use this project's retry functionality.

This project has node-fetch hardcoded as a dependency, but as the dependency footprint is small, it should theoretically be possible to use a different fetch implementation.

I've tried using npm 8.3's override functionality to make replace node-fetch transparently with @adobe/helix-fetch, but ran into subtle incompatibilities:

  • node-fetch exports fetch as a default export, in @adobe/helix-fetch it is { fetch }
  • node-fetch can be required, @adobe/helix-fetch needs to be imported

I could imagine a pull request that would

  1. turn node-fetch into a peer dependency
  2. support an options.fetch parameter that allows passing a fetch implementation
  3. uses node-fetch as the default for this parameter
  4. require()s node-fetch dynamically (so that it is optional, but that might make packaging a bit more difficult)

Added @adobe/node-fetch-retry to DefinitelyTyped

Hello all,

I have created a type definition of your package in DefinitelyTyped. Since this concerns your package, it's more than natural to kindly ask for your review. :)

Also, I have added myself as a definition owner. I can keep up with the responsibility, but I believe it makes sense for the main contributors of this package to be there as well. Please, let me know which other github usernames I should add to it.

Cheers,

fetch-retry does not retry on errors like ECONNRESET anymore

The refactoring dd0fd50 caused the error.

Before there was the check if any error occurred or the provided onHttpError function resolved to true:
dd0fd50#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346L29

The check is now only relying on the provided retryOnHttpResponse() function wich defaults to retry on 5xx errors. It is even hard to write your own retryOnHttpResponse() function, because sometimes, the response passed to it is the actual response from node-fetch and sometimes it is something different: dd0fd50#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346R192

One has the field statusText and one has the field statusMessage.

Expected Behaviour

fetch-retry should retry on any network related error returned by node-fetch or if the retryOnHttpResponse() resolves to true

If this is not an option, the response object passed to the retryOnHttpResponse() should always have the same fields like the node-fetch response. On top of that I would suggest documenting this as a breaking change.

Actual Behaviour

fetch-retry only retries when the retryOnHttpResponse() method resolves to true, which is only possible if the response has actual http error code.

Reproduce Scenario (including but not limited to)

Steps to Reproduce

It is hard to reproduce the error that this is causing for us. But basically we start a docker container in a child process and use fetch-retry to poll the endpoint exposed by the container until it returns a success. The first couple calls may result in a ECONNRESET error.

Platform and Version

node 14
Mac OS and Ubuntu

Sample Code that illustrates the problem

I added some links to the code in the description above to express what is going on.

Logs taken while reproducing problem

Logging and error behavior

Sorry that is not really an issue, but I need a clarification on how things are supposed to work.
If I have a "await fetch" with a .catch()
Does the .catch overrides the retry behavior? And if not, how can I do to check if a retry actually happened ?

Thanks.

retry on all thrown fetch errors

We used to retry on all thrown http errors:

(error !== null || (retryOptions.retryOnHttpResponse && (retryOptions.retryOnHttpResponse(response))))

Then when we refactored the code, the behavior changed and only retried on error.code 5xx which brought in this regression: #60
To be more thorough, we added a customizable function called retryOnHttpError and changed the default to retry on all FetchErrors: #63

The goal now is to expand the behavior (in addition to this change: #87) to call retryOnHttpError for all thrown errors, not just FetchError. This will make it easier to implement to make node-fetch-retry pluggable: #86

Developers can still use the custom retryOnHttpError function to add special behavior, like only retrying on FetchError's.

Error cannot be intercepted request-timeout, body-timeout, etimedout, econnreset

I get several errors that I want to catch.
However, I have tried to catch them, but I do not succeed.

The issue is the following: ECONNRESET, ETIMEDOUT, request-timeout, body-timeout.

When these errors occur, I want to set my error variable to true.

The app always crashes when these errors occur, I would like to prevent this by catching the error.

Platform and Version

NodeJS v16

Sample Code that illustrates the problem

const response = await fetch(loadedFeed[i], {
            retryOptions: {
                timeout: 3000,
                retryOnHttpResponse: function (response) {
                    if(response.status !== 200){
                        error = true;
                        console.log("Site not work");
                    }
                },
                retryOnHttpError(err){
                    if(err === true){
                        error = true;
                    }

                    if(err.message === 'ECONNRESET'){
                        error = true;
                    }

                    if(err.message === 'ETIMEDOUT'){
                        error = true;
                    }

                    if(err.message === 'request-timeout'){
                        error = true;
                    }

                    if(err.message === 'body-timeout'){
                        error = true;
                    }

                }
            }
        });

Logs taken while reproducing problem

C:\Users\admin\Desktop\v01\node_modules@adobe\node-fetch-retry\index.js:230
return reject(new FetchError(network timeout at ${url}, 'request-timeout'));
^
FetchError: network timeout at https://www.star.com.tr/rss/rss.asp?cid=14
at wrappedFetch (C:\Users\admin\Desktop\01\node_modules@adobe\node-fetch-retry\index.js:230:43)
at runMicrotasks ()
at processTicksAndRejections (node:internal/process/task_queues:96:5) {
type: 'request-timeout'
}

make fetch implementation pluggable

  1. Provide default fetch impl by default
    • to make simple case easy, just depend on node-fetch-retry and done
    • whatever seems most common, right now still node-fetch v2, soon v3, and then the nodejs native solution
  2. Allow to override fetch() impl used

See related #75 and #84

allow customizing of retry logging handler

Current Behaviour

Currently node-fetch-retry will console.log or console.error on retry (and other cases), with a specific message. For example here.

Inventory of console.*:

on retry:

on init (config read):

Issue

Apps using node-fetch-try might want a different log message or use a different logging.

For example, they might want to log error details from a specific header or the response body. Currently it only prints the response.statusText in case of a non-success HTTP response.

Apps should be in control and be able to have node-fetch-retry not call console.* at all.

Proposal

Add the option for a custom function onRetry in the retryOptions that would be used instead of the current logging. Signature might follow the usual pattern

function onRetry(response, error) {
   // response if response was returned otherwise null
   // error if an error was thrown
}

Note sure how to handle the "on init" case, maybe another onInitLog function. Or use the npm debug library (which is off by default).

silent mode?

is it possible to silence the default console outputs?

consistent URL logging approach

From #55

Note that node-fetch-retry is inconsistent and includes URLs in errors thrown (e.g. here). It's fair to assume that all errors get logged downstream. So if the policy is to hide URLs by default, then they should not be included in errors either.

Also it logs the FetchErrors (e.g. here) which sometimes contain URL as well:

FetchError failed with code: ECONNRESET; message: request to https://www.adobe.com/content/dam/shared/images/product-icons/svg/substance-3d-designer.svg failed, reason: read ECONNRESET
Retrying in 103 milliseconds, attempt 1 error: FetchError, request to https://www.adobe.com/content/dam/shared/images/product-icons/svg/substance-3d-designer.svg failed, reason: read ECONNRESET

With a custom logging option (#77) in place, there are two options:

  1. log urls by default, and let clients with sensitive cases add a custom logger function that skips urls
  2. do not log urls by default, and let clients who want to log urls add a custom logger function that does so

But for convenience there could also be a third option:

  1. add extra flag logDetails or logUrl that would be maybe off by default if security conscious

logs 2 lines for every retry

For every retry, two lines are logged (typically):

AbortError failed with type: aborted; message: The user aborted a request.
Retrying in 1005 milliseconds, attempt 1 error: AbortError, The user aborted a request.

Another error example:

FetchError failed with code: ECONNRESET; message: request to https://www.adobe.com/content/dam/dx-dc/us/en/sign/docusign-comparison/1_Icon_GreenCheckmark.svg failed, reason: read ECONNRESET
Retrying in 1030 milliseconds, attempt 1 error: FetchError, request to https://www.adobe.com/content/dam/dx-dc/us/en/sign/docusign-comparison/1_Icon_GreenCheckmark.svg failed, reason: read ECONNRESET

I think this can be limited to just the second message, as it includes the error message as well.

Create new release

The PR merged lately has not been published to NPM yet. Would you mind bumping and releasing to NPM?

Also, FYI, a colleague of mine created types for this lib here.

Release to NPM

The PR merged lately has not been published to NPM yet. Would you mind bumping and releasing to NPM?

Also, a colleague of mine created types for this lib here.

Incorrect Typescript Types

Expected Behaviour

Example in Readme does not show error, when using Typescript.

Actual Behaviour

Error, because Types are wrong

Steps to Reproduce

Implement example in ReadMe

Sample Code that illustrates the problem

const fetch = require('@adobe/node-fetch-retry');

async main() {
    const response = await fetch(url, {
        retryOptions: false
    });
}

Solution

The issue can be fixed by changing the .d.ts file to:

import {RequestInfo, RequestInit, Response} from 'node-fetch'

export interface RequestInitWithRetry extends RequestInit {
  retryOptions?: {
    retryMaxDuration?: number
    retryInitialDelay?: number
    retryBackoff?: number
    retryOnHttpError?: (error: Error) => boolean
    retryOnHttpResponse?: (response: Response) => boolean
    socketTimeout?: number
    forceSocketTimeout?: boolean
  } | bool
}

declare function fetch(
  url: RequestInfo,
  init?: RequestInitWithRetry
): Promise<Response>

export default fetch

Or even more specific

retryOptions?: {...} | false

Add retries metrics when used with openwhisk

To track retries in http metrics:

  • node-fetch-retry sets a special request header (name to be clarified) with the current retry count
  • node-fetch-retry sets a special request header (name to be clarified) if it's the last retry (can we know this when we make the requests?)
  • openwhisk-newrelic takes any "x-http-metrics-: " header and adds it as = attribute to the http metrics, then removes the header so that it does not actually get sent with the request
  • not everybody using node-fetch-retry uses openwhisk-newrelic, so is it ok to send those headers in this case?

handle developer errors from callbacks separate from fetch errors

Currently we share the same catch clause for fetch() and for the shouldRetry() here:

node-fetch-retry/index.js

Lines 217 to 227 in 3e512e5

try {
const response = await fetch(url, options);
if (await shouldRetry(retryOptions, null, response, waitTime)) {
console.error(`Retrying in ${waitTime} milliseconds, attempt ${attempt} failed (status ${response.status}): ${response.statusText}`);
} else {
// response.timeout should reflect the actual timeout
response.timeout = retryOptions.socketTimeout;
return resolve(response);
}
} catch (error) {

shouldRetry includes calling the callbacks retryOnHttpError() and retryOnHttpResponse(). These callbacks could have developer errors on which we should not retry blindly.

Instead, we should separate the error handling, and for errors on the callbacks pass them through as clear errors (and do not retry) so the developers can fix their code.

use AbortSignal.timeout() when available

This would give a nicer message than the current The user aborted a request. which is confusing when in fact it is a timeout:

AbortError failed with type: aborted; message: The user aborted a request.
Retrying in 129 milliseconds, attempt 1 error: AbortError, The user aborted a request.

MDN doc of AbortSignal.timeout(): https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout

WhatWG spec: https://dom.spec.whatwg.org/#interface-AbortSignal

Available in nodejs 16.14+: https://nodejs.org/dist/latest-v16.x/docs/api/globals.html#class-abortcontroller

Unfortunately the abort-controller library we use doesn't support it yet, neither do the other AbortController libraries on npm that I found (at this time).

Upgrade to node-fetch v3

We use node-fetch as the underlying fetch library many places in our code:

node-fetch-retry: https://github.com/adobe/node-fetch-retry/blob/master/package.json#L13
node-openwhisk-newrelic: https://github.com/adobe/node-openwhisk-newrelic/blob/master/package.json#L14

In some cases, we use a deprecated node-fetch-npm library:

node-httptransfer: https://github.com/adobe/node-httptransfer/blob/master/package.json#L26

We should be using the newest stable version of node-fetch (v3), which has breaking changes everywhere in our code to lessen the tech debt.

https://github.com/node-fetch/node-fetch/releases/tag/v3.0.0

ideally move to at least 3.1.1: https://github.com/node-fetch/node-fetch/releases/tag/v3.1.1 (or the most recent v3 version)

Note: v3 is now an ESM package.

confusing error message 'network timeout' if retryMaxDuration is hit

Observed behavior

If retryMaxDuration is reached, you get this error thrown (code here):

FetchError: network timeout at https://example.com/some/url
    at wrappedFetch (/some/filesystem/folder/myproject/node_modules/@adobe/node-fetch-retry/index.js:245:20)
    at runMicrotasks (<anonymous>)
    at runNextTicks (node:internal/process/task_queues:61:5)
    at processTimers (node:internal/timers:497:9) {
  type: 'request-timeout'
}

This is confusing, because:

  1. it's not directly a network timeout
  2. it does not mention the time that has passed (and the retryMaxDuration)
  3. and it misses the original/last error message of the actual request (in my case I am sure there must have been any since retryMaxDuration was 60 sec, and socketTimeout 30 sec)

Expected behavior

Change error message to something like this:

FetchError: retries reached max duration of 60 sec for https://example.com/some/url, last error: ECONNRESET

More details

What's also interesting is that before that error I ONLY see maximum of 1 retry attempt before, with short initial retry delay:

AbortError failed with type: aborted; message: The user aborted a request.
Retrying in 107 milliseconds, attempt 1 error: AbortError, The user aborted a request.
AbortError failed with type: aborted; message: The user aborted a request.
Retrying in 182 milliseconds, attempt 1 error: AbortError, The user aborted a request.

I would have expected to see something like Retrying in 30000 milliseconds, attempt 5 or similar before running into the retryMaxDuration limit.

There are many of those messages in the log output as my application makes many requests at the same time which all ran into some network limits at around the same time. They all have just log 1 retry attempt and then presumably work fine, except the one that failed.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.