Skip to content

Commit

Permalink
Fix Lighthouse timeout on pages with video players
Browse files Browse the repository at this point in the history
Currently, Lighthouse will consitently timeout when run against any
website page that uses our VideoPlayer module. This is because of a
potential infinite loop in its JS code that can be triggered by a
certain Lighthouse test. This commit fixes that bug and allows for
successful audits of these pages with Lighthouse.

See here [0] for an example of a failing Lighthouse test on one of these
pages [1].

The reason for the failures is down in the Lighthouse code -- currently
LH runs an audit called redirects-http that verifies that a request to a
URL with HTTP always gets redirected to HTTPS. Instead of using a HEAD
request or something similar to check this, it tries making the request
in Chrome. To speed up what is an expected redirect, the request
disables the loading of certain assets in Chrome, including images [2].

Image loading being disabled seems to trigger some error handling code
we have in the VideoPlayer: if the video thumbnail image fails to load,
we load a default thumbnail. Currently, though, this is implemented with
the 'error' listener of the <img> tag, which means that if _all_ image
loads are disabled, there will be an infinite loop of errors:

- Try loading the correct thumbnail; this fails so
- Error handler tries to load the default thumbnail; this fails so
- Error handler tries to load the default thumbnail; this fails so
- Error handler tries to load the default thumbnail; this fails so
- etc.

This causes Lighthouse to timeout when run in the console and
essentially hang forever when run in the browser.

This commit changes the logic so that we don't try loading the default
thumbnail if we've already tried and failed to do so once.

To test this change, first verify that Lighthouse hangs against
production:

  npx lighthouse https://www.consumerfinance.gov/about-us/blog/economic-impact-payment-prepaid-card/ --only-audits=redirects-http

Then run a local server with this change (first build using `yarn run
gulp scripts`) and confirm that it works properly:

  npx lighthouse http://localhost:8000/about-us/blog/economic-impact-payment-prepaid-card/ --only-audits=redirects-http

[0] https://github.com/cfpb/cfgov-lighthouse/runs/3715284303?check_suite_focus=true
[1] https://www.consumerfinance.gov/about-us/blog/economic-impact-payment-prepaid-card/
[2] https://github.com/GoogleChrome/lighthouse/blob/a691b25aff50f5b0790f661c16469b1a3a134294/lighthouse-core/config/default-config.js#L179
  • Loading branch information
chosak committed Sep 27, 2021
1 parent d601aa9 commit aac8345
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions cfgov/unprocessed/js/organisms/VideoPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function VideoPlayer( element ) {
return UNDEFINED;
}

// Load a default thumbnail image if we're not using a custom one.
// Load the thumbnail from YouTube if we haven't specified one in the DOM.
if ( _videoId && !_showCustomThumbnail ) {
_imageLoad( youTubeAPI.fetchImageURL( _videoId ) );
}
Expand Down Expand Up @@ -134,11 +134,23 @@ function VideoPlayer( element ) {
*/
function _imageLoad( imageURL ) {
_imageDom.addEventListener( 'load', _imageLoaded );
_imageDom.addEventListener( 'error', _imageLoadDefault );
_imageDom.addEventListener( 'error', _imageLoadFailed );

_imageDom.src = imageURL;
}

/**
* Error handler for thumbnail image loading.
* If for some reason we can't load the specified thumbnail image, we
* should try to load the default thumbnail. But don't do that if we've
* already tried and failed to do so once.
*/
function _imageLoadFailed() {
if ( _imageDom.src !== _defaultThumbnailURL ) {
return _imageLoadDefault();
}
}

/**
* Event handler for when image src attribute is set.
*/
Expand Down

0 comments on commit aac8345

Please sign in to comment.