Details
-
Improvement
-
Status: Open
-
Minor
-
Resolution: Unresolved
-
None
-
None
-
None
Description
Looking at cordova.js in cordova-windows along with the other platforms I noticed that the JavaScript logs a message in most but not all catch blocks. Here are a few exceptions I spotted so far:
In cordova-js src/common/builder.js:
function include (parent, objects, clobber, merge) { each(objects, function (obj, key) { try { var result = obj.path ? require(obj.path) : {}; // ... } catch (e) { utils.alert('Exception building Cordova JS globals: ' + e + ' for key "' + key + '"'); } }); }
utils.alert would show an alert if possible, otherwise log a message. I would rather to see the JavaScript log the message regardless of whether or not it is possible to show the alert (ideally log first). Possible fix (NOT TESTED):
diff --git a/src/common/utils.js b/src/common/utils.js index febfd91..7244b8f 100644 --- a/src/common/utils.js +++ b/src/common/utils.js @@ -170,12 +170,9 @@ utils.extend = (function () { }()); /** - * Alerts a message in any available way: alert or console.log. + * Alerts a message in any possible way: alert / console.log */ utils.alert = function (msg) { - if (window.alert) { - window.alert(msg); - } else if (console && console.log) { - console.log(msg); - } + console && console.log && console.log(msg); + window.alert && window.alert(msg); };
Possible fixes in cordova-windows (NOT TESTED):
diff --git a/cordova-js-src/confighelper.js b/cordova-js-src/confighelper.js index c166052..faa65e3 100644 --- a/cordova-js-src/confighelper.js +++ b/cordova-js-src/confighelper.js @@ -89,7 +89,11 @@ function requestFile(filePath, success, error) { xhr.open("get", filePath, true); xhr.send(); } catch (e) { - fail('[Windows][cordova.js][xhrFile] Could not XHR ' + filePath + ': ' + JSON.stringify(e)); + var msg = + '[Windows][cordova.js][xhrFile] Could not XHR ' + filePath + ': ' + + JSON.stringify(e); + console.error(msg); + fail(msg); } } diff --git a/cordova-js-src/platform.js b/cordova-js-src/platform.js index 4bc4025..dbc1c75 100644 --- a/cordova-js-src/platform.js +++ b/cordova-js-src/platform.js @@ -99,7 +99,9 @@ module.exports = { try { Windows.UI.ViewManagement.ApplicationView.getForCurrentView(); isCoreWindowAvailable = true; - } catch (e) { } + } catch (e) { + console && console.log && console.log('NOTICE: CoreWindow functionality is not available'); + } if (isCoreWindowAvailable) { app.addEventListener("checkpoint", checkpointHandler); @@ -160,6 +162,7 @@ function injectBackButtonHandler() { return true; } catch (e) { + console && console.log && console.log('NOTICE: backbutton handler not available, ignored'); return false; } }
I would need some time to check for unlogged exceptions on the other platforms.
A case where I think logging should NOT be done in release build is in the clobber funciton in cordova-js/src/common/builder.js:
function clobber (obj, key, value) { exports.replaceHookForTesting(obj, key); var needsProperty = false; try { obj[key] = value; } catch (e) { needsProperty = true; } // ...
I originally spotted this issue on Windows when looking at the ApplicationView calls related to the changes in CB-12238, CB-12784, and CB-13641 along with suggested changes I raised in CB-13802.