BREAKING-RELEASE: change default behavior so fetch-retry retries on FetchErrors of type system; add custom error handling option (#63)

* fetch-retry retries on FetchErrors of type system

* network timeout test mocked

* fix test

* fix function description

* make test fuzzy

* skip not mocked enotfound test since it is not stable

Author: jdelbick

README.md

@@ -24,6 +24,8 @@ Without configuring any parameters, the retry behavior will be as follows:
 - retry for 60s
 - retry inital delay of 100ms with exponential backoff, configurable as a multiplier defaulting to 2
 - retry only on 5xx response
+- retry on all FetchError system errors
+   - see node-fetch error handling: https://github.com/node-fetch/node-fetch/blob/main/docs/ERROR-HANDLING.md
 - socket timeout of 30s
 ```js
 const fetch = require('@adobe/node-fetch-retry');
@@ -57,6 +59,7 @@ All the retry options are configurable and can be set in `retryOptions` in the `
 | `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 | none | retry on all 5xx errors|
+| `retryOnHttpError` | Function | a *function* determining whether to retry given the HTTP error exception thrown | 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 |
 
@@ -129,6 +132,20 @@ async main() {
     });
 }
 ```
+This example shows how to retry on all HTTP errors thrown as an exception:
+```js
+const fetch = require('@adobe/node-fetch-retry');
+
+async main() {
+    const response = await fetch(url, {
+        retryOptions: {
+            retryOnHttpError: function (error) {
+                return true;
+            }
+        }
+    });
+}
+```
 
 
 ### Disable Retry

index.js

@@ -45,6 +45,8 @@ function isResponseTimedOut(retryOptions) {
 function shouldRetry(retryOptions, error, response, waitTime) {
     if (getTimeRemaining(retryOptions) < waitTime) {
         return false;
+    } else if (retryOptions && retryOptions.retryOnHttpError && error != null) {
+        return retryOptions.retryOnHttpError(error);
     } else if (retryOptions && retryOptions.retryOnHttpResponse) {
         return retryOptions.retryOnHttpResponse(response);
     } else {
@@ -92,6 +94,8 @@ function retryInit(options={}) {
             retryBackoff: retryOptions.retryBackoff || DEFAULT_BACKOFF,
             retryOnHttpResponse: ((typeof retryOptions.retryOnHttpResponse === 'function') && retryOptions.retryOnHttpResponse) ||
                 ((response) => { return response.status >= 500; }),
+            retryOnHttpError: ((typeof retryOptions.retryOnHttpError === 'function') && retryOptions.retryOnHttpError) ||
+                ((error) => { return shouldRetryOnHttpError(error); }),
             socketTimeout: socketTimeoutValue
         };
     }
@@ -125,6 +129,9 @@ function checkParameters(retryOptions) {
     if (retryOptions.retryOnHttpResponse && !(typeof retryOptions.retryOnHttpResponse === 'function')) {
         throw new Error(`'retryOnHttpResponse' must be a function: ${retryOptions.retryOnHttpResponse}`);
     }
+    if (retryOptions.retryOnHttpError && !(typeof retryOptions.retryOnHttpError === 'function')) {
+        throw new Error(`'retryOnHttpError' must be a function: ${retryOptions.retryOnHttpError}`);
+    }
     if (typeof retryOptions.retryBackoff !== 'undefined'
         && !(Number.isInteger(retryOptions.retryBackoff) && retryOptions.retryBackoff >= 1.0)) {
         throw new Error('`retryBackoff` must be a positive integer >= 1');
@@ -135,24 +142,26 @@ function checkParameters(retryOptions) {
 }
 
 /**
- * Format HTTP error response into response object
+ * Evaluates whether or not to retry based on HTTP error
  * @param {Object} error 
- * @returns 
+ * @returns Returns true for all FetchError's of type `system`
  */
-function getResponseFromHttpError(error) {
-    let code = error.code;
-    if (!code || typeof(code) !== 'number') {
-        console.warn(`error code is invalid ${code}. Setting status code to 500`);
-        code = 500;
+function shouldRetryOnHttpError(error) {
+    // special handling for known fetch errors: https://github.com/node-fetch/node-fetch/blob/main/docs/ERROR-HANDLING.md
+    // retry on all errors originating from Node.js core
+    if (error.name === 'FetchError' && error.type === 'system') {
+        console.error(`FetchError failed with code: ${error.code}; message: ${error.message}`);
+        return true;
     }
-    return {status: code, statusMessage : error.message || "Unknown server error"};
+    return false;
 }
 
 /**
  * @typedef {Object} RetryOptions options for retry or false if want to disable retry
  * @property {Integer} retryMaxDuration time (in milliseconds) to retry until throwing an error
  * @property {Integer} retryInitialDelay time to wait between retries in milliseconds
  * @property {Function} retryOnHttpResponse a function determining whether to retry on a specific HTTP code
+ * @property {Function} retryOnHttpError a function determining whether to retry on a specific HTTP error
  * @property {Integer} retryBackoff backoff factor for wait time between retries (defaults to 2.0)
  * @property {Integer} socketTimeout Optional socket timeout in milliseconds (defaults to 60000ms)
  * @property {Boolean} forceSocketTimeout If true, socket timeout will be forced to use `socketTimeout` property declared (defaults to false)
@@ -162,6 +171,11 @@ function getResponseFromHttpError(error) {
  * @property {Number} response response from the http fetch call
  * @returns {Boolean} true if want to retry on this response, false if do not want to retry on the response
  */
+/**
+ * @typedef {Function} retryOnHttpError determines whether to do a retry on the HTTP error response
+ * @property {Object} error error thrown during the fetch request
+ * @returns {Boolean} true if want to retry on this error, false if do not want to retry on the response
+ */
 /**
  * @typedef {Object} Options options for fetch-retry
  * @property {Object} RetryOptions options for retry or false if want to disable retry
@@ -203,15 +217,14 @@ module.exports = async function (url, options) {
                         return resolve(response);
                     }
                 } catch (error) {
-                    const response = getResponseFromHttpError(error);
-                    if (!shouldRetry(retryOptions, error, response, waitTime)) {
+                    if (!shouldRetry(retryOptions, error, null, waitTime)) {
                         if (error.name === 'AbortError') {
                             return reject(new FetchError(`network timeout at ${url}`, 'request-timeout'));
                         } else {
                             return reject(error);
                         }
                     }
-                    console.error(`Retrying in ${waitTime} milliseconds, attempt ${attempt} error: ${error.message}`);
+                    console.error(`Retrying in ${waitTime} milliseconds, attempt ${attempt} error: ${error.name}, ${error.message}`);
                 } finally {
                     clearTimeout(timeoutHandler);
                 }