Skip to content

Commit 3e512e5

Browse files
salim-runsafejdelbick
salim-runsafe
and
jdelbick
authored
FEATURE-RELEASE: Support for async retryOnHttpResponse and retryOnHttpError (#74)
Co-authored-by: Jamie Delbick <jamie.delbick@gmail.com>
1 parent 1a559b0 commit 3e512e5

File tree

3 files changed

+88
-7
lines changed

3 files changed

+88
-7
lines changed

‎README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ All the retry options are configurable and can be set in `retryOptions` in the `
5858
| `retryMaxDuration` | Number | time in milliseconds to retry until throwing an error | `NODE_FETCH_RETRY_MAX_RETRY` | 60000 ms |
5959
| `retryInitialDelay` | Number | time in milliseconds to wait between retries |`NODE_FETCH_RETRY_INITIAL_WAIT` | 100 ms |
6060
| `retryBackoff` | Number | backoff factor for wait time between retries | `NODE_FETCH_RETRY_BACKOFF` | 2.0 |
61-
| `retryOnHttpResponse` | Function | a *function* determining whether to retry given the HTTP response | none | retry on all 5xx errors|
62-
| `retryOnHttpError` | Function | a *function* determining whether to retry given the HTTP error exception thrown | none | retry on all `FetchError`'s of type `system`|
61+
| `retryOnHttpResponse` | Function | a *function* determining whether to retry given the HTTP response. Can be asynchronous | none | retry on all 5xx errors|
62+
| `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`|
6363
| `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 |
6464
| `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 |
6565

@@ -166,7 +166,7 @@ Disabling retry behavior will not prevent the usage of other options set on the
166166

167167
### Additional notes on retry duration
168168

169-
If the fetch is unsucessful, 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.
169+
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.
170170

171171
### Apache OpenWhisk Action Support
172172

‎index.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,18 @@ function isResponseTimedOut(retryOptions) {
4040
* @param {Object} error error object if the fetch request returned an error
4141
* @param {Object} response fetch call response
4242
* @param {Number} wait Amount of time we will wait before retrying next
43-
* @returns {Boolean} whether or not to retry the request
43+
* @returns {Promise<Boolean>} whether or not to retry the request
4444
*/
45-
function shouldRetry(retryOptions, error, response, waitTime) {
45+
async function shouldRetry(retryOptions, error, response, waitTime) {
4646
if (getTimeRemaining(retryOptions) < waitTime) {
4747
return false;
4848
} else if (retryOptions && retryOptions.retryOnHttpError && error != null) {
49+
// retryOnHttpError can be sync or async because either the promise or result will be
50+
// bubbled up to what shouldRetry returns
4951
return retryOptions.retryOnHttpError(error);
5052
} else if (retryOptions && retryOptions.retryOnHttpResponse) {
53+
// retryOnHttpResponse can be sync or async because either the promise or result will be
54+
// bubbled up to what shouldRetry returns
5155
return retryOptions.retryOnHttpResponse(response);
5256
} else {
5357
return false;
@@ -213,15 +217,15 @@ module.exports = async function (url, options) {
213217
try {
214218
const response = await fetch(url, options);
215219

216-
if (shouldRetry(retryOptions, null, response, waitTime)) {
220+
if (await shouldRetry(retryOptions, null, response, waitTime)) {
217221
console.error(`Retrying in ${waitTime} milliseconds, attempt ${attempt} failed (status ${response.status}): ${response.statusText}`);
218222
} else {
219223
// response.timeout should reflect the actual timeout
220224
response.timeout = retryOptions.socketTimeout;
221225
return resolve(response);
222226
}
223227
} catch (error) {
224-
if (!shouldRetry(retryOptions, error, null, waitTime)) {
228+
if (!(await shouldRetry(retryOptions, error, null, waitTime))) {
225229
if (error.name === 'AbortError') {
226230
return reject(new FetchError(`network timeout at ${url}`, 'request-timeout'));
227231
} else {

‎test/index.test.js

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,37 @@ describe('test `retryInit` function', () => {
126126
assert.strictEqual(retryOptions.socketTimeout, 2000);
127127
});
128128

129+
it('pass in custom parameters with async functions', async () => {
130+
const rewiredFetchRetry = rewire('../index');
131+
const retryInit = rewiredFetchRetry.__get__('retryInit');
132+
const retryOptions = retryInit({
133+
retryOptions: {
134+
retryMaxDuration: 3000,
135+
retryInitialDelay: 200,
136+
retryBackoff: 3.0,
137+
retryOnHttpResponse: async () => {
138+
return Promise.resolve(false);
139+
},
140+
retryOnHttpError: async () => {
141+
return Promise.resolve(false);
142+
},
143+
socketTimeout: 2000
144+
}
145+
});
146+
assert.strictEqual(typeof retryOptions.startTime, 'number');
147+
assert.strictEqual(retryOptions.retryMaxDuration, 3000);
148+
assert.strictEqual(retryOptions.retryInitialDelay, 200);
149+
assert.strictEqual(retryOptions.retryBackoff, 3);
150+
assert.strictEqual(typeof retryOptions.retryOnHttpResponse, 'function');
151+
assert.strictEqual(await retryOptions.retryOnHttpResponse({ status: 500 }), false);
152+
assert.strictEqual(await retryOptions.retryOnHttpResponse({ status: 400 }), false);
153+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('hello!', 'non-system')), false);
154+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('hello!', 'system')), false);
155+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('ECONNRESET', 'system')), false);
156+
assert.strictEqual(await retryOptions.retryOnHttpError(new Error('ECONNRESET', 'system')), false);
157+
assert.strictEqual(retryOptions.socketTimeout, 2000);
158+
});
159+
129160
it('pass in custom parameters and set enviroment variables, passed parameters take priority', () => {
130161
const rewiredFetchRetry = rewire('../index');
131162
const retryInit = rewiredFetchRetry.__get__('retryInit');
@@ -242,6 +273,29 @@ describe('test `retryInit` function', () => {
242273
assert.strictEqual(retryOptions.retryOnHttpError(new Error('ECONNRESET', 'system')), false);
243274
assert.strictEqual(retryOptions.retryOnHttpError(new SpecialError('error!')), true);
244275
});
276+
277+
it('custom async retry on http response', async () => {
278+
const rewiredFetchRetry = rewire('../index');
279+
const retryInit = rewiredFetchRetry.__get__('retryInit');
280+
const retryOptions = retryInit({
281+
retryOptions: {
282+
retryOnHttpError: async (error) => {
283+
return Promise.resolve(error.name === 'SpecialError' || false);
284+
}
285+
}
286+
});
287+
class SpecialError extends Error {
288+
constructor(message) {
289+
super(message);
290+
this.name = 'SpecialError';
291+
}
292+
}
293+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('hello!', 'non-system')), false);
294+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('hello!', 'system')), false);
295+
assert.strictEqual(await retryOptions.retryOnHttpError(new FetchError('ECONNRESET', 'system')), false);
296+
assert.strictEqual(await retryOptions.retryOnHttpError(new Error('ECONNRESET', 'system')), false);
297+
assert.strictEqual(await retryOptions.retryOnHttpError(new SpecialError('error!')), true);
298+
});
245299
});
246300

247301
describe('test fetch retry', () => {
@@ -471,6 +525,29 @@ describe('test fetch retry', () => {
471525
assert.strictEqual(response.status, 200);
472526
});
473527

528+
it('test retry with async retryOnHttpResponse', async () => {
529+
nock(FAKE_BASE_URL)
530+
.get(FAKE_PATH)
531+
.twice()
532+
.reply(403);
533+
nock(FAKE_BASE_URL)
534+
.get(FAKE_PATH)
535+
.reply(200);
536+
const response = await fetch(`${FAKE_BASE_URL}${FAKE_PATH}`,
537+
{
538+
method: 'GET', retryOptions: {
539+
retryInitialDelay: 200,
540+
retryMaxDuration: 1000,
541+
retryOnHttpResponse: async (response) => {
542+
return Promise.resolve(!response.ok);
543+
}
544+
}
545+
});
546+
assert(nock.isDone());
547+
assert.strictEqual(response.statusText, 'OK');
548+
assert.strictEqual(response.status, 200);
549+
});
550+
474551
it('do not retry on some HTTP codes', async () => {
475552
nock(FAKE_BASE_URL)
476553
.get(FAKE_PATH)

0 commit comments

Comments
 (0)