Update: Respect 'Retry-After' header if present (#582)

When fetching for file info, respect 'Retry-After' header if present. Otherwise, retry with exponential back-off.
box · Jan 13, 2018 · 2b6fa14 · 2b6fa14
1 parent afc0844
commit 2b6fa14
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 9 deletions.
diff --git a/src/lib/Preview.js b/src/lib/Preview.js
@@ -48,12 +48,12 @@ import './Preview.scss';
 
 const DEFAULT_DISABLED_VIEWERS = ['Office']; // viewers disabled by default
 const PREFETCH_COUNT = 4; // number of files to prefetch
-const MOUSEMOVE_THROTTLE = 1500; // for showing or hiding the navigation icons
-const RETRY_TIMEOUT = 500; // retry network request interval for a file
+const MOUSEMOVE_THROTTLE_MS = 1500; // for showing or hiding the navigation icons
 const RETRY_COUNT = 5; // number of times to retry network request for a file
 const KEYDOWN_EXCEPTIONS = ['INPUT', 'SELECT', 'TEXTAREA']; // Ignore keydown events on these elements
-const LOG_RETRY_TIMEOUT = 500; // retry interval for logging preview event
+const LOG_RETRY_TIMEOUT_MS = 500; // retry interval for logging preview event
 const LOG_RETRY_COUNT = 3; // number of times to retry logging preview event
+const MS_IN_S = 1000; // ms in a sec
 
 // All preview assets are relative to preview.js. Here we create a location
 // object that mimics the window location object and points to where
@@ -1121,7 +1121,7 @@ class Preview extends EventEmitter {
                 clearTimeout(this.logRetryTimeout);
                 this.logRetryTimeout = setTimeout(() => {
                     this.logPreviewEvent(fileId, options);
-                }, LOG_RETRY_TIMEOUT * this.logRetryCount);
+                }, LOG_RETRY_TIMEOUT_MS * this.logRetryCount);
             });
     }
 
@@ -1153,9 +1153,19 @@ class Preview extends EventEmitter {
         }
 
         clearTimeout(this.retryTimeout);
+
+        // Respect 'Retry-After' header if present, otherwise retry using exponential backoff
+        let timeoutMs = 2 ** this.retryCount * MS_IN_S;
+        if (err.headers) {
+            const retryAfterS = parseInt(err.headers.get('Retry-After'), 10);
+            if (!Number.isNaN(retryAfterS)) {
+                timeoutMs = retryAfterS * MS_IN_S;
+            }
+        }
+
         this.retryTimeout = setTimeout(() => {
             this.load(this.file.id);
-        }, RETRY_TIMEOUT * this.retryCount);
+        }, timeoutMs);
     }
 
     /**
@@ -1327,9 +1337,9 @@ class Preview extends EventEmitter {
                     if (this.container) {
                         this.container.classList.remove(CLASS_NAVIGATION_VISIBILITY);
                     }
-                }, MOUSEMOVE_THROTTLE);
+                }, MOUSEMOVE_THROTTLE_MS);
             },
-            MOUSEMOVE_THROTTLE - 500,
+            MOUSEMOVE_THROTTLE_MS - 500,
             true
         );
     }

diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js
@@ -11,7 +11,6 @@ import { API_HOST, CLASS_NAVIGATION_VISIBILITY } from '../constants';
 
 const tokens = require('../tokens');
 
-const RETRY_TIMEOUT = 500; // retry network request interval for a file
 const PREFETCH_COUNT = 4; // number of files to prefetch
 const MOUSEMOVE_THROTTLE = 1500; // for showing or hiding the navigation icons
 const KEYDOWN_EXCEPTIONS = ['INPUT', 'SELECT', 'TEXTAREA']; // Ignore keydown events on these elements
@@ -1789,9 +1788,46 @@ describe('lib/Preview', () => {
             preview.handleFetchError(stubs.error);
             expect(stubs.triggerError).to.not.be.called;
 
-            clock.tick(RETRY_TIMEOUT + 1);
+            clock.tick(2001);
             expect(stubs.load).to.be.calledWith(1);
         });
+
+        it('should retry using exponential backoff', () => {
+            preview.file = {
+                id: '0'
+            };
+            const clock = sinon.useFakeTimers();
+            preview.open = true;
+            preview.retryCount = 3;
+
+            preview.handleFetchError(stubs.error);
+
+            clock.tick(7000);
+            expect(stubs.load).to.not.be.called;
+
+            clock.tick(8001);
+            expect(stubs.load).to.be.called;
+        });
+
+        it('should retry after length specified in Retry-After header if set', () => {
+            preview.file = {
+                id: '0'
+            };
+            stubs.error.headers = {
+                get: sandbox.stub().withArgs('Retry-After').returns(5)
+            }
+            const clock = sinon.useFakeTimers();
+            preview.open = true;
+            preview.retryCount = 1;
+
+            preview.handleFetchError(stubs.error);
+
+            clock.tick(4000);
+            expect(stubs.load).to.not.be.called;
+
+            clock.tick(5001);
+            expect(stubs.load).to.be.called;
+        });
     });
 
     describe('triggerError()', () => {