Combine exception stacks lazily where possible

[glenjamin]

This is a bit wacky, but I figured I'd send in the PR and see what you think.

I did a big survey of error / exception libs I could find on npm today, and this one ticked the boxes I was looking for - attaching arbitrary data, and concatenating stack traces together.

One thing I was hoping to find was a lib which only attached the stack traces together if the .stack property is read - especially on v8 where this property is lazily calculated.

This PR attempts to detect if this is possible in the executing runtime, and then does so.

Passes tests on Karma and Node locally, but there might be something I've missed still

[JamesMessinger]

Omigod that rocks! Great idea. I'll do some testing of my own and maybe even add some new tests to specifically cover this, but yes, I'll be glad to merge this PR.

[JamesMessinger]

Actually... I may not have the time to get to this myself for a week or two. So why wait? :)

Would you mind adding a test case to hit this line of code? It's the only branch that's not currently covered. If you'll do that, I'll merge the PR ASAP

[glenjamin]

Is the coverage only from node?

I can patch a global to simulate the fallback to hit that case.

[JamesMessinger]

The coverage data includes node and browsers. (all the lcov.info files are concatenated together)

However, I just noticed that I only have KARMA_COVERAGE=true set for one of the CI builds, and that buid only includes Firefox and PhantomJS. Try setting KARMA_COVERAGE=true for the SauceLabs build. That build covers all browsers, so maybe one of them will hit that line of code.

[glenjamin]

I'm fairly sure firefox should hit that line, i'll give it a go.

[glenjamin]

That's all passing now and executing the line from before (older versions of firefox made .stack a property descriptor that wasn't lazy)

As part of these changes I broke a long line up and because this is line coverage and not branch coverage, it's not getting hit any more.

The line in question is https://coveralls.io/builds/4946993/source?filename=lib%2Findex.js#L192 which can only be hit if Object.getOwnPropertyDescriptor && Object.defineProperty is false.

This is only needed for IE8 support - is that a goal?

[JamesMessinger]

IE8 support isn't necessary, but it doesn't hurt to leave that line in there just in case. I'm ok with it not being hit.

Thanks for all your work! Merging in 3... 2... 1...

[JamesMessinger]

Onoes! The tests are failing on Android due to the _error property that gets added by the extendStackProperty() method.

I'll make a quick change to avoid adding that property.

[glenjamin]

Thanks! Do the android tests only run on the main branch?

[JamesMessinger]

I have no idea why the Android tests didn't fail on the PR branch. They appear to have run successfully there. No clue.