From 2b6fa142b97d0edf836c2d14b481c10e5c58af8e Mon Sep 17 00:00:00 2001 From: Tony Jin Date: Fri, 12 Jan 2018 17:00:19 -0800 Subject: [PATCH] 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. --- src/lib/Preview.js | 24 +++++++++++++------ src/lib/__tests__/Preview-test.js | 40 +++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/lib/Preview.js b/src/lib/Preview.js index ecad67bb3..7e03a1d99 100644 --- 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 index 05342216c..fa794cf8c 100644 --- 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()', () => {