[jira] [Updated] (CB-13804) Always log a message when catching an exception

["Jan Piotrowski (janpio) (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CB-13804?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jan Piotrowski (janpio) updated CB-13804:
-----------------------------------------
    Component/s:     (was: cordova-windows)

> Always log a message when catching an exception
> -----------------------------------------------
>
>                 Key: CB-13804
>                 URL: https://issues.apache.org/jira/browse/CB-13804
>             Project: Apache Cordova
>          Issue Type: Improvement
>          Components: cordova-android, cordova-browser, cordova-ios, cordova-js, cordova-osx
>            Reporter: Chris Brody
>            Assignee: Joe Bowser
>            Priority: Minor
>
> 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}}:
> {code:javascript}
> 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 + '"');
>         }
>     });
> }
> {code}
> {{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):
> {code:javascript}
> 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);
>  };
> {code}
> Possible fixes in {{cordova-windows}} (NOT TESTED):
> {code:javascript}
> 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;
>              }
>          }
> {code}
> 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}}:
> {code:javascript}
> function clobber (obj, key, value) {
>     exports.replaceHookForTesting(obj, key);
>     var needsProperty = false;
>     try {
>         obj[key] = value;
>     } catch (e) {
>         needsProperty = true;
>     }
>     // ...
> {code}
> 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.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org