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

Penthouse timed out after 30s. SyntaxError: Expected token ']' #156

Closed
MindSculpt opened this issue Mar 14, 2017 · 25 comments
Closed

Penthouse timed out after 30s. SyntaxError: Expected token ']' #156

MindSculpt opened this issue Mar 14, 2017 · 25 comments
Labels

Comments

@MindSculpt
Copy link

MindSculpt commented Mar 14, 2017

I'm running a basic gulp build using critical css. The document setup is fine as is the critical task (I'm using this for several other projects). All packages are up-to-date and using the latest, but when I run my critical task I receive the following error:

`Penthouse timed out after 30s. SyntaxError: Expected token ']'

phantomjs://platform/non-matching-media-query-remover.js:52 in loadModule
phantomjs://platform/bootstrap.js:282 in _compile
phantomjs://platform/bootstrap.js:126 in .js
phantomjs://platform/bootstrap.js:278 in _load
phantomjs://platform/bootstrap.js:311 in require
phantomjs://platform/bootstrap.js:263 in require
TypeError: Object is not a constructor (evaluating 'nonMatchingMediaQueryRemover(options.ast.stylesheet.rules, options.width, options.height)')

phantomjs://code/core.js:307 in getCriticalPathCss`

This seems like it's a syntax issue on my end, but I've linted all of my css/js, tried removing stuff piece by piece and re-running, but I'm still getting this error. I'm running other projects using the same exact gulpfile and build tasks with Bootstrap and I don't have these issues.

Any ideas on where I should be looking, or what could be causing this? I'm trying to narrow down that it's not a penthouse issue.

@pocketjoso
Copy link
Owner

Hi,
To begin with, can you share the exact Penthouse version you have installed?

Can you share the url? If so I can give a go at debugging this from my end. Otherwise if you want to look at it yourself, the function that crashes is here.

I have seen an error like this once before, but it was fixed. I believe it might be a bug in Penthouse on some edge case in your css, possibly a syntax error.

@MindSculpt
Copy link
Author

MindSculpt commented Mar 14, 2017

Sure thing. The error is thrown while using the critical plugin, and in that plugin's package.json file it lists:

"penthouse": "^0.10.9"

I'm also using version 0.8.4 of critical.

I can't share the URL because it's happening during the build process, not in production. It's interesting to note that if i pull out just about all the styles in my CSS, I still receive the error.

I would like to add that it is a one-page site, but I did some tests to make sure having only a single html page to run through critical wasn't the issue.

@pocketjoso
Copy link
Owner

I can't share the URL because it's happening during the build process, not in production.

Without a url I can't really debug this. If you could share html+css files that would work as well.

It's interesting to note that if i pull out just about all the styles in my CSS, I still receive the error.

You say you receive the error even if you comment out almost all of the css. What about if you replace the whole css file with just some generic almost empty, error free, placeholder content, like body { color: red; } - does the error go way then? If so, can you identify what part of your css causes the error?

I would like to add that it is a one-page site, but I did some tests to make sure having only a single html page to run through critical wasn't the issue.

Do you mean a site with a single page, not requiring Javascript to load, or do you mean a Single Page Application (with an empty placeholder html body that is populated via JavaScript). The latter is not suitable for critical css.

@MindSculpt
Copy link
Author

If you're looking for a production URL, you can see it here. This is the compiled version though.

Yes, the error appears even with a simple lone line of CSS, which leads me to believe it could just be an error in an external library.

It's just a simple, single page HTML website, not a single page application (like angular).

@pocketjoso
Copy link
Owner

@MindSculpt Hmm, strange; it works to generate critical css for that URL when I try it, for example in the free hosted generator. I presume that works for you as well?

Can you share the full config you're using to call penthouse with? It sounds like you must have some syntax error in your config; perhaps the url you're passing is badly formatted?

@MindSculpt
Copy link
Author

MindSculpt commented Mar 25, 2017

@pocketjoso The hosted generator works for me as well. I'm using critical in my gulp build, so I'm not using penthouse directly. Here's the gulp task that's also using cachebusting (I'm using this identical task in an identical gulp file for other projects and have no issues):

gulp.task('critical', function () {
	var cssFilename = production ? cachebust.mappings['styles.css'] : 'styles.css';
        return gulp.src(outputDir + '*.html')
        .pipe(critical({
        	base: outputDir, 
        	inline: true, 
        	minify: true, 
        	css: [outputDir + 'assets/css/'+cssFilename]
        }))
        .on('error', function(err) { gutil.log(gutil.colors.red(err.message)); })
        .pipe(gulp.dest(outputDir));
});

@pocketjoso
Copy link
Owner

@MindSculpt sounds like your problem is with your critical usage, so I would recommend opening an issue in the critical repo instead.

If you wanted to continue debugging via Penthouse directly, the next step would be to force strict: true to be passed to Penthouse, and to see the debug output. However this option doesn't seem supported via the critical package, so unless you want to hack in the code yourself, you again would need to raise it on the critical end.

Given that we know that the URL + the css found on the url really does work in Penthouse (as proven by the online tools), the error here must be coming from either: the css your passing in, the html/url, or the critical package itself.

@MindSculpt
Copy link
Author

MindSculpt commented Apr 20, 2017

@pocketjoso I wanted to follow up with you here on this as it's still an ongoing issue with all new repos I'm creating using penthouse/critical.

I removed the use of critical entirely and am now trying to just use penthouse to generate the separate file that contains just the critical css. I'm using the penthouse syntax in my gulpfile you show here and still get this error:

Error: Penthouse timed out after 30s. SyntaxError: Expected token ']'

  phantomjs://platform/non-matching-media-query-remover.js:52 in loadModule
  phantomjs://platform/bootstrap.js:282 in _compile
  phantomjs://platform/bootstrap.js:126 in .js
  phantomjs://platform/bootstrap.js:278 in _load
  phantomjs://platform/bootstrap.js:311 in require
  phantomjs://platform/bootstrap.js:263 in require
TypeError: Object is not a constructor (evaluating 'nonMatchingMediaQueryRemover(options.ast.stylesheet.rules, options.width, options.height)')

  phantomjs://code/core.js:307 in getCriticalPathCss

Seems like it's definitely something with penthouse. I am using the latest version of Node (v7.9.0). Any other ideas why this is failing? I noticed that the phantomjs-prebuilt dependency in your package.json isn't using the latest version. Not sure if that's the issue?

@pocketjoso
Copy link
Owner

pocketjoso commented Apr 21, 2017

@MindSculpt as I stated before, Penthouse succeeds generating critical css for your url (as you can verify in the free generator, which uses the latest Penthouse version: https://jonassebastianohlsson.com/criticalpathcssgenerator).

Hence, the problem is with your usage. (most likely problem with the css you pass in)

To debug, please:

  • pass strict:true in the penthouse options, to see if it finds errors in the css you're passing in. If so it will throw when you use this option.
  • set Penthouse.DEBUG = true on the imported Penthouse module, and then paste the debug logs here after running Penthouse, so we can see where the execution halts.

I need the output from the debug run to be able to help you further.

@MindSculpt
Copy link
Author

MindSculpt commented Apr 21, 2017

@pocketjoso Ok, here's the error output:

[07:11:45] Starting 'critical-css'...
time: 5 | opened css file
time: 116 | parsed ast (without errors)
[07:11:46] Finished 'critical-css' after 116 ms
[07:11:46] Finished 'critical' after 1.61 s
time: 1763 | Penthouse core start
time: 1782 | opened ast from file
time: 1795 | parsed ast from json
time: 1795 | getCriticalPathCss
time: 1795 | prepareNewPage
/Users/michael/Repos/my-website/lib/project-core/gulpfile.js:587
	        throw err;
	        ^
Error: time: 5 | opened css filetime: 116 | parsed ast (without errors)Penthouse core starttime: 1763 | Penthouse core startopened ast from filetime: 1782 | opened ast from fileparsed ast from jsontime: 1795 | parsed ast from jsongetCriticalPathCsstime: 1795 | getCriticalPathCssprepareNewPagetime: 1795 | prepareNewPagePenthouse timed out after 30s. SyntaxError: Expected token ']'

  phantomjs://platform/non-matching-media-query-remover.js:52 in loadModule
  phantomjs://platform/bootstrap.js:282 in _compile
  phantomjs://platform/bootstrap.js:126 in .js
  phantomjs://platform/bootstrap.js:278 in _load
  phantomjs://platform/bootstrap.js:311 in require
  phantomjs://platform/bootstrap.js:263 in require
TypeError: Object is not a constructor (evaluating 'nonMatchingMediaQueryRemover(options.ast.stylesheet.rules, options.width, options.height)')

  phantomjs://code/core.js:307 in getCriticalPathCss

    at ChildProcess.<anonymous> (/Users/michael/Repos/my-website/lib/project-core/node_modules/penthouse/lib/index.js:175:19)
    at emitTwo (events.js:106:13)
    at ChildProcess.emit (events.js:194:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:215:12)

And here's my critical task:

gulp.task('critical-css', function () {
	penthouse({
	    url: 'https://mywebsite.com',       // can be local html file path
	    css: outputDir + 'assets/css/styles.css',
	    // OPTIONAL params
	    width: 1300,                    // viewport width
	    height: 900,                    // viewport height
	    forceInclude: [
	      '.keepMeEvenIfNotSeenInDom',
	      /^\.regexWorksToo/
	    ],
	    timeout: 30000,                 // ms; abort critical CSS generation after this timeout
	    strict: true,                  // set to true to throw on CSS errors (will run faster if no errors)
	    maxEmbeddedBase64Length: 1000,  // characters; strip out inline base64 encoded resources larger than this
	    userAgent: 'Penthouse Critical Path CSS Generator', // specify which user agent string when loading the page
	    renderWaitTime: 100,            // ms; render wait timeout before CSS processing starts (default: 100)
	    blockJSRequests: true,          // set to false to load (external) JS (default: true)
	    phantomJsOptions: {             // see `phantomjs --help` for the list of all available options
	      'proxy': 'http://localhost:8888',
	      'ssl-protocol': 'SSLv3'
	    },
	    customPageHeaders: {
	      'Accept-Encoding': 'identity' // add if getting compression errors like 'Data corrupted'
	    }
	}, function(err, criticalCss) {
	    if (err) {
	        // handle error
	        throw err;
        	console.log(err);
	    }

	         console.log(criticalCss);
	    fs.writeFileSync('./outfile.css', criticalCss);
	});
});

And the gulp task I'm using to compile looks like:

gulp.task('build', function(callback){
  runSequence('clean:builds',
    ['js', 'sass', 'fonts'],
    'layout',
    'critical-css',
    callback);
});

Let me know if something jumps out at you.

@pocketjoso
Copy link
Owner

@MindSculpt all right, I see that there is some bug in Penthouse causing a crash with the css you're passing in. I'll will take a look at this this weekend. If you could send me the full css you're using that would help reproducing the bug, but could be that I have enough to fix the problem without - will update once I get to it.

@pocketjoso pocketjoso added bug and removed question labels Apr 21, 2017
@MindSculpt
Copy link
Author

MindSculpt commented Apr 21, 2017

@pocketjoso Whew, I knew I wasn't crazy.

All kidding aside though, I do appreciate your help and your looking into it.

Here's the sass and the uncompressed, compiled css. Live site again is here.

@pocketjoso
Copy link
Owner

pocketjoso commented Apr 21, 2017

@MindSculpt Thanks. I'm sorry I questioned your issue before and labelled it as just a question - I was blinded by the fact that it works with the css scraped from your site itself, I missed the actual bug your config triggers. My apologies.

@pocketjoso
Copy link
Owner

pocketjoso commented Apr 21, 2017

@MindSculpt running Penthouse with just your live url and your uncompressed, compiled css does not produce the bug.

  • Can you confirm that your gulp config works with Penthouse at all? I.e. can you test another url and/or css and see if it produces the same bug?

  • Are you really calling penthouse with all the optional params, including this one?

 phantomJsOptions: {             // see `phantomjs --help` for the list of all available options
	      'proxy': 'http://localhost:8888',
	      'ssl-protocol': 'SSLv3'
	    },

This will not work unless you are setup to handle the proxy. If you are not, I suggest dropping all the optional params (they are all defaults anyway, except the phantomJsOptions - I will make this more clear in the readme).
Now - does this make any difference?

If not, if we cannot setup a reproducable test case for me, you will have to go into your locally installed Penthouse, into core.js:307 as mentioned in the error, and add another debuglog invocation on the line before, to perhaps just debuglog('options: ' + JSON.stringify(options, null, 2)) - and paste what that outputs.

@MindSculpt
Copy link
Author

MindSculpt commented Apr 21, 2017

@pocketjoso I was including that entire example penthouse function. I changed it to this as requested:

gulp.task('critical-css', function () {
	penthouse({
	    url: 'https://mysite.com',       // can be local html file path
	    css: outputDir + 'assets/css/styles.css',
	    // OPTIONAL params
	   	strict: true                 // set to true to throw on CSS errors (will run faster if no errors)
	}, function(err, criticalCss) {
	    if (err) {
	        // handle error
	        throw err;
        	console.log(err);
	    }
            console.log(criticalCss);
	    fs.writeFileSync('./outfile.css', criticalCss);
	});
});

Changing the URL in the url parameter for any other URL also fails with the same error. Here's a link to the debug output per your instructions. Let me know what you come up with.

@pocketjoso
Copy link
Owner

Hi, just letting you know that I ran out of time to get down to the bottom of this last weekend - will get back to it the upcoming weekend.

@MindSculpt
Copy link
Author

MindSculpt commented Apr 25, 2017

@pocketjoso Sounds good. Keep me posted if you need any additional info from me to help you track down the issue.

@pocketjoso
Copy link
Owner

So I looked at this again, and even copying the options JSON you shared in the debug output - I still cannot reproduce your problem.

So it seems we need to continue to debug this on your end. What I would suggest next is try to isolate what part of your css causes the issue. Slow and tedious, but one way would be to split it in half, see what half produces the error, then split that half in half, rinse and repeat...

@MindSculpt
Copy link
Author

@pocketjoso Crazy. I actually already tried isolation—it's failing regardless. I'll put together a test repo for you. The lib is private, so let me know the best way to invite you to it without posting it publicly here.

@pocketjoso
Copy link
Owner

pocketjoso commented Apr 30, 2017 via email

@MindSculpt
Copy link
Author

@pocketjoso Yes — I've removed just about everything to try to narrow it down (removed CSS, reinstalled packages, etc) but I receive the same error. I'll keep at it and keep you posted.

@MindSculpt
Copy link
Author

@pocketjoso So it looks like this was a repo-specific problem. I wasn't able to nail down the exactly what it was, but new clean project repos appear to be running critical perfectly now. Thanks for looking into this, and if anything else comes up I'll reach back out.

@MindSculpt MindSculpt reopened this May 7, 2017
@MindSculpt
Copy link
Author

@pocketjoso I'm following this back up because I actually did finally figure out what the issue was. Seems like a noob mistake but one that devs might not catch right away (because it definitely didn't occur to me!).

My repo's local folder was named with an apostrophe (e.g. "My Website's Repo"), which must be tripping up penthouse. You might want to add this 'gotcha' to your docs or make sure it gets escaped in your library. Hopefully this thread will saved someone else a ton of time debugging. Or better yet, just don't name your local folders with any punctuation ;)

@pocketjoso
Copy link
Owner

Huh, thanks for coming back with this info 👍 - will definitely check out and either fix the bug or document it.

@pocketjoso
Copy link
Owner

Added info in README: 0c741b3.

FYI: the issue is with the require statement (for nonMatchingMediaQueryRemover), which fails due to the ' in your filepath. This seems to be an upstream PhantomJS issue.

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

No branches or pull requests

2 participants