Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(iframes): timed out if lazy loaded iframes #348

Open
Kristinita opened this issue Sep 16, 2022 · 2 comments
Open

bug(iframes): timed out if lazy loaded iframes #348

Kristinita opened this issue Sep 16, 2022 · 2 comments

Comments

@Kristinita
Copy link

Kristinita commented Sep 16, 2022

#257 — possibly related issue

1. Summary

If I use native lazy loading for iframes in my HTML files, Penthouse doesn’t create critical CSS for these files. I get a stack trace:

/home/travis/build/Kristinita/SashaTravis/node_modules/penthouse/lib/core.js:342

		error: new Error('Penthouse timed out after ' + timeout / 1000 + 's. ')

			   ^

Error: Penthouse timed out after 30s.

	at Timeout.<anonymous> (/home/travis/build/Kristinita/SashaTravis/node_modules/penthouse/lib/core.js:342:16)

	at listOnTimeout (node:internal/timers:564:17)

	at process.processTimers (node:internal/timers:507:7)

2. MCVE

This configuration on GitHub, Travis CI build for it.

  1. package.json:

    {
    	"dependencies": {
    		"penthouse": "^2.3.3"
    	}
    }
    
  2. KiraExamplePassed.html:

    <!DOCTYPE html>
    <html lang="ru">
    	<head>
    		<meta charset="utf-8">
    		<meta name="viewport" content="width=device-width, initial-scale=1">
    		<title>Penthouse + lazy-loaded iframes MCVE — passed</title>
    		<!-- [INFO] In real files I have HTML markup before the iframe instead of “margin-bottom” -->
    		<style>
    			.KiraExampleClass { margin-bottom: 1rem; }
    		</style>
    		<!-- [INFO] Iframes native lazy loading polyfill:
    		https://github.com/mfranzke/loading-attribute-polyfill -->
    		<script src="https://cdn.jsdelivr.net/npm/loading-attribute-polyfill/dist/loading-attribute-polyfill.umd.min.js" async></script>
    	</head>
    	<body>
    		<div class="KiraExampleClass"></div>
    		<!-- [INFO] Iframes syntax:
    		https://github.com/mfranzke/loading-attribute-polyfill#iframe -->
    		<noscript class="loading-lazy">
    			<iframe title="KiraExamplePeerTubeVideo" width="560" height="315" src="https://video.ploud.fr/videos/embed/15613130-601b-4101-bc39-2dde03256254" loading="lazy"></iframe>
    		</noscript>
    	</body>
    </html>
  3. KiraExampleFailed.html:

    <!DOCTYPE html>
    <html lang="ru">
    	<head>
    		<meta charset="utf-8">
    		<meta name="viewport" content="width=device-width, initial-scale=1">
    		<title>Penthouse + lazy-loaded iframes MCVE — passed</title>
    		<!-- [INFO] In real files I have HTML markup before the iframe instead of “margin-bottom” -->
    		<style>
    				.KiraExampleClass { margin-bottom: 500rem; }
    		</style>
    		<!-- [INFO] Iframes native lazy loading polyfill:
    		https://github.com/mfranzke/loading-attribute-polyfill -->
    		<script src="https://cdn.jsdelivr.net/npm/loading-attribute-polyfill/dist/loading-attribute-polyfill.umd.min.js" async></script>
    	</head>
    	<body>
    		<div class="KiraExampleClass"></div>
    		<!-- [INFO] Iframes syntax:
    		https://github.com/mfranzke/loading-attribute-polyfill#iframe -->
    		<noscript class="loading-lazy">
    			<iframe title="KiraExamplePeerTubeVideo" width="560" height="315" src="https://video.ploud.fr/videos/embed/15613130-601b-4101-bc39-2dde03256254" loading="lazy"></iframe>
    		</noscript>
    	</body>
    </html>
    -	.KiraExampleClass { margin-bottom: 1rem; }
    +	.KiraExampleClass { margin-bottom: 500rem; }
  4. KiraPenthousePassed.js:

    const penthouse = require('penthouse');
    const fs = require('fs');
    
    penthouse({
    	// [INFO] “url: 'file:///D:/SashaDemoRepositories/SashaTravis/KiraExamplePassed.html',” for my Windows
    	url: 'file:///home/travis/build/Kristinita/SashaTravis/KiraExamplePassed.html',
    	cssString: 'body { color: red }'
    })
    	.then(criticalCss => {
    		fs.writeFileSync('KiraOutfilePassed.css', criticalCss);
    	});
  5. KiraPenthouseFailed.js:

    const penthouse = require('penthouse');
    const fs = require('fs');
    
    penthouse({
    	// [INFO] “url: 'file:///D:/SashaDemoRepositories/SashaTravis/KiraExampleFailed.html',” for my Windows
    	url: 'file:///home/travis/build/Kristinita/SashaTravis/KiraExampleFailed.html',
    	cssString: 'body { color: red }'
    })
    	.then(criticalCss => {
    		fs.writeFileSync('KiraOutfileFailed.css', criticalCss);
    	});
    -	// [INFO] “url: 'file:///D:/SashaDemoRepositories/SashaTravis/KiraExamplePassed.html',” for my Windows
    -	url: 'file:///home/travis/build/Kristinita/SashaTravis/KiraExamplePassed.html',
    +	// [INFO] “url: 'file:///D:/SashaDemoRepositories/SashaTravis/KiraExampleFailed.html',” for my Windows
    +	url: 'file:///home/travis/build/Kristinita/SashaTravis/KiraExampleFailed.html',
    
    -	fs.writeFileSync('KiraOutfilePassed.css', criticalCss);
    +	fs.writeFileSync('KiraOutfileFailed.css', criticalCss);
  6. The part of the .travis.yml:

    install:
    - npm install
    
    script:
    - env DEBUG="penthouse,penthouse:*" node KiraPenthousePassed.js
    - env DEBUG="penthouse,penthouse:*" node KiraPenthouseFailed.js
    

3. Behavior

3.1. Desired — KiraPenthousePassed.js

Travis link:

$ env DEBUG="penthouse,penthouse:*" node KiraPenthousePassed.js

  penthouse:browser no browser instance, launching new browser.. +0ms

  penthouse:browser browser ready +250ms

  penthouse call generateCriticalCssWrapped +0ms

  penthouse:core Penthouse core start +0ms

  penthouse:core parse ast START +0ms

  penthouse:core parse ast DONE (with 0 errors) +3ms

  penthouse:preformatting:nonMatchingMediaQueryRemover BEFORE +0ms

  penthouse:preformatting:nonMatchingMediaQueryRemover matchConfig: {

  penthouse:preformatting:nonMatchingMediaQueryRemover   "type": "screen",

  penthouse:preformatting:nonMatchingMediaQueryRemover   "width": "1300px",

  penthouse:preformatting:nonMatchingMediaQueryRemover   "height": "900px"

  penthouse:preformatting:nonMatchingMediaQueryRemover }

  penthouse:preformatting:nonMatchingMediaQueryRemover keepLargerMediaQueries: false +0ms

  penthouse:core stripped out non matching media queries +1ms

  penthouse:browser re-using the page browser launched with +95ms

  penthouse:browser re-using browser page for generateCriticalCss, remaining at: 1 +0ms

  penthouse:core open page ready in browser +88ms

  penthouse:core page event listeners set +1ms

  penthouse:core userAgent set +1ms

  penthouse:core blocking js requests DONE +1ms

  penthouse:core preparePage DONE +0ms

  penthouse:core page load start +0ms

  penthouse:core turn css to formatted selectorlist START +1ms

  penthouse:preformatting:selectors-profile buildSelectorProfile START +0ms

  penthouse:preformatting:selectors-profile forceInclude body +1ms

  penthouse:preformatting:selectors-profile buildSelectorProfile DONE +0ms

  penthouse:core turn css to formatted selectorlist DONE +1ms

  penthouse:core page load DONE +2s

  penthouse:core waited for renderWaitTime: 100 +100ms

  penthouse:core pruneNonCriticalSelectors init +4ms

  penthouse:core filterSelectors START +0ms

  penthouse:core filterSelectors DONE +0ms

  penthouse:core pruneNonCriticalSelectors done +1ms

  penthouse:core AST cleanup START +0ms

  penthouse:css-cleanup start +0ms

  penthouse:css-cleanup commentRemover +0ms

  penthouse:css-cleanup ruleSelectorRemover +0ms

  penthouse:css-cleanup:unused-keyframe-remover getAllAnimationKeyframes +0ms

  penthouse:css-cleanup:unused-keyframe-remover getAllAnimationKeyframes AFTER, usedKeyFrames: 0 +3ms

  penthouse:css-cleanup unusedKeyframeRemover +3ms

  penthouse:css-cleanup:embeddedbase64Remover config: maxEmbeddedBase64Length = 1000 +0ms

  penthouse:css-cleanup embeddedbase64Remover +1ms

  penthouse:css-cleanup:unused-font-face-remover getAllFontNameValues +0ms

  penthouse:css-cleanup:unused-font-face-remover getAllFontNameValues AFTER +0ms

  penthouse:css-cleanup unusedFontFaceRemover +0ms

  penthouse:css-cleanup propertiesToRemove +0ms

  penthouse:css-cleanup finalRuleRemover +1ms

  penthouse:core AST cleanup DONE +5ms

  penthouse:core generated CSS from AST +0ms

  penthouse:core generateCriticalCss DONE +0ms

  penthouse:core cleanupAndExit +0ms

  penthouse:browser remove (maybe) browser page for generateCriticalCss, before removing was: 1 +2s

  penthouse:browser now try to close browser page +0ms

  penthouse generateCriticalCss done +2s

  penthouse:browser closed browser +18ms

The command "env DEBUG="penthouse,penthouse:*" node KiraPenthousePassed.js" exited with 0.

If my iframe at the top of my real HTML page and loads when the page is opened, I don’t get errors when using Penthouse.

3.2. Error — KiraPenthouseFailed.js

Travis link:

$ env DEBUG="penthouse,penthouse:*" node KiraPenthouseFailed.js

  penthouse:browser no browser instance, launching new browser.. +0ms

  penthouse:browser browser ready +103ms

  penthouse call generateCriticalCssWrapped +0ms

  penthouse:core Penthouse core start +0ms

  penthouse:core parse ast START +1ms

  penthouse:core parse ast DONE (with 0 errors) +2ms

  penthouse:preformatting:nonMatchingMediaQueryRemover BEFORE +0ms

  penthouse:preformatting:nonMatchingMediaQueryRemover matchConfig: {

  penthouse:preformatting:nonMatchingMediaQueryRemover   "type": "screen",

  penthouse:preformatting:nonMatchingMediaQueryRemover   "width": "1300px",

  penthouse:preformatting:nonMatchingMediaQueryRemover   "height": "900px"

  penthouse:preformatting:nonMatchingMediaQueryRemover }

  penthouse:preformatting:nonMatchingMediaQueryRemover keepLargerMediaQueries: false +0ms

  penthouse:core stripped out non matching media queries +3ms

  penthouse:browser re-using the page browser launched with +91ms

  penthouse:browser re-using browser page for generateCriticalCss, remaining at: 1 +1ms

  penthouse:core open page ready in browser +79ms

  penthouse:core page event listeners set +1ms

  penthouse:core userAgent set +2ms

  penthouse:core blocking js requests DONE +0ms

  penthouse:core preparePage DONE +0ms

  penthouse:core page load start +0ms

  penthouse:core turn css to formatted selectorlist START +1ms

  penthouse:preformatting:selectors-profile buildSelectorProfile START +0ms

  penthouse:preformatting:selectors-profile forceInclude body +0ms

  penthouse:preformatting:selectors-profile buildSelectorProfile DONE +0ms

  penthouse:core turn css to formatted selectorlist DONE +1ms

  penthouse:core cleanupAndExit +30s

  penthouse:browser remove (maybe) browser page for generateCriticalCss, before removing was: 1 +30s

  penthouse:browser now try to close browser page +0ms

  penthouse:browser closed browser +18ms

/home/travis/build/Kristinita/SashaTravis/node_modules/penthouse/lib/core.js:342

		error: new Error('Penthouse timed out after ' + timeout / 1000 + 's. ')

			   ^

Error: Penthouse timed out after 30s.

	at Timeout.<anonymous> (/home/travis/build/Kristinita/SashaTravis/node_modules/penthouse/lib/core.js:342:16)

	at listOnTimeout (node:internal/timers:564:17)

	at process.processTimers (node:internal/timers:507:7)

Node.js v18.9.0

The command "env DEBUG="penthouse,penthouse:*" node KiraPenthouseFailed.js" exited with 1.

Else my iframe is in the middle or end of my real HTML page and loads lazy, I get this stack trace when I use Penthouse.

4. Environment

  1. Operating system:

    1. Local — Microsoft Windows [Version 10.0.19041.1415]
    2. Travis CI — Ubuntu 22.04 LTS Jammy Jellyfish
  2. Node.js v18.9.0

  3. Penthouse 2.3.3

Thanks.

@pocketjoso
Copy link
Owner

Thanks a lot for the detail bug report, and the reproducible test case - I can reproduce the problem.

I will do my best to find time to look into this, but I wouldn't count on myself finding a fix in the near term. My first hunch is that - as you linked to issue #257 - that this is a problem from puppeteer, the version that is currently used. So what I would do next is to try to upgrade the puppeteer version, and see if that fixes the problem.

@Kristinita
Copy link
Author

This issue is still relevant in December 2023.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants