From f146643c75f0f69fe7e2f279ce8e1ef8296359df Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Thu, 12 Mar 2020 17:37:47 +0000 Subject: [PATCH 1/2] Delay removal of iframe to prevent Chrome hanging status bug #240 --- __tests__/utils.test.ts | 39 ++++++++++++--------------------------- src/constants.ts | 5 +++++ src/utils.ts | 12 ++++++++++-- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts index 9b3796b80..3904c3fab 100644 --- a/__tests__/utils.test.ts +++ b/__tests__/utils.test.ts @@ -502,8 +502,9 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); + jest.useFakeTimers(); await runIframe(url, origin); - + jest.runAllTimers(); expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.appendChild).toHaveBeenCalledWith(iframe); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); @@ -524,20 +525,10 @@ describe('utils', () => { ].forEach(m => { it(`ignores invalid messages: ${JSON.stringify(m)}`, async () => { const { iframe, url, origin } = setup(m); - /** - * We need to run the timers after we start `runIframe` to simulate - * the window event listener, but we also need to use `jest.useFakeTimers` - * to trigger the timeout. That's why we're using a real `setTimeout`, - * then using fake timers then rolling back to real timers - */ - setTimeout(() => { - jest.runAllTimers(); - }, 10); jest.useFakeTimers(); - await expect(runIframe(url, origin)).rejects.toMatchObject( - TIMEOUT_ERROR - ); - jest.useRealTimers(); + const promise = runIframe(url, origin); + jest.runAllTimers(); + await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); }); @@ -553,9 +544,11 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); + jest.useFakeTimers(); await expect(runIframe(url, origin)).resolves.toMatchObject( message.data.response ); + jest.runAllTimers(); expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); @@ -570,30 +563,22 @@ describe('utils', () => { } }; const { iframe, url } = setup(message); + jest.useFakeTimers(); await expect(runIframe(url, origin)).rejects.toMatchObject( message.data.response ); + jest.runAllTimers(); expect(message.source.close).toHaveBeenCalled(); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); }); it('times out after timeoutInSeconds', async () => { const { iframe, url, origin } = setup(''); const seconds = 10; - /** - * We need to run the timers after we start `runIframe` to simulate - * the window event listener, but we also need to use `jest.useFakeTimers` - * to trigger the timeout. That's why we're using a real `setTimeout`, - * then using fake timers then rolling back to real timers - */ - setTimeout(() => { - jest.runTimersToTime(seconds * 1000); - }, 10); jest.useFakeTimers(); - await expect(runIframe(url, origin, seconds)).rejects.toMatchObject( - TIMEOUT_ERROR - ); + const promise = runIframe(url, origin, seconds); + jest.runTimersToTime(seconds * 1000); + await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR); expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe); - jest.useRealTimers(); }); }); describe('getCrypto', () => { diff --git a/src/constants.ts b/src/constants.ts index 6891215e9..0ef8f68bf 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -3,6 +3,11 @@ */ export const DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS = 60; +/** + * @ignore + */ +export const CLEANUP_IFRAME_TIMEOUT_IN_SECONDS = 2; + /** * @ignore */ diff --git a/src/utils.ts b/src/utils.ts index d07179aee..94f90c0f1 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,6 +1,9 @@ import fetch from 'unfetch'; -import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants'; +import { + DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS, + CLEANUP_IFRAME_TIMEOUT_IN_SECONDS +} from './constants'; const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i); @@ -54,7 +57,12 @@ export const runIframe = ( e.data.response.error ? rej(e.data.response) : res(e.data.response); clearTimeout(timeoutSetTimeoutId); window.removeEventListener('message', iframeEventHandler, false); - window.document.body.removeChild(iframe); + // Delay the removal of the iframe to prevent hanging loading status + // in Chrome: https://github.com/auth0/auth0-spa-js/issues/240 + setTimeout( + () => window.document.body.removeChild(iframe), + CLEANUP_IFRAME_TIMEOUT_IN_SECONDS * 1000 + ); }; window.addEventListener('message', iframeEventHandler, false); window.document.body.appendChild(iframe); From 3b1df6468a2dcf406bfdbbf4f484973fa787cc45 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Mon, 16 Mar 2020 12:33:51 +0000 Subject: [PATCH 2/2] Stub Date to fix flakey test --- __tests__/cache.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/__tests__/cache.test.ts b/__tests__/cache.test.ts index 7948100c7..8b876d383 100644 --- a/__tests__/cache.test.ts +++ b/__tests__/cache.test.ts @@ -2,6 +2,24 @@ import Cache from '../src/cache'; describe('cache', () => { let cache: Cache; + let OriginalDate: Date; + + beforeEach(() => { + OriginalDate = (global).Date; + (global).Date = class { + time: number; + constructor(time: number) { + this.time = time; + } + getTime() { + return this.time || 0; + } + }; + }); + afterEach(() => { + (global).Date = OriginalDate; + }); + beforeEach(() => { cache = new Cache(); jest.useFakeTimers();