Skip to content

Commit

Permalink
Improve .retry() method functionality with delays and prevent calling…
Browse files Browse the repository at this point in the history
… on successful request (#1527)

* Enable .retry() method to use an async function as a callback, to allow easily delaying retries by using a Promise and setTimeout.

* Prevent .retry() method from being invoked on successful request. Runs only when there's a defined error.

* - Update readme with new async functionality of retry()
- Make ESLint happy
- Add babel plugin so aync/await works on node 6.x
  • Loading branch information
IAMtheIAM authored Jul 17, 2020
1 parent f8ea331 commit 62eae78
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 10 deletions.
4 changes: 3 additions & 1 deletion .dist.babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@
}
}]
],
"sourceMaps": "inline"
"plugins": [
["@babel/plugin-transform-runtime"]
]
}
3 changes: 2 additions & 1 deletion .dist.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
"no-useless-escape": "off",
"no-cond-assign": "off",
"no-redeclare": "off",
"node/no-exports-assign": "off"
"no-fallthrough": "off",
"no-constant-condition": "off"
},
"globals": {
"regeneratorRuntime": "writable"
Expand Down
4 changes: 3 additions & 1 deletion .lib.babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@
}
}]
],
"sourceMaps": "inline"
"plugins": [
["@babel/plugin-transform-runtime"]
]
}
3 changes: 2 additions & 1 deletion .lib.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"node/no-unsupported-features/node-builtins": "off",
"no-func-assign": "off",
"no-global-assign": ["error", {"exceptions": ["exports"]}],
"node/no-exports-assign": "off"
"no-fallthrough": "off",
"no-constant-condition": "off"
},
"overrides": [
{
Expand Down
9 changes: 9 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,15 @@ This method has two optional arguments: number of retries (default 1) and a call
.then(finished);
.catch(failed);

The callback supplied can be async if you need to await any data or to create a delay before retrying. This code will retry 10 times, with a 3 second delay between each retry. To add a delay:

.retry(10, async function(err, res) {
let delay = 3000; // ms
log.error(`Request failed, retrying in ${delay / 1000} seconds ...`);
await new Promise(resolve => setTimeout(() => resolve(), delay));
return true;
})

Use `.retry()` only with requests that are *idempotent* (i.e. multiple requests reaching the server won't cause undesirable side effects like duplicate purchases).

## Setting Accept
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"Nick Baugh <niftylettuce@gmail.com>"
],
"dependencies": {
"@babel/runtime": "7.4.5",
"component-emitter": "^1.3.0",
"cookiejar": "^2.1.2",
"debug": "^4.1.1",
Expand Down
4 changes: 2 additions & 2 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,8 @@ Request.prototype._getFormData = function() {
* @api private
*/

Request.prototype.callback = function(err, res) {
if (this._shouldRetry(err, res)) {
Request.prototype.callback = async function(err, res) {
if (err !== null && (await this._shouldRetry(err, res))) {
return this._retry();
}

Expand Down
4 changes: 2 additions & 2 deletions src/node/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -857,8 +857,8 @@ Request.prototype.request = function() {
* @api private
*/

Request.prototype.callback = function(err, res) {
if (this._shouldRetry(err, res)) {
Request.prototype.callback = async function(err, res) {
if (err !== null && (await this._shouldRetry(err, res))) {
return this._retry();
}

Expand Down
4 changes: 2 additions & 2 deletions src/request-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,14 @@ const ERROR_CODES = ['ECONNRESET', 'ETIMEDOUT', 'EADDRINFO', 'ESOCKETTIMEDOUT'];
* @param {Response} [res] response
* @returns {Boolean} if segment should be retried
*/
RequestBase.prototype._shouldRetry = function(err, res) {
RequestBase.prototype._shouldRetry = async function(err, res) {
if (!this._maxRetries || this._retries++ >= this._maxRetries) {
return false;
}

if (this._retryCallback) {
try {
const override = this._retryCallback(err, res);
const override = await this._retryCallback(err, res);
if (override === true) return true;
if (override === false) return false;
// undefined falls back to defaults
Expand Down

1 comment on commit 62eae78

@niftylettuce
Copy link
Collaborator

@niftylettuce niftylettuce commented on 62eae78 Aug 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix seems like it was simply err !== null && if (err && . Therefore none of this other babel stuff should have been added.

Please sign in to comment.