Patch infinite loop of S3 region switch

carboneio · Feb 26, 2024 · bc59569 · bc59569
1 parent ad7990d
commit bc59569
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+### v3.1.0
+- S3 methods patched: A request can only request one time each region.
+
 ### v3.0.1
 - Fixed SWIFT authentication: the `region` value is case insensitive, it can be `GRA` or `gra`.
 

diff --git a/s3.js b/s3.js
@@ -277,7 +277,9 @@ module.exports = (config) => {
    */
   function request (method, path, options, callback) {
 
-    if (_config.activeStorage >= _config.storages.length) {
+    if (!options?.try) { options.try = 0 }
+
+    if (_config.activeStorage >= _config.storages.length || options.try >= _config.storages.length) {
       /** Reset the index of the main storage if any storage are available */
       _config.activeStorage = 0;
       log(`S3 Storage | All storages are not available - switch to the main storage`, 'error');
@@ -346,6 +348,7 @@ module.exports = (config) => {
         if (options.originalStorage === _config.activeStorage) {
           log(`S3 Storage | Activate fallback storage: switch from "${_config.activeStorage}" to "${_config.activeStorage + 1}" | ${err?.toString() || "Status code: " + res?.statusCode}`, 'warning');
           _config.activeStorage += 1;
+          options.try += 1;
         }
         return request(method, path, options, callback);
       } else if (err) {

diff --git a/tests/s3.test.js b/tests/s3.test.js
@@ -833,7 +833,7 @@ describe('S3 SDK', function () {
       })
 
       it('should download a file from the second storage if the authentication on the main storage is not allowed', function(done) {
-        let nockRequestS1 = nock(url1S3)
+        const nockRequestS1 = nock(url1S3)
           .get('/bucket/file.docx')
           .reply(401, 'Unauthorized')
 
@@ -860,7 +860,7 @@ describe('S3 SDK', function () {
       })
 
       it('should download a file as Stream from the second storage if the authentication on the main storage is not allowed', function(done) {
-        let nockRequestS1 = nock(url1S3)
+        const nockRequestS1 = nock(url1S3)
           .get('/bucket/file.docx')
           .reply(401, 'Unauthorized')
 
@@ -888,7 +888,7 @@ describe('S3 SDK', function () {
 
       it('should download a file from the second storage if the main storage timeout', function(done) {
         storage.setTimeout(200);
-        let nockRequestS1 = nock(url1S3)
+        const nockRequestS1 = nock(url1S3)
           .get('/bucket/file.docx')
           .delayConnection(500)
           .reply(200, {});
@@ -916,7 +916,7 @@ describe('S3 SDK', function () {
       })
 
       it('should download a file from the second storage if the main storage returns any kind of error', function(done) {
-        let nockRequestS1 = nock(url1S3)
+        const nockRequestS1 = nock(url1S3)
           .get('/bucket/file.docx')
           .replyWithError('Error Message 1234');
 
@@ -966,6 +966,32 @@ describe('S3 SDK', function () {
           done();
         })
       })
+
+      it('should return an error and should not create an infinite loop of retry', function(done) {
+        storage.setTimeout(200);
+        const nockRequestS1 = nock(url1S3)
+          .get('/bucket/file.docx')
+          .replyWithError(new Error('ERR_STREAM_PREMATURE_CLOSE'))
+          .get('/')
+          .reply(200)
+          .get('/bucket/file.docx')
+          .replyWithError(new Error('TIMEOUT'))
+        const nockRequestS2 = nock(url2S3)
+          .get('/bucket/file.docx')
+          .delayConnection(400)
+          .replyWithError(new Error('TIMEOUT'));
+
+
+        storage.downloadFile('bucket', 'file.docx', function (err, resp) {
+          assert.strictEqual(err.toString(), 'Error: All S3 storages are not available');
+          assert.strictEqual(resp, undefined);
+          const _config = storage.getConfig();
+          assert.strictEqual(_config.activeStorage, 0);
+          assert.strictEqual(nockRequestS1.pendingMocks().length, 0);
+          assert.strictEqual(nockRequestS2.pendingMocks().length, 0);
+          done();
+        })
+      })
     });
 
     describe("PARALLEL REQUESTS", function () {
@@ -2558,6 +2584,8 @@ describe('S3 SDK', function () {
 
       const nockRequestS1 = nock(url1S3)
         .intercept("/bucket", "HEAD")
+        .reply(500, "")
+        .intercept("/", "GET")
         .reply(500, "");
       const nockRequestS2 = nock(url2S3)
         .intercept("/bucket", "HEAD")