fix(cli): preserve deploy error handling paths
- commit
- 913d8fc5aa1f3dc500f00712287602cf704b39a1
- author
- brice · · 2026-04-06 13:11 UTC
- parent
- 40add0f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +84 -32
M internal/adapters/in/cli/deploy.go +33 -31
| @@ -6,10 +6,15 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/spf13/cobra" | |
| 8 | 8 | ||
| 9 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |
| 9 | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | |
| 10 | 11 | "github.com/bnema/gordon/internal/app" | |
| 11 | 12 | ) | |
| 12 | 13 | ||
| 14 | + | type deployer interface { | |
| 15 | + | Deploy(ctx context.Context, deployDomain string) (*remote.DeployResult, error) | |
| 16 | + | } | |
| 17 | + | ||
| 13 | 18 | // newDeployCmd creates the deploy command. | |
| 14 | 19 | func newDeployCmd() *cobra.Command { | |
| 15 | 20 | return &cobra.Command{ | |
| @@ -27,40 +32,37 @@ Examples: | |||
| 27 | 32 | gordon deploy myapp.example.com --remote https://gordon.mydomain.com --token $TOKEN`, | |
| 28 | 33 | Args: cobra.ExactArgs(1), | |
| 29 | 34 | RunE: func(cmd *cobra.Command, args []string) error { | |
| 30 | - | ctx := context.Background() | |
| 31 | - | deployDomain := args[0] | |
| 32 | - | ||
| 33 | - | client, isRemote := GetRemoteClient() | |
| 34 | - | if isRemote { | |
| 35 | - | // Remote deployment via admin API | |
| 36 | - | result, err := client.Deploy(ctx, deployDomain) | |
| 37 | - | if err != nil { | |
| 38 | - | if shouldFallbackToLocal(err) { | |
| 39 | - | domain, localErr := app.SendDeploySignal(deployDomain) | |
| 40 | - | if localErr == nil { | |
| 41 | - | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | |
| 42 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |
| 43 | - | return nil | |
| 44 | - | } | |
| 45 | - | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | |
| 46 | - | } | |
| 47 | - | return formatDeployFailure(err) | |
| 48 | - | } | |
| 49 | - | containerID := result.ContainerID | |
| 50 | - | if len(containerID) > 12 { | |
| 51 | - | containerID = containerID[:12] | |
| 52 | - | } | |
| 53 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |
| 54 | - | return nil | |
| 55 | - | } | |
| 56 | - | ||
| 57 | - | // Local deployment via signal | |
| 58 | - | domain, err := app.SendDeploySignal(deployDomain) | |
| 35 | + | handle, err := resolveControlPlane(configPath) | |
| 59 | 36 | if err != nil { | |
| 60 | 37 | return err | |
| 61 | 38 | } | |
| 62 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |
| 63 | - | return nil | |
| 39 | + | defer handle.close() | |
| 40 | + | ||
| 41 | + | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0]) | |
| 64 | 42 | }, | |
| 65 | 43 | } | |
| 66 | 44 | } | |
| 45 | + | ||
| 46 | + | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string) error { | |
| 47 | + | result, err := deployer.Deploy(ctx, deployDomain) | |
| 48 | + | if err != nil { | |
| 49 | + | if isRemote && shouldFallbackToLocal(err) { | |
| 50 | + | domain, localErr := app.SendDeploySignal(deployDomain) | |
| 51 | + | if localErr == nil { | |
| 52 | + | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | |
| 53 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |
| 54 | + | return nil | |
| 55 | + | } | |
| 56 | + | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | |
| 57 | + | } | |
| 58 | + | ||
| 59 | + | return formatDeployFailure(err) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | containerID := result.ContainerID | |
| 63 | + | if len(containerID) > 12 { | |
| 64 | + | containerID = containerID[:12] | |
| 65 | + | } | |
| 66 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |
| 67 | + | return nil | |
| 68 | + | } | |
| @@ -6,10 +6,15 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/spf13/cobra" | 7 | "github.com/spf13/cobra" | ||
| 8 | 8 | ||||
| 9 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |||
| 9 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | 10 | "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles" | ||
| 10 | "github.com/bnema/gordon/internal/app" | 11 | "github.com/bnema/gordon/internal/app" | ||
| 11 | ) | 12 | ) | ||
| 12 | 13 | ||||
| 14 | + | type deployer interface { | |||
| 15 | + | Deploy(ctx context.Context, deployDomain string) (*remote.DeployResult, error) | |||
| 16 | + | } | |||
| 17 | + | ||||
| 13 | // newDeployCmd creates the deploy command. | 18 | // newDeployCmd creates the deploy command. | ||
| 14 | func newDeployCmd() *cobra.Command { | 19 | func newDeployCmd() *cobra.Command { | ||
| 15 | return &cobra.Command{ | 20 | return &cobra.Command{ | ||
| @@ -27,40 +32,37 @@ Examples: | |||||
| 27 | gordon deploy myapp.example.com --remote https://gordon.mydomain.com --token $TOKEN`, | 32 | gordon deploy myapp.example.com --remote https://gordon.mydomain.com --token $TOKEN`, | ||
| 28 | Args: cobra.ExactArgs(1), | 33 | Args: cobra.ExactArgs(1), | ||
| 29 | RunE: func(cmd *cobra.Command, args []string) error { | 34 | RunE: func(cmd *cobra.Command, args []string) error { | ||
| 30 | - | ctx := context.Background() | 35 | + | handle, err := resolveControlPlane(configPath) |
| 31 | - | deployDomain := args[0] | |||
| 32 | - | ||||
| 33 | - | client, isRemote := GetRemoteClient() | |||
| 34 | - | if isRemote { | |||
| 35 | - | // Remote deployment via admin API | |||
| 36 | - | result, err := client.Deploy(ctx, deployDomain) | |||
| 37 | - | if err != nil { | |||
| 38 | - | if shouldFallbackToLocal(err) { | |||
| 39 | - | domain, localErr := app.SendDeploySignal(deployDomain) | |||
| 40 | - | if localErr == nil { | |||
| 41 | - | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | |||
| 42 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |||
| 43 | - | return nil | |||
| 44 | - | } | |||
| 45 | - | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | |||
| 46 | - | } | |||
| 47 | - | return formatDeployFailure(err) | |||
| 48 | - | } | |||
| 49 | - | containerID := result.ContainerID | |||
| 50 | - | if len(containerID) > 12 { | |||
| 51 | - | containerID = containerID[:12] | |||
| 52 | - | } | |||
| 53 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |||
| 54 | - | return nil | |||
| 55 | - | } | |||
| 56 | - | ||||
| 57 | - | // Local deployment via signal | |||
| 58 | - | domain, err := app.SendDeploySignal(deployDomain) | |||
| 59 | if err != nil { | 36 | if err != nil { | ||
| 60 | return err | 37 | return err | ||
| 61 | } | 38 | } | ||
| 62 | - | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | 39 | + | defer handle.close() |
| 63 | - | return nil | 40 | + | |
| 41 | + | return runDeploy(cmd.Context(), handle.plane, handle.isRemote, args[0]) | |||
| 64 | }, | 42 | }, | ||
| 65 | } | 43 | } | ||
| 66 | } | 44 | } | ||
| 45 | + | ||||
| 46 | + | func runDeploy(ctx context.Context, deployer deployer, isRemote bool, deployDomain string) error { | |||
| 47 | + | result, err := deployer.Deploy(ctx, deployDomain) | |||
| 48 | + | if err != nil { | |||
| 49 | + | if isRemote && shouldFallbackToLocal(err) { | |||
| 50 | + | domain, localErr := app.SendDeploySignal(deployDomain) | |||
| 51 | + | if localErr == nil { | |||
| 52 | + | fmt.Println(styles.RenderWarning(fmt.Sprintf("Remote deploy failed (%v), used local signal fallback", err))) | |||
| 53 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deploy signal sent for domain: %s", domain))) | |||
| 54 | + | return nil | |||
| 55 | + | } | |||
| 56 | + | return fmt.Errorf("failed to deploy: remote error: %w; local fallback also failed: %v", err, localErr) | |||
| 57 | + | } | |||
| 58 | + | ||||
| 59 | + | return formatDeployFailure(err) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | containerID := result.ContainerID | |||
| 63 | + | if len(containerID) > 12 { | |||
| 64 | + | containerID = containerID[:12] | |||
| 65 | + | } | |||
| 66 | + | fmt.Println(styles.RenderSuccess(fmt.Sprintf("Deployed %s (container: %s)", deployDomain, containerID))) | |||
| 67 | + | return nil | |||
| 68 | + | } | |||
A internal/adapters/in/cli/deploy_test.go +33 -0
| @@ -0,0 +1,33 @@ | |||
| 1 | + | package cli | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |
| 11 | + | "github.com/bnema/gordon/internal/domain" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | type deployTestDeployer struct { | |
| 15 | + | result *remote.DeployResult | |
| 16 | + | err error | |
| 17 | + | } | |
| 18 | + | ||
| 19 | + | func (d *deployTestDeployer) Deploy(context.Context, string) (*remote.DeployResult, error) { | |
| 20 | + | return d.result, d.err | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func TestRunDeploy_LocalTypedErrorUsesDeployFormatter(t *testing.T) { | |
| 24 | + | err := runDeploy(context.Background(), &deployTestDeployer{err: &domain.DeployFailureError{ | |
| 25 | + | Summary: "failed to deploy", | |
| 26 | + | Cause: "health check failed", | |
| 27 | + | Hint: "check DATABASE_URL", | |
| 28 | + | Logs: []string{"booting app", "connection refused"}, | |
| 29 | + | }}, false, "app.example.com") | |
| 30 | + | ||
| 31 | + | require.Error(t, err) | |
| 32 | + | assert.Equal(t, "failed to deploy\nCause: health check failed\nHint: check DATABASE_URL\n\nRecent container logs:\n booting app\n connection refused", err.Error()) | |
| 33 | + | } | |
| @@ -0,0 +1,33 @@ | |||||
| 1 | + | package cli | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/gordon/internal/adapters/in/cli/remote" | |||
| 11 | + | "github.com/bnema/gordon/internal/domain" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | type deployTestDeployer struct { | |||
| 15 | + | result *remote.DeployResult | |||
| 16 | + | err error | |||
| 17 | + | } | |||
| 18 | + | ||||
| 19 | + | func (d *deployTestDeployer) Deploy(context.Context, string) (*remote.DeployResult, error) { | |||
| 20 | + | return d.result, d.err | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func TestRunDeploy_LocalTypedErrorUsesDeployFormatter(t *testing.T) { | |||
| 24 | + | err := runDeploy(context.Background(), &deployTestDeployer{err: &domain.DeployFailureError{ | |||
| 25 | + | Summary: "failed to deploy", | |||
| 26 | + | Cause: "health check failed", | |||
| 27 | + | Hint: "check DATABASE_URL", | |||
| 28 | + | Logs: []string{"booting app", "connection refused"}, | |||
| 29 | + | }}, false, "app.example.com") | |||
| 30 | + | ||||
| 31 | + | require.Error(t, err) | |||
| 32 | + | assert.Equal(t, "failed to deploy\nCause: health check failed\nHint: check DATABASE_URL\n\nRecent container logs:\n booting app\n connection refused", err.Error()) | |||
| 33 | + | } | |||
M internal/adapters/in/cli/remote/client.go +1 -1
| @@ -343,7 +343,7 @@ func parseErrorResponse(resp *http.Response, body []byte) error { | |||
| 343 | 343 | Logs []string `json:"logs"` | |
| 344 | 344 | } | |
| 345 | 345 | if err := json.Unmarshal(body, &errResp); err == nil { | |
| 346 | - | structured = errResp.Error != "" || errResp.Cause != "" || errResp.Hint != "" || len(errResp.Logs) > 0 | |
| 346 | + | structured = errResp.Cause != "" || errResp.Hint != "" || len(errResp.Logs) > 0 | |
| 347 | 347 | if errResp.Error != "" { | |
| 348 | 348 | msg = errResp.Error | |
| 349 | 349 | } | |
| @@ -343,7 +343,7 @@ func parseErrorResponse(resp *http.Response, body []byte) error { | |||||
| 343 | Logs []string `json:"logs"` | 343 | Logs []string `json:"logs"` | ||
| 344 | } | 344 | } | ||
| 345 | if err := json.Unmarshal(body, &errResp); err == nil { | 345 | if err := json.Unmarshal(body, &errResp); err == nil { | ||
| 346 | - | structured = errResp.Error != "" || errResp.Cause != "" || errResp.Hint != "" || len(errResp.Logs) > 0 | 346 | + | structured = errResp.Cause != "" || errResp.Hint != "" || len(errResp.Logs) > 0 |
| 347 | if errResp.Error != "" { | 347 | if errResp.Error != "" { | ||
| 348 | msg = errResp.Error | 348 | msg = errResp.Error | ||
| 349 | } | 349 | } | ||
M internal/adapters/in/cli/remote/client_test.go +17 -0
| @@ -351,6 +351,23 @@ func TestParseErrorResponse_DeployFailureFields(t *testing.T) { | |||
| 351 | 351 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | |
| 352 | 352 | } | |
| 353 | 353 | ||
| 354 | + | func TestParseErrorResponse_ErrorOnlyJSONNotStructured(t *testing.T) { | |
| 355 | + | resp := &http.Response{ | |
| 356 | + | StatusCode: 500, | |
| 357 | + | Status: "500 Internal Server Error", | |
| 358 | + | Body: io.NopCloser(strings.NewReader("")), | |
| 359 | + | } | |
| 360 | + | err := parseErrorResponse(resp, []byte(`{"error":"failed to deploy container"}`)) | |
| 361 | + | ||
| 362 | + | var httpErr *HTTPError | |
| 363 | + | require.True(t, errors.As(err, &httpErr)) | |
| 364 | + | assert.Equal(t, "failed to deploy container", httpErr.Body) | |
| 365 | + | assert.False(t, httpErr.Structured) | |
| 366 | + | assert.Empty(t, httpErr.Cause) | |
| 367 | + | assert.Empty(t, httpErr.Hint) | |
| 368 | + | assert.Empty(t, httpErr.Logs) | |
| 369 | + | } | |
| 370 | + | ||
| 354 | 371 | func TestParseErrorResponse_NonJSON(t *testing.T) { | |
| 355 | 372 | resp := &http.Response{ | |
| 356 | 373 | StatusCode: 500, | |
| @@ -351,6 +351,23 @@ func TestParseErrorResponse_DeployFailureFields(t *testing.T) { | |||||
| 351 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | 351 | assert.Equal(t, []string{"booting app", "connection refused"}, httpErr.Logs) | ||
| 352 | } | 352 | } | ||
| 353 | 353 | ||||
| 354 | + | func TestParseErrorResponse_ErrorOnlyJSONNotStructured(t *testing.T) { | |||
| 355 | + | resp := &http.Response{ | |||
| 356 | + | StatusCode: 500, | |||
| 357 | + | Status: "500 Internal Server Error", | |||
| 358 | + | Body: io.NopCloser(strings.NewReader("")), | |||
| 359 | + | } | |||
| 360 | + | err := parseErrorResponse(resp, []byte(`{"error":"failed to deploy container"}`)) | |||
| 361 | + | ||||
| 362 | + | var httpErr *HTTPError | |||
| 363 | + | require.True(t, errors.As(err, &httpErr)) | |||
| 364 | + | assert.Equal(t, "failed to deploy container", httpErr.Body) | |||
| 365 | + | assert.False(t, httpErr.Structured) | |||
| 366 | + | assert.Empty(t, httpErr.Cause) | |||
| 367 | + | assert.Empty(t, httpErr.Hint) | |||
| 368 | + | assert.Empty(t, httpErr.Logs) | |||
| 369 | + | } | |||
| 370 | + | ||||
| 354 | func TestParseErrorResponse_NonJSON(t *testing.T) { | 371 | func TestParseErrorResponse_NonJSON(t *testing.T) { | ||
| 355 | resp := &http.Response{ | 372 | resp := &http.Response{ | ||
| 356 | StatusCode: 500, | 373 | StatusCode: 500, | ||