From b406b1fd09d72f9193d5b4011fa6f24bd33e3576 Mon Sep 17 00:00:00 2001 From: Jey Nandakumar Date: Mon, 3 Feb 2020 20:09:13 +0000 Subject: [PATCH] fix(preload): reject promise `axe.utils.preload` when XHR fails (#2009) * fix: reject on preload assets * remove clearTimeout * update url for non existing css * change the way non-existing url is loaded * Number.isNaN is not supported in IE * mock preloadCssom for testing timeout --- lib/core/utils/preload.js | 54 ++++++++------ test/integration/full/preload/preload.js | 90 ++++++++++++++++++------ 2 files changed, 100 insertions(+), 44 deletions(-) diff --git a/lib/core/utils/preload.js b/lib/core/utils/preload.js index e4cba60c80..8e42aaad49 100644 --- a/lib/core/utils/preload.js +++ b/lib/core/utils/preload.js @@ -66,7 +66,7 @@ axe.utils.getPreloadConfig = function getPreloadConfig(options) { if ( options.preload.timeout && typeof options.preload.timeout === 'number' && - !Number.isNaN(options.preload.timeout) + !isNaN(options.preload.timeout) ) { config.timeout = options.preload.timeout; } @@ -97,12 +97,14 @@ axe.utils.preload = function preload(options) { * Start `timeout` timer for preloading assets * -> reject if allowed time expires. */ - setTimeout(() => reject(`Preload assets timed out.`), timeout); + const preloadTimeout = setTimeout( + () => reject(new Error(`Preload assets timed out.`)), + timeout + ); /** * Fetch requested `assets` */ - Promise.all( assets.map(asset => preloadFunctionsMap[asset](options).then(results => { @@ -111,26 +113,32 @@ axe.utils.preload = function preload(options) { }; }) ) - ).then(results => { - /** - * Combine array of results into an object map - * - * From -> - * [{cssom: [...], aom: [...]}] - * To -> - * { - * cssom: [...] - * aom: [...] - * } - */ - const preloadAssets = results.reduce((out, result) => { - return { - ...out, - ...result - }; - }, {}); + ) + .then(results => { + /** + * Combine array of results into an object map + * + * From -> + * [{cssom: [...], aom: [...]}] + * To -> + * { + * cssom: [...] + * aom: [...] + * } + */ + const preloadAssets = results.reduce((out, result) => { + return { + ...out, + ...result + }; + }, {}); - resolve(preloadAssets); - }); + clearTimeout(preloadTimeout); + resolve(preloadAssets); + }) + .catch(err => { + clearTimeout(preloadTimeout); + reject(err); + }); }); }; diff --git a/test/integration/full/preload/preload.js b/test/integration/full/preload/preload.js index 82b308cbfa..4c72e9f3da 100644 --- a/test/integration/full/preload/preload.js +++ b/test/integration/full/preload/preload.js @@ -1,4 +1,5 @@ -/* global axe */ +/* global axe, Promise */ + describe('axe.utils.preload integration test', function() { 'use strict'; @@ -8,6 +9,10 @@ describe('axe.utils.preload integration test', function() { id: 'crossOriginLinkHref', href: 'https://unpkg.com/gutenberg-css@0.4' }, + crossOriginDoesNotExist: { + id: 'styleTag', + text: '@import "https://i-do-not-exist.css"' + }, crossOriginLinkHrefMediaPrint: { id: 'crossOriginLinkHrefMediaPrint', href: @@ -70,17 +75,19 @@ describe('axe.utils.preload integration test', function() { if (err) { done(err); } - getPreload().then(function(preloadedAssets) { - assert.property(preloadedAssets, 'cssom'); - assert.lengthOf(preloadedAssets.cssom, 1); - var sheetData = preloadedAssets.cssom[0].sheet; - axe.testUtils.assertStylesheet( - sheetData, - '.inline-css-test', - stylesForPage[0].text - ); - done(); - }); + getPreload() + .then(function(preloadedAssets) { + assert.property(preloadedAssets, 'cssom'); + assert.lengthOf(preloadedAssets.cssom, 1); + var sheetData = preloadedAssets.cssom[0].sheet; + axe.testUtils.assertStylesheet( + sheetData, + '.inline-css-test', + stylesForPage[0].text + ); + done(); + }) + .catch(done); }); }); @@ -90,24 +97,65 @@ describe('axe.utils.preload integration test', function() { if (err) { done(err); } - getPreload().then(function(preloadedAssets) { - assert.property(preloadedAssets, 'cssom'); - assert.lengthOf(preloadedAssets.cssom, 0); - done(); - }); + getPreload() + .then(function(preloadedAssets) { + assert.property(preloadedAssets, 'cssom'); + assert.lengthOf(preloadedAssets.cssom, 0); + done(); + }) + .catch(done); + }); + }); + + it('returns NO preloaded CSSOM assets when requested stylesheet does not exist`', function(done) { + stylesForPage = [styleSheets.crossOriginDoesNotExist]; + attachStylesheets({ styles: stylesForPage }, function(err) { + if (err) { + done(err); + } + getPreload() + .then(function() { + done(new Error('Not expecting to complete the promise')); + }) + .catch(function(err) { + assert.isNotNull(err); + assert.isTrue(!err.message.includes('Preload assets timed out')); + done(); + }) + .catch(done); }); }); it('rejects preload function when timed out before fetching assets', function(done) { stylesForPage = [styleSheets.crossOriginLinkHref]; + + var origPreloadCssom = axe.utils.preloadCssom; + axe.utils.preloadCssom = function() { + return new Promise(function(res) { + setTimeout(function() { + res(true); + }, 2000); + }); + }; + attachStylesheets({ styles: stylesForPage }, function(err) { if (err) { done(err); } - getPreload(1).catch(function(err) { - assert.isNotNull(err); - done(); - }); + getPreload(1) + .then(function() { + done(new Error('Not expecting to complete the promise')); + }) + .catch(function(err) { + assert.isNotNull(err); + assert.isTrue(err.message.includes('Preload assets timed out')); + + done(); + }) + .catch(done) + .finally(function() { + axe.utils.preloadCssom = origPreloadCssom; + }); }); });