CB-9366 Log error.stack as it helps with diagnosing the source of the…

[nikhilkh]

… error

https://issues.apache.org/jira/browse/CB-9366

[purplecabbage]

I would leave this to userland.
window.onerror = function(err,filename,lineNumber) { ... }

Also, on iOS this is going to look like :
callMe@file:///Users/purplecabbage/Library/Application%20Support/iPhone%20Simulator/7.1/Applications/F40363A6-5AFE-4E19-8125-A1A4C928595D/HelloCordova.app/www/js/index.js:12:13
iAmFunky@file:///Users/purplecabbage/Library/Application%20Support/iPhone%20Simulator/7.1/Applications/F40363A6-5AFE-4E19-8125-A1A4C928595D/HelloCordova.app/www/js/index.js:26:33
fire@file:///Users/purplecabbage/Library/Application%20Support/iPhone%20Simulator/7.1/Applications/F40363A6-5AFE-4E19-8125-A1A4C928595D/HelloCordova.app/www/cordova.js:750:28
file:///Users/purplecabbage/Library/Application%20Support/iPhone%20Simulator/7.1/Applications/F40363A6-5AFE-4E19-8125-A1A4C928595D/HelloCordova.app/www/cordova.js:223:53

[nikhilkh]

What's the downside of adding this? This is really an exceptional scenario and should not hit commonly to clutter console log with ugly stack traces. Also, rethrowing the error will reset it's stack property - depending on which runtime and platform you are using and hence. window.onerror might not get the actual stack that caused the error.
Putting it in console.log makes sure it shows in ADB logcat, Windows debug logs - hence, it makes it easier for people to root cause such issues.

[purplecabbage]

Not a huge deal. I was assuming that most errors here would be the result of a missing callback, in which case stack.length is 0.
In the case where the user has an error in their success/error handler this might help them, but I am still not sure how much more value they would get than window.onerror

[nikhilkh]

If you have multiple calls to cordova methods and one of them might be missing a callback or passing another incorrect argument (like in a scenario similar to the recent new Media() bug) a call stack tells you in your code the line which might have resulting in that exception causing your app to crash. The bug fixed in apache/cordova-plugin-media@5f8738c would have been much easier to identify from buildBot ADB logs if we had a stack. Instead, I had to get into the debugger to identify the issue.

[jimmont]

This try-catch for callbacks is a real pain. Having access to errors as they occur is highly desirable. And there appears to be a logic error here where callbacks are never deleted if they throw an error. Isn't managing errors a responsibility outside of self.cordova? Just started looking at this javascript after a lot of problematic worker debugging.

[nikhilkh]

@jimmont Why is the try-catch a pain? Is it because the stack gets reset here? You have a fair point about the callback not being deleted on exception scenarios.

[jimmont]

@nikhilkh Yes. Throwing errors is a reliable way of both debugging and finding problems. In fact I don't know of a more reliable way to debug. When they're intercepted I can't do that as easily. (Similar is true of cordova-plugin-console.) It's my opinion (for whatever it's worth) that making assumptions about the cause of an error, especially in a library, and especially in one that targets so many scenarios, is pretty significant. In looking through the source cordova uses a module loader and http requests to setup global APIs from the local filesystems on the device, and various events/event-system to get the sequence and setup right. Seems sub-optimal to me. (For example, a concatenated script which works with the global namespace directly seems like the simple approach, all things considered, and is what happens in the end anyway.) But I haven't finished looking at all the contemporary platforms (that I want to target anyhow). No one asked for these observations but given I'm seeing them, and how expensive time-wise it is to work either with or around them, thought I'd mention it. (In case more context is helpful: imagine debugging getting cordova and all the desired plugins working in a worker.)

[janpio]

@nikhilkh Is this PR still relevant? If so, could you take a look at the conflicts please?

[brodycj]

Looks like it would be really nice to have this enhancement, maybe for the next major release.

[purplecabbage]

I am okay if you want to merge it, my initial point of view was just that there are other ways of handling this, but it’s not a huge deal to me ...

…

On Jun 18, 2018, at 1:29 AM, Christopher J. Brody ***@***.***> wrote: Looks like it would be really nice to have this enhancement, maybe for the next major release. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.