From 7e91bda3ccaea64ee5b9980380ef803652e8a0b3 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 17:10:25 -0700 Subject: [PATCH 1/8] feat!: Remove `request` fallback, Update docs/types, --- index.d.ts | 4 ++-- index.js | 7 +------ package.json | 6 +++--- readme.md | 50 +++++++++++++++++++++++++------------------------- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/index.d.ts b/index.d.ts index 3a399b9..28b0a23 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,12 +1,12 @@ declare module 'retry-request' { - // eslint-disable-next-line node/no-unpublished-import import * as request from 'request'; + import * as teenyRequest from 'teeny-request'; namespace retryRequest { function getNextRetryDelay(retryNumber: number): void; interface Options { objectMode?: boolean; - request?: typeof request; + request: typeof request | typeof teenyRequest; retries?: number; noResponseRetries?: number; currentRetryAttempt?: number; diff --git a/index.js b/index.js index a293298..cfdbe66 100644 --- a/index.js +++ b/index.js @@ -71,12 +71,7 @@ function retryRequest(requestOpts, opts, callback) { opts = extend({}, DEFAULTS, opts); if (typeof opts.request === 'undefined') { - try { - // eslint-disable-next-line node/no-unpublished-require - opts.request = require('request'); - } catch (e) { - throw new Error('A request library must be provided to retry-request.'); - } + throw new Error('A request library must be provided to retry-request.'); } let currentRetryAttempt = opts.currentRetryAttempt; diff --git a/package.json b/package.json index 9cb43e5..7c23041 100644 --- a/package.json +++ b/package.json @@ -30,11 +30,12 @@ "node": ">=14" }, "dependencies": { + "@types/request": "^2.48.8", "debug": "^4.1.1", - "extend": "^3.0.2" + "extend": "^3.0.2", + "teeny-request": "^9.0.0" }, "devDependencies": { - "@types/request": "^2.48.8", "async": "^3.0.1", "gts": "^5.0.0", "jsdoc": "^4.0.0", @@ -43,7 +44,6 @@ "linkinator": "^4.0.0", "lodash.range": "^3.2.0", "mocha": "^9.2.2", - "request": "^2.87.0", "typescript": "^5.1.6" } } diff --git a/readme.md b/readme.md index 8c5beda..f7c038b 100644 --- a/readme.md +++ b/readme.md @@ -3,31 +3,30 @@ |Retry a [request][request] with built-in [exponential backoff](https://developers.google.com/analytics/devguides/reporting/core/v3/coreErrors#backoff). ```sh -$ npm install --save request +$ npm install --save teeny-request $ npm install --save retry-request ``` + ```js var request = require('retry-request', { - request: require('request') + request: require('teeny-request'), }); ``` -It should work the same as `request` in both callback mode and stream mode. +It should work the same as `request` and `teeny-request` in both callback mode and stream mode. -Note: This module only works when used as a readable stream, i.e. POST requests aren't supported ([#3](https://github.com/stephenplusplus/retry-request/issues/3)). +Note: This module only works when used as a readable stream, i.e. POST requests aren't supported ([#3](https://github.com/stephenplusplus/retry-request/issues/3)). ## Do I need to install `request`? -Yes! You must independently install `request` and provide it to this library: +Yes! You must independently install `teeny-request` OR `request` (_deprecated_) and provide it to this library: ```js var request = require('retry-request', { - request: require('request') + request: require('teeny-request'), }); ``` -*The code will actually look for the `request` module automatically to save you this step. But, being explicit like in the example is also welcome.* - #### Callback `urlThatReturns503` will be requested 3 total times before giving up and executing the callback. @@ -49,7 +48,7 @@ request(urlThatReturns503) ## Can I monitor what retry-request is doing internally? -Yes! This project uses [debug](https://www.npmjs.com/package/debug) to provide the current retry attempt, each response status, and the delay computed until the next retry attempt is made. To enable the debug mode, set the environment variable `DEBUG` to *retry-request*. +Yes! This project uses [debug](https://www.npmjs.com/package/debug) to provide the current retry attempt, each response status, and the delay computed until the next retry attempt is made. To enable the debug mode, set the environment variable `DEBUG` to _retry-request_. (Thanks for the implementation, @yihaozhadan!) @@ -57,9 +56,12 @@ Yes! This project uses [debug](https://www.npmjs.com/package/debug) to provide t ### requestOptions -Passed directly to `request`. See the list of options supported: https://github.com/request/request/#requestoptions-callback. +Passed directly to `request` or `teeny-request`. See the list of options supported: -### opts *(optional)* +- https://github.com/request/request/#requestoptions-callback +- https://github.com/googleapis/teeny-request#teenyrequestoptions-callback + +### opts _(optional)_ #### `opts.noResponseRetries` @@ -71,7 +73,7 @@ The number of times to retry after a response fails to come through, such as a D ```js var opts = { - noResponseRetries: 0 + noResponseRetries: 0, }; request(url, opts, function (err, resp, body) { @@ -96,7 +98,7 @@ Default: `2` ```js var opts = { - retries: 4 + retries: 4, }; request(urlThatReturns503, opts, function (err, resp, body) { @@ -113,7 +115,7 @@ Default: `0` ```js var opts = { - currentRetryAttempt: 1 + currentRetryAttempt: 1, }; request(urlThatReturns503, opts, function (err, resp, body) { @@ -131,7 +133,7 @@ Default: Returns `true` if [http.incomingMessage](https://nodejs.org/api/http.ht var opts = { shouldRetryFn: function (incomingHttpMessage) { return incomingHttpMessage.statusMessage !== 'OK'; - } + }, }; request(urlThatReturnsNonOKStatusMessage, opts, function (err, resp, body) { @@ -146,21 +148,19 @@ request(urlThatReturnsNonOKStatusMessage, opts, function (err, resp, body) { Type: `Function` -Default: `try { require('request') }` - -If we cannot locate `request`, we will throw an error advising you to provide it explicitly. +If we not provided we will throw an error advising you to provide it. -*NOTE: If you override the request function, and it returns a stream in object mode, be sure to set `opts.objectMode` to `true`.* +_NOTE: If you override the request function, and it returns a stream in object mode, be sure to set `opts.objectMode` to `true`._ ```js -var originalRequest = require('request').defaults({ +var originalRequest = require('teeny-request').defaults({ pool: { - maxSockets: Infinity - } + maxSockets: Infinity, + }, }); var opts = { - request: originalRequest + request: originalRequest, }; request(urlThatReturns503, opts, function (err, resp, body) { @@ -190,9 +190,9 @@ Type: `Number` Default: `600` -The length of time to keep retrying in seconds. The last sleep period will be shortened as necessary, so that the last retry runs at deadline (and not considerably beyond it). The total time starting from when the initial request is sent, after which an error will be returned, regardless of the retrying attempts made meanwhile. +The length of time to keep retrying in seconds. The last sleep period will be shortened as necessary, so that the last retry runs at deadline (and not considerably beyond it). The total time starting from when the initial request is sent, after which an error will be returned, regardless of the retrying attempts made meanwhile. -### cb *(optional)* +### cb _(optional)_ Passed directly to `request`. See the callback section: https://github.com/request/request/#requestoptions-callback. From 33b5d9292ce8546ea762bd58e308c074d1f27361 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 17:50:23 -0700 Subject: [PATCH 2/8] fix: undocumented `string` URL param support --- index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/index.js b/index.js index cfdbe66..1ba9955 100644 --- a/index.js +++ b/index.js @@ -60,6 +60,10 @@ const DEFAULTS = { }; function retryRequest(requestOpts, opts, callback) { + if (typeof requestOpts === 'string') { + requestOpts = {url: requestOpts}; + } + const streamMode = typeof arguments[arguments.length - 1] !== 'function'; if (typeof opts === 'function') { From 79beddc07dee61a0057bdfc40429ea4c4a76c634 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:04:45 -0700 Subject: [PATCH 3/8] feat: Export `defaults` --- index.d.ts | 1 + index.js | 1 + 2 files changed, 2 insertions(+) diff --git a/index.d.ts b/index.d.ts index 28b0a23..1aeafe5 100644 --- a/index.d.ts +++ b/index.d.ts @@ -3,6 +3,7 @@ declare module 'retry-request' { import * as teenyRequest from 'teeny-request'; namespace retryRequest { + defaults = retryRequest.Options; function getNextRetryDelay(retryNumber: number): void; interface Options { objectMode?: boolean; diff --git a/index.js b/index.js index 1ba9955..b13cf5d 100644 --- a/index.js +++ b/index.js @@ -269,4 +269,5 @@ function getNextRetryDelay(config) { ); } +module.exports.defaults = DEFAULTS; module.exports.getNextRetryDelay = getNextRetryDelay; From 67aa175ebacc90ce5e9ab7ffa75c33d1f5bc9a18 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:09:55 -0700 Subject: [PATCH 4/8] fix: test timeout --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7c23041..f9f159d 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "docs-test": "linkinator docs", "fix": "gts fix", "lint": "gts check", - "test": "mocha --timeout 0", + "test": "mocha", "system-test": "" }, "files": [ From bbe38c1cdb751cdffd296ce394d245bf21eed66c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:13:38 -0700 Subject: [PATCH 5/8] chore(deps): update `mocha` --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f9f159d..3fc9f58 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "jsdoc-region-tag": "^3.0.0", "linkinator": "^4.0.0", "lodash.range": "^3.2.0", - "mocha": "^9.2.2", + "mocha": "^10.2.0", "typescript": "^5.1.6" } } From 47d5f59169a67ae4c37b21a00d36f125c5bf76e5 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:19:27 -0700 Subject: [PATCH 6/8] test: use `teeny-request` as replacement default --- test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test.js b/test.js index 2543954..4957655 100644 --- a/test.js +++ b/test.js @@ -5,12 +5,15 @@ const async = require('async'); const range = require('lodash.range'); const {describe, it, beforeEach} = require('mocha'); const {PassThrough} = require('stream'); +const {teenyRequest} = require('teeny-request'); const retryRequest = require('./index.js'); +retryRequest.defaults.request = teenyRequest.defaults(); + describe('retry-request', () => { - const URI_404 = 'http://yahoo.com/theblahstore'; - const URI_200 = 'http://yahoo.com/'; + const URI_404 = 'http://google.com/theblahstore'; + const URI_200 = 'http://google.com/'; const URI_NON_EXISTENT = 'http://theblahstore'; describe('streams', () => { From a150c4f5a2fc4519e245cf69cb3a09099d9769dc Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:32:38 -0700 Subject: [PATCH 7/8] fix: handle stream complete/finish events uniformly on errors `teeny-request` and `request` emit different calls when an error occurs. --- index.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index b13cf5d..972e92b 100644 --- a/index.js +++ b/index.js @@ -130,9 +130,17 @@ function retryRequest(requestOpts, opts, callback) { } function makeRequest() { + let finishHandled = false; currentRetryAttempt++; debug(`Current retry attempt: ${currentRetryAttempt}`); + function handleFinish(args = []) { + if (!finishHandled) { + finishHandled = true; + retryStream.emit('complete', ...args); + } + } + if (streamMode) { streamResponseHandled = false; @@ -163,7 +171,8 @@ function retryRequest(requestOpts, opts, callback) { streamResponseHandled = true; onResponse(null, resp, body); }) - .on('complete', retryStream.emit.bind(retryStream, 'complete')); + .on('complete', (...params) => handleFinish(params)) + .on('finish', (...params) => handleFinish(params)); requestStream.pipe(delayStream); } else { From 02d3d68209a7eb615b2f23110e2f229915b65b30 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 10 Oct 2023 18:54:24 -0700 Subject: [PATCH 8/8] chore: update timeout for (slow) `windows` instance --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3fc9f58..8d6057f 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "docs-test": "linkinator docs", "fix": "gts fix", "lint": "gts check", - "test": "mocha", + "test": "mocha --timeout 30000", "system-test": "" }, "files": [