refactor(GUI): unify error handling (#620)

Error handling is currently a mess. The knowledge to correctly report an
error both to the end user and to us is scattered in many places.

This PR introduces the following changes:

- Rename `AnalyticsService.logDebug()` to `AnalyticsService.logDebug()`
  to clarify better the intention of the function.

- Move `$log` decorators from an `AnalyticsService` run block to
  `AnalyticsService.logDebug()`.

- Implement `AnalyticsService.logException()`, whose duty is to log an
  exception to TrackJS or any related service, and log it to DevTools.

- Implement `ErrorService.reportException()`, whose duty is to report an
  exception to every interested party. This means logging the error to
  TrackJS, displaying it DevTools and showing a nice alert to the user.
  This function is based from `handleError()` from `MainController`.

- Move global `$exceptionHandler` error handler to the entry point of
  the application, and make it simply call
  `ErrorService.reportException()`.

- Replace every `handleError()` call in `MainController` with
  `ErrorService.reportException()`.

Signed-off-by: Juan Cruz Viotti <jviottidc@gmail.com>

lib/gui/app.js

@@ -35,6 +35,7 @@ const app = angular.module('Etcher', [
 
   // Etcher modules
   require('./modules/analytics'),
+  require('./modules/error'),
 
   // Models
   require('./models/selection-state'),
@@ -95,7 +96,7 @@ app.run((AnalyticsService, OSWindowProgressService, FlashStateModel) => {
       return;
     }
 
-    AnalyticsService.log([
+    AnalyticsService.logDebug([
       `Progress (${flashState.type}):`,
       `${flashState.percentage}% at ${flashState.speed} MB/s`,
       `(eta ${flashState.eta}s)`
@@ -108,3 +109,13 @@ app.run((AnalyticsService, OSWindowProgressService, FlashStateModel) => {
 app.config(($urlRouterProvider) => {
   $urlRouterProvider.otherwise('/main');
 });
+
+app.config(($provide) => {
+  $provide.decorator('$exceptionHandler', ($delegate, $injector) => {
+    return (exception, cause) => {
+      const ErrorService = $injector.get('ErrorService');
+      ErrorService.reportException(exception);
+      $delegate(exception, cause);
+    };
+  });
+});

lib/gui/modules/analytics.js

@@ -39,6 +39,9 @@ const analytics = angular.module(MODULE_NAME, [
   require('../models/settings')
 ]);
 
+// Mixpanel integration
+// https://github.com/kuhnza/angular-mixpanel
+
 analytics.config(($mixpanelProvider) => {
   $mixpanelProvider.apiKey('63e5fc4563e00928da67d1226364dd4c');
 
@@ -74,41 +77,7 @@ analytics.run(($window) => {
   });
 });
 
-analytics.config(($provide) => {
+analytics.service('AnalyticsService', function($log, $window, $mixpanel, SettingsModel) {
-  $provide.decorator('$exceptionHandler', ($delegate, $window, $injector) => {
-    return (exception, cause) => {
-      const SettingsModel = $injector.get('SettingsModel');
-
-      if (SettingsModel.get('errorReporting') && isRunningInAsar()) {
-        $window.trackJs.track(exception);
-      }
-
-      $delegate(exception, cause);
-    };
-  });
-
-  $provide.decorator('$log', ($delegate, $window, $injector) => {
-
-    // Save the original $log.debug()
-    const debugFn = $delegate.debug;
-
-    $delegate.debug = (message) => {
-      message = new Date() + ' ' + message;
-
-      const SettingsModel = $injector.get('SettingsModel');
-
-      if (SettingsModel.get('errorReporting') && isRunningInAsar()) {
-        $window.trackJs.console.debug(message);
-      }
-
-      debugFn(message);
-    };
-
-    return $delegate;
-  });
-});
-
-analytics.service('AnalyticsService', function($log, $mixpanel, SettingsModel) {
 
   /**
    * @summary Log a debug message
@@ -123,7 +92,13 @@ analytics.service('AnalyticsService', function($log, $mixpanel, SettingsModel) {
    * @example
    * AnalyticsService.log('Hello World');
    */
-  this.log = (message) => {
+  this.logDebug = (message) => {
+    message = new Date() + ' ' + message;
+
+    if (SettingsModel.get('errorReporting') && isRunningInAsar()) {
+      $window.trackJs.console.debug(message);
+    }
+
     $log.debug(message);
   };
 
@@ -158,7 +133,28 @@ analytics.service('AnalyticsService', function($log, $mixpanel, SettingsModel) {
       message += ` (${JSON.stringify(data)})`;
     }
 
-    this.log(message);
+    this.logDebug(message);
+  };
+
+  /**
+   * @summary Log an exception
+   * @function
+   * @public
+   *
+   * @description
+   * This function logs an exception in TrackJS.
+   *
+   * @param {Error} exception - exception
+   *
+   * @example
+   * AnalyticsService.logException(new Error('Something happened'));
+   */
+  this.logException = (exception) => {
+    if (SettingsModel.get('errorReporting') && isRunningInAsar()) {
+      $window.trackJs.track(exception);
+    }
+
+    $log.error(exception);
   };
 
 });

lib/gui/modules/error.js

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2016 Resin.io
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+'use strict';
+
+/**
+ * @module Etcher.Modules.Error
+ */
+
+const angular = require('angular');
+
+const MODULE_NAME = 'Etcher.Modules.Error';
+const error = angular.module(MODULE_NAME, [
+  require('../modules/analytics'),
+  require('../os/dialog/dialog')
+]);
+
+error.service('ErrorService', function(AnalyticsService, OSDialogService) {
+
+  /**
+   * @summary Report an exception
+   * @function
+   * @public
+   *
+   * @param {Error} exception - exception
+   *
+   * @example
+   * ErrorService.reportException(new Error('Something happened'));
+   */
+  this.reportException = (exception) => {
+
+    // This particular error is handled by the alert ribbon
+    // on the main application page.
+    if (exception.code === 'ENOSPC') {
+      AnalyticsService.logEvent('Drive ran out of space');
+      return;
+    }
+
+    OSDialogService.showError(exception, exception.description);
+    AnalyticsService.logException(exception);
+  };
+
+});
+
+module.exports = MODULE_NAME;

lib/gui/pages/main/controllers/main.js

@@ -29,6 +29,7 @@ module.exports = function(
   DrivesModel,
   ImageWriterService,
   AnalyticsService,
+  ErrorService,
   DriveSelectorService,
   TooltipModalService,
   OSWindowProgressService,
@@ -44,25 +45,9 @@ module.exports = function(
   this.settings = SettingsModel;
   this.tooltipModal = TooltipModalService;
 
-  const handleError = (error) => {
-
-    // This particular error is handled by the alert ribbon
-    // on the main application page.
-    if (error.code === 'ENOSPC') {
-      AnalyticsService.logEvent('Drive ran out of space');
-      return;
-    }
-
-    OSDialogService.showError(error, error.description);
-
-    // Also throw it so it gets displayed in DevTools
-    // and its reported by TrackJS.
-    throw error;
-  };
-
   DriveScannerService.start();
 
-  DriveScannerService.on('error', handleError);
+  DriveScannerService.on('error', ErrorService.reportException);
 
   DriveScannerService.on('drives', (drives) => {
 
@@ -134,7 +119,7 @@ module.exports = function(
       }
 
       this.selectImage(image);
-    }).catch(handleError);
+    }).catch(ErrorService.reportException);
   };
 
   this.selectDrive = (drive) => {
@@ -152,7 +137,7 @@ module.exports = function(
   this.openDriveSelector = () => {
     DriveSelectorService.open()
       .then(this.selectDrive)
-      .catch(handleError);
+      .catch(ErrorService.reportException);
   };
 
   this.reselectImage = () => {
@@ -225,7 +210,7 @@ module.exports = function(
         AnalyticsService.logEvent('Flash error');
       }
 
-      handleError(error);
+      ErrorService.reportException(error);
     })
     .finally(OSWindowProgressService.clear);
   };

lib/gui/pages/main/main.js

@@ -43,6 +43,7 @@ const MainPage = angular.module(MODULE_NAME, [
   require('../../modules/drive-scanner'),
   require('../../modules/image-writer'),
   require('../../modules/analytics'),
+  require('../../modules/error'),
   require('../../models/selection-state'),
   require('../../models/flash-state'),
   require('../../models/settings'),