From c5f5380eba2d42eab61e097408196cfbaf931bcd Mon Sep 17 00:00:00 2001 From: "K. Shankari" Date: Tue, 7 May 2019 20:58:01 -0700 Subject: [PATCH] Unify the handling of error messages - Define a new function (displayError) that deals with undefined messages, formatting the error stack, etc. - Replace all .catch(function(error)) calls with a call to displayError so that we can: - have a unified format that we can modify in one location - display meaningful error messages that should make it easier to figure out what is wrong. This makes the error message from https://github.com/e-mission/e-mission-docs/issues/378 more meaningful Bonus fix: fix the formatting in https://github.com/e-mission/e-mission-phone/pull/552/commits/94ffa9fa1f42dc34d5b5c9010394901b6b30c04b I am not sure how this ever worked! --- www/js/app.js | 4 +++- www/js/common/services.js | 5 +++-- www/js/control/general-settings.js | 5 ++--- www/js/diary/current.js | 5 ++--- www/js/diary/detail.js | 2 +- www/js/diary/list.js | 2 +- www/js/diary/services.js | 2 +- www/js/goals.js | 3 ++- www/js/heatmap.js | 8 +++----- www/js/incident/post-trip-map-display.js | 9 ++------- www/js/incident/post-trip-prompt.js | 15 +++++---------- www/js/metrics.js | 15 ++++----------- www/js/plugin/logger.js | 11 ++++++++++- www/js/recent.js | 4 +--- www/js/services.js | 11 ++++------- www/js/splash/pushnotify.js | 8 ++------ www/js/splash/startprefs.js | 3 ++- www/js/splash/updatecheck.js | 21 ++++----------------- www/js/survey/launch.js | 8 ++++---- www/js/tripconfirm/post-trip-map-display.js | 9 ++------- www/js/tripconfirm/post-trip-prompt.js | 13 +++---------- www/js/tripconfirm/trip-confirm-services.js | 8 +++++--- 22 files changed, 66 insertions(+), 105 deletions(-) diff --git a/www/js/app.js b/www/js/app.js index f2d4fafd3..0914f9960 100644 --- a/www/js/app.js +++ b/www/js/app.js @@ -56,12 +56,14 @@ angular.module('emission', ['ionic', Logger.log("connectionConfigString = "+JSON.stringify(connectionConfig.data)); window.cordova.plugins.BEMConnectionSettings.setSettings(connectionConfig.data); }).catch(function(err) { + // not displaying the error here since we have a backup Logger.log("error "+JSON.stringify(err)+" while reading connection config, reverting to defaults"); window.cordova.plugins.BEMConnectionSettings.getDefaultSettings().then(function(defaultConfig) { Logger.log("defaultConfig = "+JSON.stringify(defaultConfig)); window.cordova.plugins.BEMConnectionSettings.setSettings(defaultConfig); }).catch(function(err) { - Logger.log("error "+JSON.stringify(err)+" reading or setting defaults, giving up"); + // displaying the error here since we don't have a backup + Logger.displayError("Error reading or setting connection defaults", err); }); }); }); diff --git a/www/js/common/services.js b/www/js/common/services.js index 722c767b3..b2e0b3b14 100644 --- a/www/js/common/services.js +++ b/www/js/common/services.js @@ -1,8 +1,8 @@ 'use strict'; -angular.module('emission.main.common.services', []) +angular.module('emission.main.common.services', ['emission.plugin.logger']) -.factory('CommonGraph', function($rootScope, $http) { +.factory('CommonGraph', function($rootScope, $http, Logger) { var commonGraph = {}; commonGraph.data = {}; commonGraph.UPDATE_DONE = "COMMON_GRAPH_UPDATE_DONE"; @@ -25,6 +25,7 @@ angular.module('emission.main.common.services', []) cmGraph = commonGraph.createEmpty(); } } catch(err) { + // No popup for this since it is the common case and we have a fallback window.Logger.log("Error "+err+" while parsing common trip data"); // If there was an error in parsing the current common trips, and // there is no existing cached common trips, we create a blank diff --git a/www/js/control/general-settings.js b/www/js/control/general-settings.js index da18d37d4..74bad2c24 100644 --- a/www/js/control/general-settings.js +++ b/www/js/control/general-settings.js @@ -341,7 +341,7 @@ angular.module('emission.main.control',['emission.services', $ionicPopup.alert({template: 'all data pushed!'}); } }).catch(function(error) { - $ionicPopup.alert({template: 'error -> '+JSON.stringify(error)}); + Logger.displayError("Error while forcing sync", error); }); }; @@ -491,8 +491,7 @@ angular.module('emission.main.control',['emission.services', handleConsent(resultDoc); } }, function(error) { - $ionicPopup.alert({title: "Error reading consent document from cache", - template: error}); + Logger.displayError("Error reading consent document from cache", error) }); } diff --git a/www/js/diary/current.js b/www/js/diary/current.js index 477ebefe5..3d87c4a50 100644 --- a/www/js/diary/current.js +++ b/www/js/diary/current.js @@ -178,8 +178,7 @@ }); }); }).catch(function(error) { - Logger.log("While loading location data, error "+JSON.stringify(error)); - $ionicPopup.alert({"template": "While loading location data, error = "+ JSON.stringify(error)}) + Logger.displayError("Error while loading location data", error); }); console.log($scope.mapCtrl); }; @@ -266,7 +265,7 @@ PostTripManualMarker.showSheet($scope.features, latlng, ts, marker, _map); }) .catch(function(error) { - Logger.log("error while getting map current from leafletData"); + Logger.displayError("Error while getting selected map location ", error); }); }; diff --git a/www/js/diary/detail.js b/www/js/diary/detail.js index ceb7170f5..afa67bc64 100644 --- a/www/js/diary/detail.js +++ b/www/js/diary/detail.js @@ -137,7 +137,7 @@ angular.module('emission.main.diary.detail',['ui-leaflet', 'ng-walkthrough', nzTour.start(tour).then(function(result) { Logger.log("detail walkthrough start completed, no error"); }).catch(function(err) { - Logger.log("detail walkthrough start errored" + err); + Logger.displayError("detail walkthrough start errored", err); }); }; diff --git a/www/js/diary/list.js b/www/js/diary/list.js index a063cbdb3..63f344415 100644 --- a/www/js/diary/list.js +++ b/www/js/diary/list.js @@ -424,7 +424,7 @@ angular.module('emission.main.diary.list',['ui-leaflet', nzTour.start(tour).then(function(result) { Logger.log("list walkthrough start completed, no error"); }).catch(function(err) { - Logger.log("list walkthrough start errored" + err); + Logger.displayError("list walkthrough start errored", err); }); }; diff --git a/www/js/diary/services.js b/www/js/diary/services.js index f2c845bc2..249461f00 100644 --- a/www/js/diary/services.js +++ b/www/js/diary/services.js @@ -970,9 +970,9 @@ angular.module('emission.main.diary.services', ['emission.plugin.logger', }).then(function(combinedTripList) { processOrDisplayNone(day, combinedTripList); }).catch(function(error) { - Logger.log("while reading data from cache for "+day +" error = "+JSON.stringify(error)); console.log("About to hide loading overlay"); $ionicLoading.hide(); + Logger.displayError("while reading data from cache for "+day, error); }) }); } diff --git a/www/js/goals.js b/www/js/goals.js index 7f5b76645..fbcbc1b27 100644 --- a/www/js/goals.js +++ b/www/js/goals.js @@ -175,7 +175,8 @@ angular.module('emission.main.goals',['emission.services', 'emission.plugin.logg .then(function(event) { // success }) - .catch(function(event) { + .catch(function(error) { + Logger.displayError("Error while launching habitica website", error); // error }); diff --git a/www/js/heatmap.js b/www/js/heatmap.js index bf6af215b..aaa76e62c 100644 --- a/www/js/heatmap.js +++ b/www/js/heatmap.js @@ -54,8 +54,7 @@ angular.module('emission.main.heatmap',['ui-leaflet', 'emission.services', } $scope.countData.isLoading = false; }, function(error) { - Logger.log("Got error %s while trying to read heatmap data" + - JSON.stringify(error)); + Logger.displayError("Error while trying to read heatmap data", error); $scope.countData.isLoading = false; }); }; @@ -297,8 +296,7 @@ angular.module('emission.main.heatmap',['ui-leaflet', 'emission.services', } $scope.stressData.isLoading = false; }, function(error) { - Logger.log("Got error %s while trying to read heatmap data" + - JSON.stringify(error)); + Logger.displayError("Error while trying to read stress data", error); $scope.stressData.isLoading = false; }); }; @@ -361,7 +359,7 @@ angular.module('emission.main.heatmap',['ui-leaflet', 'emission.services', nzTour.start(tour).then(function(result) { Logger.log("heatmap walkthrough start completed, no error"); }).catch(function(err) { - Logger.log("heatmap walkthrough start errored" + err); + Logger.displayError("Error in heatmap walkthrough", err); }); }; diff --git a/www/js/incident/post-trip-map-display.js b/www/js/incident/post-trip-map-display.js index aff514b1e..23cbd0f4f 100644 --- a/www/js/incident/post-trip-map-display.js +++ b/www/js/incident/post-trip-map-display.js @@ -135,13 +135,8 @@ angular.module('emission.incident.posttrip.map',['ui-leaflet', 'ng-walkthrough', $ionicLoading.hide(); }) .catch(function(error) { - var errStr = JSON.stringify(error); $ionicLoading.hide(); - Logger.log(errStr); - $ionicPopup.alert({ - title: "Unable to retrieve data", - template: errStr - }); + Logger.displayError("Unable to retrieve location data for map", error); }); } @@ -176,7 +171,7 @@ angular.module('emission.incident.posttrip.map',['ui-leaflet', 'ng-walkthrough', nzTour.start(tour).then(function(result) { Logger.log("list walkthrough start completed, no error"); }).catch(function(err) { - Logger.log("incident walkthrough start errored" + err); + Logger.displayError("incident walkthrough start errored", err); }); }; diff --git a/www/js/incident/post-trip-prompt.js b/www/js/incident/post-trip-prompt.js index 3790b2b9e..329187392 100644 --- a/www/js/incident/post-trip-prompt.js +++ b/www/js/incident/post-trip-prompt.js @@ -69,12 +69,8 @@ angular.module('emission.incident.posttrip.prompt', ['emission.plugin.logger']) Logger.log("Finished registering "+notifyPlugin.TRIP_END+" with result "+JSON.stringify(result)); }) .catch(function(error) { - Logger.log(JSON.stringify(error)); - $ionicPopup.alert({ - title: "Unable to register notifications for trip end", - template: JSON.stringify(error) - }); - });; + Logger.displayError("Unable to register notifications for trip end", error); + }); } var getFormattedTime = function(ts_in_secs) { @@ -209,10 +205,9 @@ angular.module('emission.incident.posttrip.prompt', ['emission.plugin.logger']) }); } }).catch(function(error) { - $ionicPopup.alert({ - title: "Error while muting notifications for trip end. Try again later.", - template: JSON.stringify(error) - }); + Logger.displayError( + "Error while muting notifications for trip end. Try again later.", + error); }); } }); diff --git a/www/js/metrics.js b/www/js/metrics.js index 015f13ad4..2431e02ec 100644 --- a/www/js/metrics.js +++ b/www/js/metrics.js @@ -490,11 +490,7 @@ angular.module('emission.main.metrics',['nvd3', 'emission.services', 'ionic-date }) .catch(function(error) { $ionicLoading.hide(); - $ionicPopup.alert({ - title: "Error Loading Data", - template: JSON.stringify(error) - }); - console.log(error); + Logger.displayError("Error loading user data", error); }) getAggMetricsFromServer().then(function(results) { @@ -518,11 +514,8 @@ angular.module('emission.main.metrics',['nvd3', 'emission.services', 'ionic-date $ionicLoading.hide(); $scope.carbonData.aggrCarbon = "Unknown"; $scope.caloriesData.aggrCalories = "Unknown..."; - $ionicPopup.alert({ - title: "Error loading aggregate data, averages not available", - template: JSON.stringify(error) - }); - console.log(error); + Logger.displayError("Error loading aggregate data, averages not available", + error); }); }; @@ -1159,7 +1152,7 @@ angular.module('emission.main.metrics',['nvd3', 'emission.services', 'ionic-date showTodayButton: true, dateFormat: 'MMM dd yyyy', closeOnSelect: false, - # add this instruction if you want to exclude a particular weekday, e.g. Saturday disableWeekdays: [6] + // add this instruction if you want to exclude a particular weekday, e.g. Saturday disableWeekdays: [6] }; $scope.pickFromDay = function() { diff --git a/www/js/plugin/logger.js b/www/js/plugin/logger.js index 15262dc77..b604b7645 100644 --- a/www/js/plugin/logger.js +++ b/www/js/plugin/logger.js @@ -1,9 +1,18 @@ angular.module('emission.plugin.logger', []) -.factory('Logger', function($window, $state, $interval, $rootScope) { +.factory('Logger', function($window, $ionicPopup) { var loggerJs = {} loggerJs.log = function(message) { $window.Logger.log($window.Logger.LEVEL_DEBUG, message); } + loggerJs.displayError = function(title, error) { + var display_msg = error.message + "\n" + error.stack; + if (!angular.isDefined(error.message)) { + display_msg = JSON.stringify(error); + } + $ionicPopup.alert({"title": title, "template": display_msg}); + console.log(title + display_msg); + $window.Logger.log($window.Logger.LEVEL_ERROR, title + display_msg); + } return loggerJs; }); diff --git a/www/js/recent.js b/www/js/recent.js index a1867970c..db2bc3bc0 100644 --- a/www/js/recent.js +++ b/www/js/recent.js @@ -201,9 +201,7 @@ angular.module('emission.main.recent', ['ngCordova', 'emission.services']) // This should really be within a try/catch/finally block $scope.$broadcast('scroll.refreshComplete'); }, function(error) { - $ionicPopup.alert({title: "Error updating entries", - template: JSON.stringify(error)}) - .then(function(res) {console.log("finished showing alert");}); + window.logger.Logger.displayError("Error updating entries", error); }) } diff --git a/www/js/services.js b/www/js/services.js index 08c48e9e1..bca716247 100644 --- a/www/js/services.js +++ b/www/js/services.js @@ -240,7 +240,8 @@ angular.module('emission.services', ['emission.plugin.logger']) }) .service('ControlHelper', function($cordovaEmailComposer, $ionicPopup, - CommHelper) { + CommHelper, + Logger) { this.emailLog = function() { var parentDir = "unknown"; @@ -391,14 +392,10 @@ angular.module('emission.services', ['emission.plugin.logger']) .then(writeDumpFile) .then(emailData) .then(function() { - window.Logger.log(window.Logger.LEVEL_DEBUG, - "Email queued successfully"); + Logger.log("Email queued successfully"); }) .catch(function(error) { - window.Logger.log(window.Logger.LEVEL_INFO, - "Email cancel reported, seems to be an error on android"); - $ionicPopup.alert({'title': "Error sending email", - 'template': JSON.stringify(error)}); + Logger.displayError("Error emailing JSON dump", error); }) }; diff --git a/www/js/splash/pushnotify.js b/www/js/splash/pushnotify.js index 16c4a2db6..f72312677 100644 --- a/www/js/splash/pushnotify.js +++ b/www/js/splash/pushnotify.js @@ -79,10 +79,7 @@ angular.module('emission.splash.pushnotify', ['emission.plugin.logger', // alert("Finished saving token = "+JSON.stringify(t.token)); Logger.log("Finished saving token = "+JSON.stringify(t.token)); }).catch(function(error) { - var display_msg = error.message + "\n" + error.stack; - $ionicPopup.alert({title: "Error in registering push notifications", - template: display_msg}); - Logger.log("Error in registering push notifications "+display_msg); + Logger.displayError("Error in registering push notifications", error); }); } @@ -114,8 +111,7 @@ angular.module('emission.splash.pushnotify', ['emission.plugin.logger', }) .catch(function(error) { push.finish(function(){}, finishErrFn, notId); - $ionicPopup.alert({title: "Error while handling silent push notifications", - template: JSON.stringify(error)}); + Logger.displayError("Error while redirecting silent push", error); }); } diff --git a/www/js/splash/startprefs.js b/www/js/splash/startprefs.js index 1a16bab03..30fa42359 100644 --- a/www/js/splash/startprefs.js +++ b/www/js/splash/startprefs.js @@ -207,7 +207,8 @@ angular.module('emission.splash.startprefs', ['emission.plugin.logger', startprefs.loadPreferredScreen = function() { logger.log("About to navigate to preferred tab"); startprefs.getNextState().then(changeState).catch(function(error) { - logger.log("error "+error+" loading finding tab, loading root.intro"); + logger.displayError("Error loading preferred tab, loading root.intro", error); + // logger.log("error "+error+" loading finding tab, loading root.intro"); changeState('root.intro'); }); }; diff --git a/www/js/splash/updatecheck.js b/www/js/splash/updatecheck.js index 44f996752..d7604ffa8 100644 --- a/www/js/splash/updatecheck.js +++ b/www/js/splash/updatecheck.js @@ -100,13 +100,7 @@ angular.module('emission.splash.updatecheck', ['emission.plugin.logger', Logger.log("successfully set the channel to "+urlComponents['new_client']); uc.checkForUpdates(); }).catch(function(error) { - var display_msg = error.message + "\n" + error.stack; - if (!angular.isDefined(error.message)) { - display_msg = JSON.stringify(error); - } - $ionicPopup.alert({"title": "Unable to handle client change", - "template": display_msg}); - Logger.log('Ionic Deploy: Unable to handle client change URL '+display_msg); + Logger.displayError("Unable to handle client change", error); }) }; @@ -156,18 +150,12 @@ angular.module('emission.splash.updatecheck', ['emission.plugin.logger', }).catch(function(err) { $rootScope.isDownloading = false; extractPop.close(); - $ionicPopup.alert({ - title: "Extraction error", - message: JSON.stringify(err) - }); + Logger.displayError("Extraction error", err); }) }).catch(function(err) { $rootScope.isDownloading = false; downloadPop.close(); - $ionicPopup.alert({ - title: "Download error", - message: JSON.stringify(err) - }); + Logger.displayError("Download error", err); }); }; @@ -213,8 +201,7 @@ angular.module('emission.splash.updatecheck', ['emission.plugin.logger', }) }).catch(function(err) { $rootScope.isDownloading = false; - Logger.log('Ionic Deploy: Unable to check for updates'+err); - console.error('Ionic Deploy: Unable to check for updates',err) + Logger.displayError("Unable to check for updates", err); }) } diff --git a/www/js/survey/launch.js b/www/js/survey/launch.js index f5dee2947..f4f4ddb67 100644 --- a/www/js/survey/launch.js +++ b/www/js/survey/launch.js @@ -91,8 +91,8 @@ angular.module('emission.survey.launch', ['emission.services', }); }); }) - .catch(function(event) { - // error + .catch(function(error) { + Logger.displayError("Unable to launch survey", error); }); $rootScope.$on('$cordovaInAppBrowser:loadstart', function(e, event) { console.log("started loading, event = "+JSON.stringify(event)); @@ -124,8 +124,8 @@ angular.module('emission.survey.launch', ['emission.services', }); }); }) - .catch(function(event) { - // error + .catch(function(error) { + Logger.displayError("Unable to launch survey", error); }); $rootScope.$on('$cordovaInAppBrowser:loadstart', function(e, event) { console.log("started loading, event = "+JSON.stringify(event)); diff --git a/www/js/tripconfirm/post-trip-map-display.js b/www/js/tripconfirm/post-trip-map-display.js index 53aea4db4..e363617e8 100644 --- a/www/js/tripconfirm/post-trip-map-display.js +++ b/www/js/tripconfirm/post-trip-map-display.js @@ -142,13 +142,8 @@ angular.module('emission.tripconfirm.posttrip.map',['ui-leaflet', 'ng-walkthroug $ionicLoading.hide(); }) .catch(function(error) { - var errStr = JSON.stringify(error); $ionicLoading.hide(); - Logger.log(errStr); - $ionicPopup.alert({ - title: "Unable to retrieve data", - template: errStr - }); + Logger.displayError("Unable to retrieve data for map display", error); }); } @@ -183,7 +178,7 @@ angular.module('emission.tripconfirm.posttrip.map',['ui-leaflet', 'ng-walkthroug nzTour.start(tour).then(function(result) { Logger.log("post trip mode walkthrough start completed, no error"); }).catch(function(err) { - Logger.log("post trip mode walkthrough start errored" + err); + Logger.displayError("post trip mode walkthrough errored", err); }); }; diff --git a/www/js/tripconfirm/post-trip-prompt.js b/www/js/tripconfirm/post-trip-prompt.js index 68e38cfc9..1df0c8429 100644 --- a/www/js/tripconfirm/post-trip-prompt.js +++ b/www/js/tripconfirm/post-trip-prompt.js @@ -69,12 +69,8 @@ angular.module('emission.tripconfirm.posttrip.prompt', ['emission.plugin.logger' Logger.log("Finished registering "+notifyPlugin.TRIP_END+" with result "+JSON.stringify(result)); }) .catch(function(error) { - Logger.log(JSON.stringify(error)); - $ionicPopup.alert({ - title: "Unable to register notifications for trip end", - template: JSON.stringify(error) - }); - });; + Logger.displayError("Unable to register notifications for trip end", error); + }); } var getFormattedTime = function(ts_in_secs) { @@ -196,10 +192,7 @@ angular.module('emission.tripconfirm.posttrip.prompt', ['emission.plugin.logger' }); } }).catch(function(error) { - $ionicPopup.alert({ - title: "Error while muting notifications for trip end. Try again later.", - template: JSON.stringify(error) - }); + Logger.displayError("Error while muting notifications for trip end. Try again later.", error); }); } }); diff --git a/www/js/tripconfirm/trip-confirm-services.js b/www/js/tripconfirm/trip-confirm-services.js index e9475a5fa..d155a495b 100644 --- a/www/js/tripconfirm/trip-confirm-services.js +++ b/www/js/tripconfirm/trip-confirm-services.js @@ -1,5 +1,5 @@ -angular.module('emission.tripconfirm.services', ['ionic']) -.factory("ConfirmHelper", function($http, $ionicPopup) { +angular.module('emission.tripconfirm.services', ['ionic', "emission.plugin.logger"]) +.factory("ConfirmHelper", function($http, $ionicPopup, Logger) { var ch = {}; ch.otherModes = []; ch.otherPurposes = []; @@ -16,11 +16,13 @@ angular.module('emission.tripconfirm.services', ['ionic']) return $http.get(filename) .then(fillInOptions) .catch(function(err) { + // no prompt here since we have a fallback console.log("error "+JSON.stringify(err)+" while reading confirm options, reverting to defaults"); return $http.get(filename+".sample") .then(fillInOptions) .catch(function(err) { - console.log("error "+JSON.stringify(err)+" while reading default confirm options, giving up"); + // prompt here since we don't have a fallback + Logger.displayError("Error while reading default confirm options", err); }); }); }