fix(core/task): properly cleanup TaskMonitor polling, fix digest thra…

…shing (spinnaker#7458)
christopherthielen · Sep 30, 2019 · 57d28c9 · 57d28c9
1 parent 564af88
commit 57d28c9
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 10 deletions.
diff --git a/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts b/app/scripts/modules/core/src/task/monitor/TaskMonitor.ts
@@ -42,8 +42,14 @@ export class TaskMonitor {
     return {
       deferred,
       result: deferred.promise,
-      close: onClose,
-      dismiss: onDismiss || onClose,
+      close: (result: T) => {
+        deferred.resolve(result);
+        return onClose(result);
+      },
+      dismiss: (result: T) => {
+        deferred.reject(result);
+        return (onDismiss || onClose)(result);
+      },
     } as IModalServiceInstanceEmulation;
   }
 

diff --git a/app/scripts/modules/core/src/task/task.read.service.ts b/app/scripts/modules/core/src/task/task.read.service.ts
@@ -26,17 +26,13 @@ export class TaskReader {
     return API.one('tasks', taskId)
       .get()
       .then((task: ITask) => {
-        OrchestratedItemTransformer.defineProperties(task);
-        if (task.steps && task.steps.length) {
-          task.steps.forEach(step => OrchestratedItemTransformer.defineProperties(step));
-        }
+        this.setTaskProperties(task);
         if (task.execution) {
           OrchestratedItemTransformer.defineProperties(task.execution);
           if (task.execution.stages) {
             task.execution.stages.forEach((stage: any) => OrchestratedItemTransformer.defineProperties(stage));
           }
         }
-        this.setTaskProperties(task);
         return task;
       })
       .catch((error: any) => $log.warn('There was an issue retrieving taskId: ', taskId, error));

diff --git a/app/scripts/modules/core/src/task/tasks.controller.js b/app/scripts/modules/core/src/task/tasks.controller.js
@@ -57,6 +57,7 @@ module.exports = angular
         };
         viewState.nameFilter = $stateParams.q || '';
         viewState.loading = true;
+        viewState.cancelling = false;
         viewState.itemsPerPage = tasksViewStateCache.get('#common')
           ? tasksViewStateCache.get('#common').itemsPerPage
           : 20;
@@ -147,7 +148,17 @@ module.exports = angular
           return task.id === taskId;
         })[0];
         var submitMethod = function() {
-          return TaskWriter.cancelTask(taskId).then(() => application.tasks.refresh());
+          // cancelTask() polls aggressively waiting for a sucessful cancellation,
+          // which triggers equally aggressive updates to the runningTimeInMs field
+          // on the hydrated task object. Because we render that field in templates,
+          // updating so quickly can make Angular think we're doing Bad Things(tm)
+          // and abort its change detection. Instead, we stop rendering that field
+          // while the polling is active.
+          $scope.viewState.cancelling = true;
+          return TaskWriter.cancelTask(taskId).then(() => {
+            $scope.viewState.cancelling = false;
+            application.tasks.refresh();
+          });
         };
 
         confirmationModalService.confirm({

diff --git a/app/scripts/modules/core/src/task/tasks.html b/app/scripts/modules/core/src/task/tasks.html
@@ -139,10 +139,10 @@ <h4>
                 {{ task.endTime | timestamp }}
               </td>
               <td>
-                <span ng-if="task.runningTimeInMs">
+                <span ng-if="task.runningTimeInMs && !viewState.cancelling">
                   {{ task.runningTimeInMs | duration }}
                 </span>
-                <span ng-if="!task.runningTimeInMs">
+                <span ng-if="viewState.cancelling || !task.runningTimeInMs">
                   -
                 </span>
               </td>