safely stringify PhantomJS undefined value

[ahamid]

When running in PhantomJS, due to http://stackoverflow.com/questions/14218670/why-are-null-and-undefined-of-the-type-domwindow canonicalize can throw an exception, resulting from Runner.prototype.uncaught returning without emitting the end event (causing test mocha-phantomjs test runner to just hang and not exit). I'm not sure whether it's better to alter logic in Runner.prototype.uncaught to ensure end is always emitting, as I don't really understand the logic there (why in certain cases it returns without emitting).

This patch just catches an exception stringifying the canonicalized value, which seems like a reasonable thing to do anyway.

[kcrwfrd]

👍

[danielstjules]

Thanks for the PR! Can you please revert changes to mocha.js? We only build that file before releases.

Also, could we not use canonicalizedObj = value +''; instead of canonicalizedObj = value.toString();?

var obj = {a: "b", toString: function() { return "foobar"; }};
obj.toString(); // "foobar"
obj+''; // "foobar"

[ahamid]

@danielstjules I have reverted the change to mocha.js. I'm using value + '' explicitly because the failure may be due to the value being null or undefined. Arguably something more strict should happen here, but at least string concatenation coerces those values to "null" and "undefined". It's a workaround.

[danielstjules]

Thanks for fixing that :) And what I meant was, why couldn't you do:

// Replace this
try {
  canonicalizedObj = value.toString();
} catch (e) {
  debug("Error invoking `toString()` on value: '%s'", value);
  canonicalizedObj = value + '';
}

// With this =>
canonicalizedObj = value + '';

Note the example I posted in my initial reply:

var obj = {a: "b", toString: function() { return "foobar"; }};
obj.toString(); // "foobar"
obj+''; // "foobar"

[danielstjules]

Also, mind rebasing/squashing when done? :)

[ahamid]

@danielstjules oh sorry, of course, I thought you were asking for the opposite. I didn't touch that line for the sake of preserving the initial intent, but I think you're right it's semantically the same.

So...here's the squashed one-liner :)

[danielstjules]

Awesome, thanks again! :)