Skip to content
This repository has been archived by the owner on Feb 9, 2019. It is now read-only.

Error reporting is broken in dust plugin #35

Open
brikis98 opened this issue Sep 18, 2012 · 3 comments
Open

Error reporting is broken in dust plugin #35

brikis98 opened this issue Sep 18, 2012 · 3 comments

Comments

@brikis98
Copy link

If the dust plugin hits an error while compiling a template, the error reporting does not work correctly. It used to show the line number and code snippet in the browser, but now it just shows:

NumberFormatException: empty String

Here's the stack trace:

java.lang.NumberFormatException: empty String
    at sun.misc.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:992) ~[na:1.6.0_31]
    at java.lang.Double.parseDouble(Double.java:510) ~[na:1.6.0_31]
    at scala.collection.immutable.StringLike$class.toDouble(StringLike.scala:234) ~[scala-library.jar:0.12.0]
    at scala.collection.immutable.StringOps.toDouble(StringOps.scala:31) ~[scala-library.jar:0.12.0]
    at com.typesafe.plugin.DustTasks$class.compile(DustTasks.scala:48) ~[na:na]
    at com.typesafe.plugin.DustPlugin$.compile(DustPlugin.scala:7) ~[na:na]

Looking at the code, it looks like this line in DustTasks is suspicious:

val line = ScriptableObject.getProperty(jsError, "fileName").toString.toDouble.toInt

It would be easy to check that the fileName property is not null/empty, but why are we converting something called fileName to an Int?

@freekh
Copy link
Contributor

freekh commented Sep 19, 2012

Yeah, I agree this is pretty darn confusing - sorry about that! There should DEFINITELY have been a comment there. I am sure that in some religions you end up in hell for doing this...

I do think I remember the story though: the error we (used to) get from Dust was wrongly formatted, basically putting line number of the errors on the fileName key. I even seem to remember thinking that we could have regexp matched on this, but ended up not doing it because if Dust changed at least this way it would fail quickly. It was absolutely worthy of a comment though.

I think what would be interesting to do is to make sure we still find the line number in the exception from somewhere. If fileName is empty perhaps they have changed around on the format of the jsError.

@brikis98
Copy link
Author

I think this is the commit where the error handling changed in dust.js.

@brikis98
Copy link
Author

Here is how errors are being handled now:

  try {
    var ast = filterAST(dust.parse(source));
    return compile(ast, name);
  }
  catch(err)
  {
    if(!err.line || !err.column) throw err;    
    throw new SyntaxError(err.message + " At line : " + err.line + ", column : " + err.column);
  }

Fredrik: I'm not too familiar with Rhino. Is there an easy way to parse the errors from this so that Play displays them correctly?

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

No branches or pull requests

2 participants