play-plugins icon indicating copy to clipboard operation
play-plugins copied to clipboard

Error reporting is broken in dust plugin

Open brikis98 opened this issue 13 years ago • 3 comments

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?

brikis98 avatar Sep 18 '12 23:09 brikis98

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.

freekh avatar Sep 19 '12 09:09 freekh

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

brikis98 avatar Sep 19 '12 18:09 brikis98

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?

brikis98 avatar Sep 19 '12 18:09 brikis98