Include a stacktrace from execjs

[andygeers]

The latest version of execjs now includes backtraces in Javascript errors - this makes debugging problems much easier!

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[vipulnsward]

@andygeers tests are failing, could you take a look?
Also we support rubyracer, etc. Would this be affected by this?

[rmosolgo]

Looks like the same failing test that's been bugging us all along:

  2) Error:
ViewHelperTest#test_react_server_rendering_does_not_include_internal_properties:
ActionView::Template::Error: TypeError: Cannot set property 'root' of null
  (in /home/travis/.rvm/gems/ruby-2.0.0-p598/gems/turbolinks-2.5.3/lib/assets/javascripts/turbolinks.js.coffee)

That lingering Turbolinks bug, tried to address here, #168 but locking to 1.8.0 gave people other problems.

@andygeers could you add a test that checks for the presence of a backtrace? That would give some insight to how it's treated on the different JS runtimes (and make sure this feature keeps working!)

[rmosolgo]

Sorry, should have said first: thanks, this will be a great feature!! :D

[andygeers]

@rmosolgo I'm not sure if that's the kind of test you were looking for?
@vipulnsward From what I can tell, the change in execjs applies to all the different runtime engines they support - we're using rubyracer here, and there are definitely commits related to fixing it in rhino too

[rmosolgo]

Yeah, like that, but preferably without stubbing ExecJS::ProgramError#backtrace. While the stub is present, we wouldn't know if the ExecJS API changed.

I imagine it's hard (and pointless) to test the actual content of the backtrace, but I think it would be an improvement even just to test for a bunch of newlines, for example,

assert(err.message.count("\n") > 2, "The error has a multi-line backtrace")

That puts a nice regression test on getting what we expect from ExecJS!

[andygeers]

Ok, done

[rmosolgo]

👍 😻 it'll be great to actually get some context from the prerender environment!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[rmosolgo]

thanks again!