From aa87a524e20f36871119c2067ca0695424083a63 Mon Sep 17 00:00:00 2001
From: James Rhodes <jrhodes@redpointsoftware.com.au>
Date: Fri, 1 Aug 2014 08:08:28 +1000
Subject: [PATCH] Allow build steps to explicitly fail the build

Summary: We've received feedback that the "core - exception" is incredibly confusing, to the point where developers see this and write off the build failure as a Phabricator error that is unrelated to their changes.

Test Plan: Ran a build with a `exit 1` run step, didn't see the "core - exception" appear.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D10090
---
 src/__phutil_library_map__.php                               | 2 ++
 .../exception/HarbormasterBuildFailureException.php          | 5 +++++
 .../step/HarbormasterCommandBuildStepImplementation.php      | 2 +-
 .../harbormaster/worker/HarbormasterTargetWorker.php         | 4 ++++
 4 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 src/applications/harbormaster/exception/HarbormasterBuildFailureException.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 452e1f631e..68b9c13a9c 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -654,6 +654,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildCommand' => 'applications/harbormaster/storage/HarbormasterBuildCommand.php',
     'HarbormasterBuildDependencyDatasource' => 'applications/harbormaster/typeahead/HarbormasterBuildDependencyDatasource.php',
     'HarbormasterBuildEngine' => 'applications/harbormaster/engine/HarbormasterBuildEngine.php',
+    'HarbormasterBuildFailureException' => 'applications/harbormaster/exception/HarbormasterBuildFailureException.php',
     'HarbormasterBuildGraph' => 'applications/harbormaster/engine/HarbormasterBuildGraph.php',
     'HarbormasterBuildItem' => 'applications/harbormaster/storage/build/HarbormasterBuildItem.php',
     'HarbormasterBuildItemPHIDType' => 'applications/harbormaster/phid/HarbormasterBuildItemPHIDType.php',
@@ -3394,6 +3395,7 @@ phutil_register_library_map(array(
     'HarbormasterBuildCommand' => 'HarbormasterDAO',
     'HarbormasterBuildDependencyDatasource' => 'PhabricatorTypeaheadDatasource',
     'HarbormasterBuildEngine' => 'Phobject',
+    'HarbormasterBuildFailureException' => 'Exception',
     'HarbormasterBuildGraph' => 'AbstractDirectedGraph',
     'HarbormasterBuildItem' => 'HarbormasterDAO',
     'HarbormasterBuildItemPHIDType' => 'PhabricatorPHIDType',
diff --git a/src/applications/harbormaster/exception/HarbormasterBuildFailureException.php b/src/applications/harbormaster/exception/HarbormasterBuildFailureException.php
new file mode 100644
index 0000000000..6edb437f94
--- /dev/null
+++ b/src/applications/harbormaster/exception/HarbormasterBuildFailureException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class HarbormasterBuildFailureException extends Exception {
+
+}
diff --git a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php
index 70e791adc2..552ee7f996 100644
--- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php
@@ -68,7 +68,7 @@ final class HarbormasterCommandBuildStepImplementation
     $log_stderr->finalize($start_stderr);
 
     if ($err) {
-      throw new Exception(pht('Command failed with error %d.', $err));
+      throw new HarbormasterBuildFailureException();
     }
   }
 
diff --git a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
index 47e492f4ee..da61cfe82e 100644
--- a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
+++ b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
@@ -56,6 +56,10 @@ final class HarbormasterTargetWorker extends HarbormasterWorker {
       // If the target wants to yield, let that escape without further
       // processing. We'll resume after the task retries.
       throw $ex;
+    } catch (HarbormasterBuildFailureException $ex) {
+      // A build step wants to fail explicitly.
+      $target->setTargetStatus(HarbormasterBuildTarget::STATUS_FAILED);
+      $target->save();
     } catch (Exception $ex) {
       phlog($ex);