feat: retry when net err or timeout (#876)

* feat: retry when net err or timeout

* test: should not retry when params.stream is not readable

* test: retry in browser

Co-authored-by: beajer <919060679@qq.com>

README.md

@@ -336,6 +336,7 @@ options:
 `fetch` mode ,else `XMLHttpRequest`
 - [enableProxy] {Boolean}, Enable proxy request, default is false.
 - [proxy] {String | Object}, proxy agent uri or options, default is null.
+- [retryMax] {Number}, used by auto retry send request count when request error is net error or timeout.
 
 example:
 

lib/browser/client.js

@@ -13,6 +13,7 @@ const { createRequest } = require('../common/utils/createRequest');
 const { encoder } = require('../common/utils/encoder');
 const { getReqUrl } = require('../common/client/getReqUrl');
 const { setSTSToken } = require('../common/utils/setSTSToken');
+const { retry } = require('../common/utils/retry');
 
 const globalHttpAgent = new AgentKeepalive();
 
@@ -193,7 +194,27 @@ proto.authorization = function authorization(method, resource, subres, headers)
  * @api private
  */
 
-proto.request = async function request(params) {
+proto.request = async function (params) {
+ if (this.options.retryMax) {
+ this.request = retry(request.bind(this), this.options.retryMax, {
+ errorHandler: (err) => {
+ const _errHandle = (_err) => {
+ const statusErr = [-1, -2].includes(_err.status);
+ const requestErrorRetryHandle = this.options.requestErrorRetryHandle || (() => true);
+ return statusErr && requestErrorRetryHandle(_err);
+ };
+ if (_errHandle(err)) return true;
+ return false;
+ }
+ });
+ } else {
+ this.request = request.bind(this);
+ }
+
+ return await this.request(params);
+};
+
+async function request(params) {
  const reqParams = createRequest.call(this, params);
 
  if (!this.options.useFetch) {

lib/client.js

@@ -16,6 +16,7 @@ const { createRequest } = require('./common/utils/createRequest');
 const { encoder } = require('./common/utils/encoder');
 const { getReqUrl } = require('./common/client/getReqUrl');
 const { setSTSToken } = require('./common/utils/setSTSToken');
+const { retry } = require('./common/utils/retry');
 
 const globalHttpAgent = new AgentKeepalive();
 const globalHttpsAgent = new HttpsAgentKeepalive();
@@ -165,7 +166,28 @@ proto.authorization = function authorization(method, resource, subres, headers)
  * @api private
  */
 
-proto.request = async function request(params) {
+proto.request = async function (params) {
+ const isAvailableStream = params.stream ? params.stream.readable : true;
+ if (this.options.retryMax && isAvailableStream) {
+ this.request = retry(request.bind(this), this.options.retryMax, {
+ errorHandler: (err) => {
+ const _errHandle = (_err) => {
+ const statusErr = [-1, -2].includes(_err.status);
+ const requestErrorRetryHandle = this.options.requestErrorRetryHandle || (() => true);
+ return statusErr && requestErrorRetryHandle(_err);
+ };
+ if (_errHandle(err)) return true;
+ return false;
+ }
+ });
+ } else {
+ this.request = request.bind(this);
+ }
+
+ return await this.request(params);
+};
+
+async function request(params) {
  const reqParams = createRequest.call(this, params);
  let result;
  let reqErr;

lib/common/utils/retry.d.ts

@@ -0,0 +1 @@
+export declare function retry(this: any, func: Function, retryMax: number, config?: any): (...arg: any[]) => Promise<unknown>;

lib/common/utils/retry.js

@@ -0,0 +1,30 @@
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.retry = void 0;
+function retry(func, retryMax, config = {}) {
+ let retryNum = 0;
+ const { retryDelay = 500, errorHandler = () => true } = config;
+ const funcR = (...arg) => {
+ return new Promise((resolve, reject) => {
+ func(...arg)
+ .then(result => {
+ retryNum = 0;
+ resolve(result);
+ })
+ .catch(err => {
+ if (retryNum < retryMax && errorHandler(err)) {
+ retryNum++;
+ setTimeout(() => {
+ resolve(funcR(...arg));
+ }, retryDelay);
+ }
+ else {
+ retryNum = 0;
+ reject(err);
+ }
+ });
+ });
+ };
+ return funcR;
+}
+exports.retry = retry;

lib/common/utils/retry.ts

@@ -0,0 +1,26 @@
+export function retry(this: any, func: Function, retryMax: number, config: any = {}) {
+ let retryNum = 0;
+ const { retryDelay = 500, errorHandler = () => true } = config;
+ const funcR = (...arg) => {
+ return new Promise((resolve, reject) => {
+ func(...arg)
+ .then(result => {
+ retryNum = 0;
+ resolve(result);
+ })
+ .catch(err => {
+ if (retryNum < retryMax && errorHandler(err)) {
+ retryNum++;
+ setTimeout(() => {
+ resolve(funcR(...arg));
+ }, retryDelay);
+ } else {
+ retryNum = 0;
+ reject(err);
+ }
+ });
+ });
+ };
+
+ return funcR;
+}

package.json

@@ -21,8 +21,8 @@
  },
  "scripts": {
  "build-change-log": "standard-version",
- "test": "mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js",
- "test-cov": "nyc --reporter=lcov node_modules/.bin/_mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js",
+ "test": "mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js test/node/**/*.test.js",
+ "test-cov": "nyc --reporter=lcov node_modules/.bin/_mocha -t 60000 -r thunk-mocha -r should test/node/*.test.js test/node/**/*.test.js",
  "jshint": "jshint .",
  "autod": "autod",
  "build-test": "MINIFY=1 node browser-build.js > test/browser/build/aliyun-oss-sdk.min.js && node task/browser-test-build.js > test/browser/build/tests.js",

test/browser/browser.test.js

@@ -1555,7 +1555,7 @@ describe('browser', () => {
  const store = oss(ossConfig);
  const name = `${prefix}put/skew_date`;
  const body = Buffer.from('body');
- const requestSpy = sinon.spy(store, 'request');
+ const requestSpy = sinon.spy(store.urllib, 'request');
  const requestErrorSpy = sinon.spy(store, 'requestError');
 
  timemachine.config({
@@ -1575,15 +1575,16 @@ describe('browser', () => {
 
  const resultDel = await store.delete(name);
  assert.equal(resultDel.res.status, 204);
-
+ store.urllib.request.restore();
+ store.requestError.restore();
  timemachine.reset();
  });
 
 
  it('date is skew, put file will retry', async () => {
  const store = oss(ossConfig);
  const name = `${prefix}put/skew_date_file`;
- const requestSpy = sinon.spy(store, 'request');
+ const requestSpy = sinon.spy(store.urllib, 'request');
  const requestErrorSpy = sinon.spy(store, 'requestError');
  const fileContent = Array(1024 * 1024).fill('a').join('');
  const file = new File([fileContent], 'skew_date_file');
@@ -1605,7 +1606,8 @@ describe('browser', () => {
 
  const resultDel = await store.delete(name);
  assert.equal(resultDel.res.status, 204);
-
+ store.urllib.request.restore();
+ store.requestError.restore();
  timemachine.reset();
  });
  });
@@ -1739,4 +1741,76 @@ describe('browser', () => {
  assert.equal(info.meta.b, latin1_content);
  });
  });
+
+ describe('test/retry.test.js', () => {
+ let store;
+ const RETRY_MAX = 3;
+ let testRetryCount = 0;
+ let autoRestoreWhenRETRY_LIMIE = true;
+
+ let ORIGIN_REQUEST;
+ const mock = () => {
+ store.urllib.request = () => {
+ const e = new Error('NetError');
+ e.status = -1;
+ throw e;
+ };
+ };
+ const restore = () => {
+ store.urllib.request = ORIGIN_REQUEST;
+ };
+
+ before(async () => {
+ const ossConfigz = {
+ region: stsConfig.region,
+ accessKeyId: stsConfig.Credentials.AccessKeyId,
+ accessKeySecret: stsConfig.Credentials.AccessKeySecret,
+ stsToken: stsConfig.Credentials.SecurityToken,
+ bucket: stsConfig.bucket,
+ retryMax: RETRY_MAX,
+ requestErrorRetryHandle: () => {
+ testRetryCount++;
+ if (testRetryCount === RETRY_MAX && autoRestoreWhenRETRY_LIMIE) {
+ restore();
+ }
+ return true;
+ }
+ };
+ store = oss(ossConfigz);
+ ORIGIN_REQUEST = store.urllib.request;
+ });
+ beforeEach(() => {
+ testRetryCount = 0;
+ autoRestoreWhenRETRY_LIMIE = true;
+ mock();
+ });
+ afterEach(() => {
+ restore();
+ });
+
+ it('set retryMax to test request auto retry when networkError or timeout', async () => {
+ const res = await store.list();
+ assert.strictEqual(res.res.status, 200);
+ assert.strictEqual(testRetryCount, RETRY_MAX);
+ });
+
+ it('should throw when retry count bigger than options retryMax', async () => {
+ autoRestoreWhenRETRY_LIMIE = false;
+ try {
+ await store.list();
+ assert(false, 'should throw error');
+ } catch (error) {
+ assert(error.status === -1);
+ }
+ });
+
+ it('should succeed when put with filename', async () => {
+ const name = `ali-oss-test-retry-file-${Date.now()}`;
+ const res = await store.put(name, new File([1, 2, 3, 4, 5, 6, 7], name));
+ assert.strictEqual(res.res.status, 200);
+ assert.strictEqual(testRetryCount, RETRY_MAX);
+ const onlineFile = await store.get(name);
+ assert.strictEqual(onlineFile.content.toString(), '1234567');
+ });
+ });
 });

test/node/utils/retry.test.js

@@ -0,0 +1,86 @@
+const assert = require('assert');
+const OSS = require('../../..');
+const config = require('../../config').oss;
+const utils = require('../utils');
+const { md5 } = require('utility');
+const mm = require('mm');
+const fs = require('fs');
+const { after } = require('mocha');
+
+describe('test/retry.test.js', () => {
+ let store;
+ const RETRY_MAX = 3;
+ let testRetryCount = 0;
+ let autoRestoreWhenRETRY_LIMIE = true;
+ const bucket = `ali-oss-test-bucket-${utils.prefix.replace(/[/.]/g, '-').replace(/-$/, '')}`;
+ before(async () => {
+ store = new OSS({
+ ...config,
+ retryMax: RETRY_MAX,
+ requestErrorRetryHandle: () => {
+ testRetryCount++;
+ if (testRetryCount === RETRY_MAX && autoRestoreWhenRETRY_LIMIE) {
+ mm.restore();
+ }
+ return true;
+ }
+ });
+ const result = await store.putBucket(bucket);
+ assert.strictEqual(result.bucket, bucket);
+ assert.strictEqual(result.res.status, 200);
+ store.useBucket(bucket);
+ });
+ beforeEach(() => {
+ testRetryCount = 0;
+ autoRestoreWhenRETRY_LIMIE = true;
+ mm.error(store.urllib, 'request', {
+ status: -1, // timeout
+ headers: {},
+ });
+ });
+ afterEach(() => {
+ mm.restore();
+ });
+ after(async () => {
+ await utils.cleanBucket(store, bucket);
+ });
+
+ it('set retryMax to test request auto retry when networkError or timeout', async () => {
+ const res = await store.listBuckets();
+ assert.strictEqual(res.res.status, 200);
+ assert.strictEqual(testRetryCount, RETRY_MAX);
+ });
+
+ it('should throw when retry count bigger than options retryMax', async () => {
+ autoRestoreWhenRETRY_LIMIE = false;
+ try {
+ await store.listBuckets();
+ assert(false, 'should throw error');
+ } catch (error) {
+ assert(error.status === -1);
+ }
+ });
+
+ it('should succeed when put with filename', async () => {
+ const name = `ali-oss-test-retry-file-${Date.now()}`;
+ const fileName = await utils.createTempFile(name, 1 * 1024);
+ const res = await store.put(name, fileName);
+ assert.strictEqual(res.res.status, 200);
+ assert.strictEqual(testRetryCount, RETRY_MAX);
+ const onlineFile = await store.get(name);
+ assert.strictEqual(md5(fs.readFileSync(fileName)), md5(onlineFile.content));
+ });
+
+ it('should fail when putStream', async () => {
+ autoRestoreWhenRETRY_LIMIE = false;
+ const name = `ali-oss-test-retry-file-${Date.now()}`;
+ const fileName = await utils.createTempFile(name, 1 * 1024);
+ try {
+ await store.putStream(name, fs.createReadStream(fileName));
+ assert(false, 'should not reach here');
+ } catch (e) {
+ assert.strictEqual(e.status, -1);
+ }
+ });
+
+});