feat: include errors from the sdk in outputs of erroring hooks (#109)

Author: zimeg

internal/hooks/hook_executor_default.go

@@ -72,8 +72,14 @@ func (e *HookExecutorDefaultProtocol) Execute(ctx context.Context, opts HookExec
 
 	response := strings.TrimSpace(buffout.String())
 	if err != nil {
+		// Include stderr outputs in error details if these aren't streamed
+		details := slackerror.ErrorDetails{}
+		if opts.Stderr == nil {
+			details = append(details, slackerror.ErrorDetail{Message: strings.TrimSpace(bufferr.String())})
+		}
 		return "", slackerror.New(slackerror.ErrSDKHookInvocationFailed).
-			WithMessage("Command for '%s' returned an error: %s\n%s", opts.Hook.Name, err, strings.TrimSpace(bufferr.String()))
+			WithMessage("Error running '%s' command: %s", opts.Hook.Name, err).
+			WithDetails(details)
 	}
 
 	// Special handling for the baseline protocol for the `start` hook

internal/hooks/hook_executor_default_test.go

@@ -90,7 +90,12 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
 				},
 			},
 			expectedError: slackerror.New(slackerror.ErrSDKHookInvocationFailed).
-				WithMessage("Command for 'sadpath' returned an error: explosion\nthere was a problem compiling your app"),
+				WithMessage("Error running 'sadpath' command: explosion").
+				WithDetails(slackerror.ErrorDetails{
+					slackerror.ErrorDetail{
+						Message: "there was a problem compiling your app",
+					},
+				}),
 			expectedResponse: "",
 		},
 		"successful deploy command": {

internal/hooks/hook_executor_v2.go

@@ -86,8 +86,14 @@ func (e *HookExecutorMessageBoundaryProtocol) Execute(ctx context.Context, opts
 
 	cmd := opts.Exec.Command(cmdEnvVars, &stdout, stderr, opts.Stdin, cmdArgs[0], cmdArgVars...)
 	if err = cmd.Run(); err != nil {
+		// Include stderr outputs in error details if these aren't streamed
+		details := slackerror.ErrorDetails{}
+		if opts.Stderr == nil {
+			details = append(details, slackerror.ErrorDetail{Message: strings.TrimSpace(bufferr.String())})
+		}
 		return "", slackerror.New(slackerror.ErrSDKHookInvocationFailed).
-			WithMessage("Error running '%s' command: %s", opts.Hook.Name, err)
+			WithMessage("Error running '%s' command: %s", opts.Hook.Name, err).
+			WithDetails(details)
 	}
 	return buffout.String(), nil
 }

internal/hooks/hook_executor_v2_test.go

@@ -113,13 +113,21 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
 				Hook: HookScript{Command: "boom", Name: "sadpath"},
 				Exec: &MockExec{
 					mockCommand: &MockCommand{
-						Err: errors.New("explosion"),
+						Err:        errors.New("explosion"),
+						MockStderr: []byte("fireworks for the skies above"),
 					},
 				},
 			},
 			check: func(t *testing.T, response string, err error, mockExec ExecInterface) {
 				require.Equal(t, slackerror.New(slackerror.ErrSDKHookInvocationFailed).
-					WithMessage("Error running 'sadpath' command: explosion"), err)
+					WithMessage("Error running 'sadpath' command: explosion").
+					WithDetails(slackerror.ErrorDetails{
+						slackerror.ErrorDetail{
+							Message: "fireworks for the skies above",
+						},
+					}),
+					err,
+				)
 			},
 		},
 		"fail to parse payload due to improper boundary strings": {

internal/shared/clients_test.go

@@ -234,7 +234,7 @@ func Test_ClientFactory_InitSDKConfigFromJSON_brokenGetHooks(t *testing.T) {
 	err := clients.InitSDKConfigFromJSON(ctx, []byte(getHooksJSON))
 	require.Error(t, err)
 	assert.Equal(t, slackerror.New(slackerror.ErrSDKHookInvocationFailed).Code, slackerror.ToSlackError(err).Code)
-	assert.Contains(t, slackerror.ToSlackError(err).Message, "Command for 'GetHooks' returned an error")
+	assert.Contains(t, slackerror.ToSlackError(err).Message, "Error running 'GetHooks' command")
 }
 
 func Test_ClientFactory_InitSDKConfigFromJSON_brokenJSONFile(t *testing.T) {