[JENKINS-29221] If a build fails to start, make sure we still perform…

… all relevant steps as if it finished normally.
jenkinsci · Jul 28, 2015 · 7db48c8 · 7db48c8
1 parent 121889f
commit 7db48c8
Showing 1 changed file with 26 additions and 24 deletions.
diff --git a/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java b/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
@@ -165,7 +165,6 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
         if (!firstTime) {
             throw sleep();
         }
-        // Some code here copied from execute(RunExecution), but subsequently modified quite a bit.
         try {
             onStartBuilding();
             OutputStream logger = new FileOutputStream(getLogFile());
@@ -175,8 +174,7 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
             updateSymlinks(listener);
             FlowDefinition definition = getParent().getDefinition();
             if (definition == null) {
-                listener.error("No flow definition, cannot run");
-                return;
+                throw new AbortException("No flow definition, cannot run");
             }
             checkouts = new LinkedList<SCMCheckout>();
             Owner owner = new Owner(this);
@@ -193,12 +191,7 @@ public WorkflowRun(WorkflowJob job, File dir) throws IOException {
             execution = newExecution;
             executionPromise.set(newExecution);
         } catch (Throwable x) {
-            if (listener == null) {
-                LOGGER.log(Level.WARNING, this + " failed to start", x);
-            } else {
-                x.printStackTrace(listener.error("failed to start build"));
-            }
-            setResult(Result.FAILURE);
+            doFinish(Result.FAILURE, x);
             executionPromise.setException(x);
             return;
         }
@@ -458,20 +451,29 @@ private String key() {
     }
 
     private void finish(Result r) {
+        doFinish(r, execution.getCauseOfFailure());
+        FlowExecutionList.get().unregister(execution.getOwner());
+    }
+
+    /** Normally called from {@link #finish} but also handles the case that the flow did not even start correctly, for example due to an error in {@link FlowExecution#start}. */
+    private void doFinish(@Nonnull Result r, @CheckForNull Throwable t) {
         setResult(r);
         LOGGER.log(Level.INFO, "{0} completed: {1}", new Object[] {this, getResult()});
         // TODO set duration
-        RunListener.fireCompleted(WorkflowRun.this, listener);
-        Throwable t = execution.getCauseOfFailure();
-        if (t instanceof AbortException) {
-            listener.error(t.getMessage());
-        } else if (t instanceof FlowInterruptedException) {
-            ((FlowInterruptedException) t).handle(this, listener);
-        } else if (t != null) {
-            t.printStackTrace(listener.getLogger());
-        }
-        listener.finished(getResult());
-        listener.closeQuietly();
+        if (listener == null) {
+            LOGGER.log(Level.WARNING, this + " failed to start", t);
+        } else {
+            RunListener.fireCompleted(WorkflowRun.this, listener);
+            if (t instanceof AbortException) {
+                listener.error(t.getMessage());
+            } else if (t instanceof FlowInterruptedException) {
+                ((FlowInterruptedException) t).handle(this, listener);
+            } else if (t != null) {
+                t.printStackTrace(listener.getLogger());
+            }
+            listener.finished(getResult());
+            listener.closeQuietly();
+        }
         logsToCopy = null;
         duration = Math.max(0, System.currentTimeMillis() - getStartTimeInMillis());
         try {
@@ -481,11 +483,11 @@ private void finish(Result r) {
             LOGGER.log(Level.WARNING, null, x);
         }
         onEndBuilding();
-        assert completed != null;
-        synchronized (completed) {
-            completed.set(true);
+        if (completed != null) {
+            synchronized (completed) {
+                completed.set(true);
+            }
         }
-        FlowExecutionList.get().unregister(execution.getOwner());
     }
 
     /**