fix(web): chain Spinnaker*Exceptions in PipelineController.invokePipe…

…lineConfig

so it's clear which operation is failing.  This improves both logging and gate's http response.

As part of this, remove the no-op catch and throw for NotFoundException.  With no other
more general catch block, this code isn't necessary.

gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy

@@ -23,6 +23,8 @@ import com.netflix.spinnaker.gate.services.PipelineService
 import com.netflix.spinnaker.gate.services.TaskService
 import com.netflix.spinnaker.gate.services.internal.Front50Service
 import com.netflix.spinnaker.kork.exceptions.HasAdditionalAttributes
+import com.netflix.spinnaker.kork.exceptions.SpinnakerException
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerRetrofitErrorHandler
 import com.netflix.spinnaker.kork.web.exceptions.NotFoundException
 import com.netflix.spinnaker.security.AuthenticatedRequest
@@ -324,17 +326,30 @@ class PipelineController {
     AuthenticatedRequest.setApplication(application)
     try {
       pipelineService.trigger(application, pipelineNameOrId, trigger)
-    } catch (NotFoundException e) {
-      throw e
     } catch (RetrofitError e) {
-      throw spinnakerRetrofitErrorHandler.handleError(e)
-    } catch (e) {
-      log.error("Unable to trigger pipeline (application: {}, pipelineId: {})",
-        value("application", application), value("pipelineId", pipelineNameOrId), e)
-      throw e
+      // If spinnakerRetrofitErrorHandler were registered as a "real" error handler, the code here would look something like
+      //
+      // } catch (SpinnakerHttpException e) {
+      //   throw new SpinnakerHttpException(triggerFailureMessage(application, pipelineNameOrId, e), e)
+      // } catch (SpinnakerException e) {
+      //   throw new SpinnakerException(triggerFailureMessage(application, pipelineNameOrId, e), e)
+      // }
+      Throwable throwable = spinnakerRetrofitErrorHandler.handleError(e)
+      if (throwable instanceof SpinnakerHttpException) {
+        throw new SpinnakerHttpException(triggerFailureMessage(application, pipelineNameOrId, throwable), throwable)
+      }
+      if (throwable instanceof SpinnakerException) {
+        throw new SpinnakerException(triggerFailureMessage(application, pipelineNameOrId, throwable), throwable)
+      }
+      throw throwable
     }
   }
 
+  private String triggerFailureMessage(String application, String pipelineNameOrId, Throwable e) {
+    String.format("Unable to trigger pipeline (application: %s, pipelineId: %s). Error: %s",
+        value("application", application), value("pipelineId", pipelineNameOrId), e.getMessage())
+  }
+
   @ApiOperation(value = "Trigger a pipeline execution", response = Map.class)
   @PreAuthorize("hasPermission(#application, 'APPLICATION', 'EXECUTE')")
   @PostMapping("/v2/{application}/{pipelineNameOrId:.+}")

gate-web/src/test/java/com/netflix/spinnaker/gate/controllers/InvokePipelineConfigTest.java

@@ -241,17 +241,19 @@ void invokePipelineConfig404ResponseFromFront50() throws Exception {
     String front50ResponseJson = objectMapper.writeValueAsString(front50Response);
     simulateFront50HttpResponse(HttpStatus.NOT_FOUND, front50ResponseJson);
 
-    // gate's 404 response in this case makes it difficult to distinguish between a 404 because
-    // the endpoint itself doesn't exist and a "real" 404 -- one where all the requests are
-    // correct, but front50 doesn't know about the pipeline.  This seems fairly minor.
+    // gate's 404 response in this case makes it difficult to distinguish
+    // between a 404 because the endpoint itself doesn't exist and a "real" 404
+    // -- one where all the requests are correct, but front50 doesn't know about
+    // the pipeline.  This seems fairly minor, and callers who want to
+    // distinguish can look at more of the resposne than the status code.
     webAppMockMvc
         .perform(invokePipelineConfigRequest())
         .andDo(print())
         .andExpect(status().isNotFound())
         .andExpect(
             status()
                 .reason(
-                    "Status: 404, URL: "
+                    "Unable to trigger pipeline (application: my-application, pipelineId: my-pipeline-id). Error: Status: 404, URL: "
                         + wmFront50.baseUrl()
                         + "/pipelines/my-application?refresh=true, Message: message from front50"))
         .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID));
@@ -308,7 +310,7 @@ void invokePipelineConfigFront50BadRequest() throws Exception {
         .andExpect(
             status()
                 .reason(
-                    "Status: 400, URL: "
+                    "Unable to trigger pipeline (application: my-application, pipelineId: my-pipeline-id). Error: Status: 400, URL: "
                         + wmFront50.baseUrl()
                         + "/pipelines/my-application?refresh=true, Message: message from front50"))
         .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID));
@@ -361,7 +363,7 @@ void invokePipelineConfigFront50Error() throws Exception {
         .andExpect(
             status()
                 .reason(
-                    "Status: 500, URL: "
+                    "Unable to trigger pipeline (application: my-application, pipelineId: my-pipeline-id). Error: Status: 500, URL: "
                         + wmFront50.baseUrl()
                         + "/pipelines/my-application?refresh=true, Message: jOOQ; message from front50"))
         .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID));
@@ -447,7 +449,7 @@ void invokePipelineConfigOrcaError() throws Exception {
         .andExpect(
             status()
                 .reason(
-                    "Status: 500, URL: "
+                    "Unable to trigger pipeline (application: my-application, pipelineId: my-pipeline-id). Error: Status: 500, URL: "
                         + wmOrca.baseUrl()
                         + "/orchestrate?user=some+user, Message: message from orca"))
         .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID));
@@ -482,7 +484,7 @@ void invokePipelineConfigOrcaBadRequest() throws Exception {
         .andExpect(
             status()
                 .reason(
-                    "Status: 400, URL: "
+                    "Unable to trigger pipeline (application: my-application, pipelineId: my-pipeline-id). Error: Status: 400, URL: "
                         + wmOrca.baseUrl()
                         + "/orchestrate?user=some+user, Message: message from orca"))
         .andExpect(header().string(REQUEST_ID.getHeader(), SUBMITTED_REQUEST_ID));