Merge pull request #1770 from anotherchrisberry/rx-on-error-surprise

do not call onError from an Observable

app/scripts/modules/core/application/service/applications.read.service.js

@@ -102,13 +102,13 @@ module.exports = angular
  })
  .catch(function(rejection) {
  // Gate will send back a 429 error code (TOO_MANY_REQUESTS) and will be caught here
- // As a quick fix we are just adding an empty list to of tasks to the
+ // As a quick fix we are just adding an empty list to the
  // application, which will let the user know that no tasks were found for the app.
  addTasksToApplication(application, []);
  $log.warn('Error retrieving [tasks]', rejection);
- application.taskRefreshStream.onError(rejection);
  application.tasksLoading = false;
  application.tasksLoadFailure = true;
+ application.taskRefreshStream.onNext(rejection);
  });
  }
 
@@ -128,14 +128,14 @@ module.exports = angular
  application.executionsLoaded = true;
  application.executionsLoading = false;
  application.executionsLoadFailure = false;
- application.executionRefreshStream.onNext(true);
+ application.executionRefreshStream.onNext();
  })
  .catch(function(rejection) {
  // Gate will send back a 429 error code (TOO_MANY_REQUESTS) and will be caught here.
  $log.warn('Error retrieving [executions]', rejection);
- application.executionRefreshStream.onError(rejection);
  application.executionsLoading = false;
  application.executionsLoadFailure = true;
+ application.executionRefreshStream.onNext(rejection);
  });
  }
 
@@ -151,12 +151,15 @@ module.exports = angular
  .then((configs) => {
  application.pipelineConfigs = configs.pipelines;
  application.strategyConfigs = configs.strategies;
+ application.pipelineConfigsLoaded = true;
  application.pipelineConfigsLoading = false;
+ application.pipelineConfigsLoadFailure = false;
  application.pipelineConfigRefreshStream.onNext();
  }).catch(function(rejection) {
  $log.warn('Error retrieving [pipelineConfigs]', rejection);
- application.pipelineConfigRefreshStream.onError();
  application.pipelineConfigsLoading = false;
+ application.pipelineConfigsLoadFailure = true;
+ application.pipelineConfigRefreshStream.onNext();
  });
  }
 
@@ -295,7 +298,7 @@ module.exports = angular
  executionService.getExecutions(applicationName) :
  executionService.getRunningExecutions(applicationName) :
  $q.when(null),
- tasksLoader = options && options.loadAllTasks ?
+ tasksLoader = options && options.tasks ?
  options.loadAllTasks ?
  taskReader.getTasks(applicationName) :
  taskReader.getRunningTasks(applicationName) :
@@ -321,8 +324,9 @@ module.exports = angular
  application.tasksLoadFailure = false;
  },
  () => {
- application.tasksLoadFailure = true;
+ application.tasksLoaded = false;
  application.tasksLoading = false;
+ application.tasksLoadFailure = true;
  }
  );
  }
@@ -337,6 +341,7 @@ module.exports = angular
  application.executionsLoading = false;
  },
  () => {
+ application.executionsLoaded = false;
  application.executionsLoadFailure = true;
  application.executionsLoading = false;
  }

app/scripts/modules/core/application/service/applications.read.service.spec.js

@@ -7,6 +7,9 @@ describe('Service: applicationReader', function () {
  var securityGroupReader;
  var loadBalancerReader;
  var clusterService;
+ var executionService;
+ var pipelineConfigService;
+ var taskReader;
  var $http;
  var $q;
  var $scope;
@@ -18,12 +21,17 @@ describe('Service: applicationReader', function () {
  );
 
  beforeEach(
- window.inject(function (_applicationReader_, _securityGroupReader_, _clusterService_, $httpBackend, _$q_, _loadBalancerReader_, $rootScope) {
+ window.inject(function (_applicationReader_, _securityGroupReader_, _clusterService_, $httpBackend, _$q_,
+ _loadBalancerReader_, _executionService_, _taskReader_, _pipelineConfigService_,
+ $rootScope) {
  applicationReader = _applicationReader_;
  securityGroupReader = _securityGroupReader_;
  clusterService = _clusterService_;
  loadBalancerReader = _loadBalancerReader_;
  $http = $httpBackend;
+ executionService = _executionService_;
+ pipelineConfigService = _pipelineConfigService_;
+ taskReader = _taskReader_;
  $q = _$q_;
  application = {};
  $scope = $rootScope.$new();
@@ -32,7 +40,7 @@ describe('Service: applicationReader', function () {
 
  describe('load application', function () {
 
- function loadApplication(serverGroups, loadBalancers, securityGroupsByApplicationName) {
+ function loadApplication(serverGroups, loadBalancers, securityGroupsByApplicationName, options) {
  $http.expectGET('/applications/deck').respond(200, {name: 'deck', attributes: {}});
  spyOn(securityGroupReader, 'loadSecurityGroupsByApplicationName').and.returnValue($q.when(securityGroupsByApplicationName));
  spyOn(loadBalancerReader, 'loadLoadBalancers').and.returnValue($q.when(loadBalancers));
@@ -43,9 +51,222 @@ describe('Service: applicationReader', function () {
  return $q.when(app);
  });
 
- return applicationReader.getApplication('deck');
+ return applicationReader.getApplication('deck', options);
  }
 
+ describe('loading executions', function () {
+ it('loads executions and sets appropriate flags', function () {
+ spyOn(executionService, 'getRunningExecutions').and.returnValue($q.when([{status: 'COMPLETED', stages: []}]));
+ var result = null;
+ loadApplication([], [], [], {executions: true}).then((app) => {
+ result = app;
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.executionsLoaded).toBe(true);
+ expect(result.executionsLoading).toBe(false);
+ expect(result.executionsLoadFailure).toBe(false);
+ });
+
+ it('sets appropriate flags when execution load fails', function () {
+ spyOn(executionService, 'getRunningExecutions').and.returnValue($q.reject(null));
+ var result = null;
+ loadApplication([], [], [], {executions: true}).then((app) => {
+ result = app;
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.executionsLoaded).toBe(false);
+ expect(result.executionsLoading).toBe(false);
+ expect(result.executionsLoadFailure).toBe(true);
+ });
+ });
+
+ describe('reload executions', function () {
+ it('reloads executions and sets appropriate flags', function () {
+ spyOn(executionService, 'getRunningExecutions').and.returnValue($q.when([{status: 'COMPLETED', stages: []}]));
+ var result = null,
+ nextCalls = 0;
+ loadApplication([], [], [], {executions: true}).then((app) => {
+ result = app;
+ result.executionRefreshStream.subscribe(() => nextCalls++);
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.executionsLoaded).toBe(true);
+ expect(result.executionsLoading).toBe(false);
+ expect(result.executionsLoadFailure).toBe(false);
+
+ result.reloadExecutions();
+ expect(result.executionsLoading).toBe(true);
+
+ $scope.$digest();
+ expect(result.executionsLoaded).toBe(true);
+ expect(result.executionsLoading).toBe(false);
+ expect(result.executionsLoadFailure).toBe(false);
+
+ expect(nextCalls).toBe(1);
+ });
+
+ it('sets appropriate flags when executions reload fails; subscriber is responsible for error checking', function () {
+ spyOn(executionService, 'getRunningExecutions').and.returnValue($q.reject(null));
+ var result = null,
+ errorsHandled = 0;
+ loadApplication([], [], []).then((app) => {
+ result = app;
+ result.executionRefreshStream.subscribe(() => {
+ if (result.executionsLoadFailure) {
+ errorsHandled++;
+ }
+ });
+ });
+ $scope.$digest();
+ $http.flush();
+
+ result.reloadExecutions();
+ $scope.$digest();
+
+ expect(result.executionsLoading).toBe(false);
+ expect(result.executionsLoadFailure).toBe(true);
+
+ result.reloadExecutions();
+ $scope.$digest();
+
+ expect(errorsHandled).toBe(2);
+ });
+ });
+
+ describe('loading tasks', function () {
+ it('loads tasks and sets appropriate flags', function () {
+ spyOn(taskReader, 'getRunningTasks').and.returnValue($q.when([{status: 'COMPLETED'}]));
+ var result = null;
+ loadApplication([], [], [], {tasks: true}).then((app) => {
+ result = app;
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.tasksLoaded).toBe(true);
+ expect(result.tasksLoading).toBe(false);
+ expect(result.tasksLoadFailure).toBe(false);
+ });
+
+ it('sets appropriate flags when task load fails', function () {
+ spyOn(taskReader, 'getRunningTasks').and.returnValue($q.reject(null));
+ var result = null;
+ loadApplication([], [], [], {tasks: true}).then((app) => {
+ result = app;
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.tasksLoaded).toBe(false);
+ expect(result.tasksLoading).toBe(false);
+ expect(result.tasksLoadFailure).toBe(true);
+ });
+ });
+
+ describe('reload tasks', function () {
+ it('reloads tasks and sets appropriate flags', function () {
+ spyOn(taskReader, 'getRunningTasks').and.returnValue($q.when([{status: 'COMPLETED'}]));
+ var result = null,
+ nextCalls = 0;
+ loadApplication([], [], [], {tasks: true}).then((app) => {
+ result = app;
+ result.taskRefreshStream.subscribe(() => nextCalls++);
+ });
+ $scope.$digest();
+ $http.flush();
+ expect(result.tasksLoaded).toBe(true);
+ expect(result.tasksLoading).toBe(false);
+ expect(result.tasksLoadFailure).toBe(false);
+
+ result.reloadTasks();
+ expect(result.tasksLoading).toBe(true);
+
+ $scope.$digest();
+ expect(result.tasksLoaded).toBe(true);
+ expect(result.tasksLoading).toBe(false);
+ expect(result.tasksLoadFailure).toBe(false);
+
+ expect(nextCalls).toBe(1);
+ });
+
+ it('sets appropriate flags when task reload fails; subscriber is responsible for error checking', function () {
+ spyOn(taskReader, 'getRunningTasks').and.returnValue($q.reject(null));
+ var result = null,
+ errorsHandled = 0;
+ loadApplication([], [], []).then((app) => {
+ result = app;
+ result.taskRefreshStream.subscribe(() => {
+ if (result.tasksLoadFailure) {
+ errorsHandled++;
+ }
+ });
+ });
+ $scope.$digest();
+ $http.flush();
+
+ result.reloadTasks();
+ $scope.$digest();
+
+ expect(result.tasksLoading).toBe(false);
+ expect(result.tasksLoadFailure).toBe(true);
+
+ result.reloadTasks();
+ $scope.$digest();
+
+ expect(errorsHandled).toBe(2);
+ });
+ });
+
+ describe('loading pipeline configs', function () {
+ it('loads configs and sets appropriate flags', function () {
+ spyOn(pipelineConfigService, 'getPipelinesForApplication').and.returnValue($q.when([]));
+ spyOn(pipelineConfigService, 'getStrategiesForApplication').and.returnValue($q.when([]));
+ var result = null;
+ loadApplication([], [], []).then((app) => {
+ result = app;
+ });
+ $scope.$digest();
+ $http.flush();
+
+ result.reloadPipelineConfigs();
+ expect(result.pipelineConfigsLoading).toBe(true);
+ $scope.$digest();
+
+ expect(result.pipelineConfigsLoaded).toBe(true);
+ expect(result.pipelineConfigsLoading).toBe(false);
+ expect(result.pipelineConfigsLoadFailure).toBe(false);
+ });
+
+ it('sets appropriate flags when pipeline config reload fails; subscriber is responsible for error checking', function () {
+ spyOn(pipelineConfigService, 'getPipelinesForApplication').and.returnValue($q.when([]));
+ spyOn(pipelineConfigService, 'getStrategiesForApplication').and.returnValue($q.reject([]));
+ var result = null,
+ errorsHandled = 0;
+ loadApplication([], [], []).then((app) => {
+ result = app;
+ result.pipelineConfigRefreshStream.subscribe(() => {
+ if (result.pipelineConfigsLoadFailure) {
+ errorsHandled++;
+ }
+ });
+ });
+ $scope.$digest();
+ $http.flush();
+
+ result.reloadPipelineConfigs();
+ $scope.$digest();
+
+ expect(result.pipelineConfigsLoading).toBe(false);
+ expect(result.pipelineConfigsLoadFailure).toBe(true);
+
+ result.reloadPipelineConfigs();
+ $scope.$digest();
+
+ expect(errorsHandled).toBe(2);
+ });
+ });
+
  describe('setting default credentials and regions', function () {
  it('sets default credentials and region from server group when only one account/region found', function () {
  var serverGroups = [{

app/scripts/modules/core/delivery/executions/executions.controller.js

@@ -78,7 +78,15 @@ module.exports = angular.module('spinnaker.core.delivery.executions.controller',
 
  });
 
+ let dataInitializationFailure = () => {
+ this.viewState.loading = false;
+ this.viewState.initializationError = true;
+ };
+
  function normalizeExecutionNames() {
+ if (application.executionsLoadFailure) {
+ dataInitializationFailure();
+ }
  let executions = application.executions || [];
  var configurations = application.pipelineConfigs || [];
  executions.forEach(function(execution) {
@@ -93,11 +101,6 @@ module.exports = angular.module('spinnaker.core.delivery.executions.controller',
  });
  }
 
- let dataInitializationFailure = () => {
- this.viewState.loading = false;
- this.viewState.initializationError = true;
- };
-
  let executionWatcher = this.application.executionRefreshStream.subscribe(normalizeExecutionNames, dataInitializationFailure);
  $scope.$on('$destroy', () => executionWatcher.dispose());
 

app/scripts/modules/core/delivery/filter/executionFilter.controller.js

@@ -37,6 +37,9 @@ module.exports = angular.module('spinnaker.core.delivery.filter.executionFilter.
  };
 
  this.initialize = () => {
+ if (this.application.pipelineConfigsLoadFailure) {
+ return;
+ }
  let allOptions = _.sortBy(this.application.pipelineConfigs, 'index')
  .concat(this.application.executions)
  .filter((option) => option && option.name)