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

Error message for syntax error missing location in 4.2.1+ #1562

Closed
dougwilson opened this issue Sep 26, 2019 · 11 comments
Closed

Error message for syntax error missing location in 4.2.1+ #1562

dougwilson opened this issue Sep 26, 2019 · 11 comments

Comments

@dougwilson
Copy link
Contributor

@dougwilson dougwilson commented Sep 26, 2019

It seems like there was a change (I assume from looking at the commits the change was in the auto generated code that is not checked in) where the err.stack no longer includes the err.message, which explains what the syntax error is. Most places that log an error that want a stack trace will console.log(err.stack) assuming that err.message is included on the first line, and it was though the 4.2.0 release, no with 4.2.1+ it is missing.

Example code to reproduce:

node -pe 'require("handlebars").parse("{{")'

In 4.2.0 and prior versions, that produces the following:

$ node -pe 'require("handlebars").parse("{{")'
node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:267
            throw new Error(str);
            ^

Error: Parse error on line 1:
{{
--^
Expecting 'ID', 'STRING', 'NUMBER', 'BOOLEAN', 'UNDEFINED', 'NULL', 'DATA', got 'EOF'
    at Object.parseError (node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:267:19)
    at Object.parse (node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:336:30)
    at HandlebarsEnvironment.parse (node_modules/handlebars/dist/cjs/handlebars/compiler/base.js:46:43)
    at [eval]:1:23
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:577:32)
    at evalScript (bootstrap_node.js:358:27)
    at run (bootstrap_node.js:133:11)

But in 4.2.1+ it produces the following:

$ node -pe 'require("handlebars").parse("{{")'
Error
    at Object.parseError (node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:278:41)
    at Object.parse (node_modules/handlebars/dist/cjs/handlebars/compiler/parser.js:365:26)
    at HandlebarsEnvironment.parse (node_modules/handlebars/dist/cjs/handlebars/compiler/base.js:46:43)
    at [eval]:1:23
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:577:32)
    at evalScript (bootstrap_node.js:358:27)
    at run (bootstrap_node.js:133:11)
@rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Oct 2, 2019

This is also biting us over in the Ember world. The error messages on 4.2.1+ are completely useless (there is absolutely no indication of what is wrong in the template).

Here is an easy way to display the published package diff between 4.2.0 and 4.2.1:

https://diff.intrinsic.com/handlebars/4.2.0/4.2.1

@rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Oct 2, 2019

I suspect the issue is from this change in the published bundle (at dist/cjs/handlebars/compiler/parser.js):

         parseError: function parseError(str, hash) {
-            throw new Error(str);
+            if (hash.recoverable) {
+                this.trace(str);
+            } else {
+                var _parseError = function _parseError(msg, hash) {
+                    this.message = msg;
+                    this.hash = hash;
+                };
+
+                _parseError.prototype = new Error();
+
+                throw new _parseError(str, hash);
+            }
         },

@rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Oct 2, 2019

Based on the output, it appears that even though the package.json on master specifies jison@0.4.16 (here) the published versions of Handlebars since 4.0.0 have been building with jison@0.3.10 (because the v4.x branch has jison@~0.3.0 in its package.json here).

It looks like version v4.2.1 and newer have been published with jison@0.4.16 (the updated jison dependency landed on master in #1234, but seems to have never been ported to the 4.x branch).

@nknapp - Can you confirm that the location you publish from has the correct version of jison (needs 0.3.10) in its node_modules/jison/package.json?

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

I'll have to check. Thanks for the hint. Indeed, the npm package is published from my local computer. I may have switched to the master branch and back without running "npm ci'.

The version that can be downloaded from handlebarsjs.com should be OK than, because it is built in Travis. Could you check?

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

And I would be grateful if you (or anybody else) could contribute a test case. I wonder why no test exists for that case. Everything else is well tested...

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

Actually, 4.x should use jison 0.3.x

@rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Oct 2, 2019

Actually, 4.x should use jison 0.3.x

Aye, I updated my comment above with more info after realizing that 4.x had been branched from master quite a long time ago and that the jison dev dep on master was updated to 0.4.16 in the meantime. I suspect that master will also have some variant of this problem (until we can get onto jison@0.4.18).

The version that can be downloaded from handlebarsjs.com should be OK than, because it is built in Travis. Could you check?

Yes, I can double check it’s contents, but since I’m using handlebars as a nested dependency in a node project it’s a bit hard to actually use that version as a stop gap solution. Still happy to check and confirm your suspicion though.

I would be grateful if you (or anybody else) could contribute a test case.

I will try to work on a test case later this afternoon (if someone else doesn’t beat me to it 😉).

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

I'll try to get it reproduced as well.

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

The strange thing is: There is a test case. And since the release is done with "generator-release", it should have failed.

As far as I know, the tests are ran with the built js-files.

I don't really understand how this could happen.

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 2, 2019

I have re-published all 4.2.1+ versions (4.2.2, 4.3.5 and 4.4.2). They should work (I have tested manually).

@nknapp nknapp closed this Oct 2, 2019
@rwjblue
Copy link
Contributor

@rwjblue rwjblue commented Oct 7, 2019

Thank you very much @nknapp!

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

Successfully merging a pull request may close this issue.

None yet
3 participants