Ambiguous Less error

[Skwai]

The following LESS code generates a pretty unhelpful error (missing closing parenthesis on media query).

Cannot read property 'length' of null in examples.less on line null, column 0:

@width: 1000px;

@media (min-width: (@width - 1px) {
    color: green;
}

[Soviut]

You aren't closing your media query properly because you're missing a closing parenthesis.

@media (min-width: (@width - 1px)) {

[seven-phases-max]

Just in case, here's minimal code to get the same error:

@media {}

[Soviut]

Sorry, I was reading this as you asking why you were getting the error, not that the error message was ambiguous. My bad.

[seven-phases-max]

I guess it's the same for 1.4.x - 1.5.x

[seven-phases-max]

Another interesting snippet (not the same but related):

@mediaz {boo:boo}

result:

@media z {
  boo: boo;
}

I guess in all these cases compiler expects some "query" to go right after @media (with white spaces ignored) and when there's none or an incomplete one - boom!

[Soviut]

Yeah, an "invalid selector" or "invalid query" might be a good error to raise.

[seven-phases-max]

OK, I was almost about to submit a patch that improves the error message but then I looked into w3c docs to find out what error message it's better to be ("expected media type or media features expression" or so) and stepped into this:

A media query consists of a media type and zero or more expressions that check for the conditions of particular media features.

And though it does not say that media type is optional, a few lines below it states:

I.e. these are equivalent:

@media all {...}
@media {...}

Quite surprising, i.e. @media {...} appears to be valid CSS and we are supposed to output it "as is"... (Sadly this makes a patch a bit more complicated).

[lukeapage]

The performance improvements I am merging in will now error on unclosed
parentheses which should catch this better?

[seven-phases-max]

@lukeapage, mediaFeature handles its outer parens on its own, will this be affected by the "performance improvements" changes?

(After all we will always can simply give up @media {...} by saying "use @media all {...} instead" if this all becomes too complicated, I guess it may be a reasonable compromise if this "never really used CSS feature" would require too many changes).

[lukeapage]

No the changes affect the tokeniser s the bug would still happen in the
unlikely scenario you had a missing close brace one place and extra one
somewhere else...

[seven-phases-max]

OK, either way I think I'll wait for the performance improvements to appear in the master. (So far I'm not happy with my attempts to fix it to handle both @media {...} and @media (x:(0) {...} the right way).

[andrewwakeling]

It's unfortunate that there isn't a clearer message whilst having issues parsing @media.

There's a few try/catch blocks in parser.js which are immediately wrapping in-built errors (e.g. SyntaxError) in LessError.

The problem is that when these errors are getting reported:

• errorConsole in browser.js
• errorHTML in browser.js
• error in rhino.js

The extract property is truthy but the values (e.g. line, column) aren't populated. Also, this code path means that the stack isn't displayed, which is the real source of the error.

The code is slightly different in index.js but it serves the same purpose.

It should be possible to make this error reporting more robust for in-built errors.
e.g.

An error occurred whilst parsing <filename>.less. 
<stack trace here>

Thoughts?

[seven-phases-max]

Stack trace? I doubt a stack trace of the compiler code will be helpful to understand a error of a LESS code. (Though it might be useful for the compiler development itself as sometimes it's really hard to find where some "unexpected in line: null column: 0" comes from).

[andrewwakeling]

There's 2 issues here:

• a specific parsing issue with @media
• unhelpful error reporting which mentions "line null, column 0"

It's great if you can fix every single parsing error that occurs so that a helpful message can be displayed.
There's also programmatic error in the parser which needs to be fixed in the code (and not in LESS templates). In those cases, it's invaluable to have the stack trace, especially if the problem is not easily reproduceable.

Assuming that the parser is fairly robust, the frequency of these programmatic errors should be rare, so the appearance of stack traces to end-users should be rare too.

[andrewwakeling]

Also, for interest sake, here's the stack trace when you have @media {}.

"TypeError: Cannot read property 'length' of null
    at Object.tree.Value.eval (http://localhost:12000/less-1.5.1.js:5078:23)
    at Object.tree.Media.eval (http://localhost:12000/less-1.5.1.js:3885:44)
    at Object.tree.Ruleset.eval (http://localhost:12000/less-1.5.1.js:4552:53)
    at Object.toCSS (http://localhost:12000/less-1.5.1.js:479:46)
    at http://localhost:12000/less-1.5.1.js:6911:28
    at http://localhost:12000/less-1.5.1.js:6806:21
    at Object.Parser.parser.parse.finish [as _finish] (http://localhost:12000/less-1.5.1.js:566:28)
    at Object.tree.importVisitor.run (http://localhost:12000/less-1.5.1.js:5371:22)
    at Object.Parser.parser.parse [as parse] (http://localhost:12000/less-1.5.1.js:572:22)
    at http://localhost:12000/less-1.5.1.js:6803:35"

[lukeapage]

this particular issue is fixed now. there is a test for it. I've been slowly improving error reporting since I started contributing and there are extra things we could do and I welcome any pull requests to improve that.