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
dequelabs · Feb 3, 2020 · b406b1f · b406b1f
1 parent e559be0
commit b406b1f
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 44 deletions.
diff --git 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
@@ -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;
+				});
 		});
 	});