From 9547b06c2801a026c5250d4cf5635789492fa085 Mon Sep 17 00:00:00 2001 From: Facundo Farias Date: Fri, 29 May 2026 08:35:35 +0200 Subject: [PATCH] fix(hello): clarify account prompt and surface real validation errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Telemetry for "dhq hello" (interactive setup) showed 15 internal errors over 30d that were really auth/user failures dressed up as CLI bugs: - 8 were ".deployhq.com.deployhq.com" TLS errors (users typed their full hostname at the "Account subdomain:" prompt). - 4 were 403 (AccessDenied x2 + api_access_restricted x2). - 3 were 404 Not found on the validate-credentials API call. The URL-doubling case is already covered defensively in sdk.New (PR #15). What's left is helloLogin wrapping the validation error in *output.InternalError unconditionally, which bypasses ClassifyError's status-code fallback (PR #16) and gives users a generic "internal error" message instead of an actionable hint. Changes: - Clarify the interactive prompt to "Account subdomain (e.g. 'mycompany' for mycompany.deployhq.com):" — same example shown in the --account flag help. Lifts the hint into the moment it's needed. - Extract classifyHelloValidateErr that maps the validation API error to AuthError (401, 403) or UserError (404) or NetworkError, and returns nil for everything else so the raw error passes through. ClassifyError can then handle 5xx via its status-code fallback once PR #16 lands; if it doesn't, behavior matches main today. - Uses errors.As so the change is self-contained and doesn't depend on sdk.IsForbidden (added in PR #16). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/commands/hello.go | 48 ++++++++++++++++++++++------ internal/commands/hello_test.go | 56 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 internal/commands/hello_test.go diff --git a/internal/commands/hello.go b/internal/commands/hello.go index 44c8483..99c08d7 100644 --- a/internal/commands/hello.go +++ b/internal/commands/hello.go @@ -2,7 +2,9 @@ package commands import ( "bufio" + "errors" "fmt" + "net/http" "os" "strings" @@ -176,7 +178,7 @@ func helloAuth(env *output.Envelope) (*auth.Credentials, error) { func helloLogin(env *output.Envelope, reader *bufio.Reader) (*auth.Credentials, error) { env.Status("") - fmt.Fprint(env.Stderr, "Account subdomain: ") //nolint:errcheck + fmt.Fprint(env.Stderr, "Account subdomain (e.g. 'mycompany' for mycompany.deployhq.com): ") //nolint:errcheck account, _ := reader.ReadString('\n') account = strings.TrimSpace(account) @@ -203,16 +205,10 @@ func helloLogin(env *output.Envelope, reader *bufio.Reader) (*auth.Credentials, } if _, err := client.ListProjects(cliCtx.Background(), nil); err != nil { - if sdk.IsUnauthorized(err) { - return nil, &output.AuthError{ - Message: "Invalid credentials", - Hint: "Check your email and API key at Profile > API Key in DeployHQ", - } - } - if output.IsNetworkErr(err) { - return nil, &output.NetworkError{Message: "validate credentials", Cause: err} + if userErr := classifyHelloValidateErr(err); userErr != nil { + return nil, userErr } - return nil, &output.InternalError{Message: "validate credentials", Cause: err} + return nil, err } creds := &auth.Credentials{Account: account, Email: email, APIKey: apiKey} @@ -232,6 +228,38 @@ func helloLogin(env *output.Envelope, reader *bufio.Reader) (*auth.Credentials, return creds, nil } +// classifyHelloValidateErr maps an error from the credential-validation API +// call into a typed *output.AuthError / *output.UserError / *output.NetworkError +// when the cause is recognizable. Returns nil when the caller should pass the +// error through unchanged (which lets output.ClassifyError handle status-code +// fallback for any remaining *sdk.APIError). +func classifyHelloValidateErr(err error) error { + if output.IsNetworkErr(err) { + return &output.NetworkError{Message: "validate credentials", Cause: err} + } + var apiErr *sdk.APIError + if errors.As(err, &apiErr) { + switch apiErr.StatusCode { + case http.StatusUnauthorized: + return &output.AuthError{ + Message: "Invalid credentials", + Hint: "Check your email and API key at Profile > API Key in DeployHQ", + } + case http.StatusForbidden: + return &output.AuthError{ + Message: "Access denied", + Hint: "Your account may have API access restricted, or your user may not be a member of this account. Check Account Settings > API.", + } + case http.StatusNotFound: + return &output.UserError{ + Message: "Account not found", + Hint: "Double-check the account subdomain. List your accounts at https://www.deployhq.com/account.", + } + } + } + return nil +} + func helloSignup(env *output.Envelope, reader *bufio.Reader) (*auth.Credentials, error) { env.Status("") diff --git a/internal/commands/hello_test.go b/internal/commands/hello_test.go new file mode 100644 index 0000000..bb39a58 --- /dev/null +++ b/internal/commands/hello_test.go @@ -0,0 +1,56 @@ +package commands + +import ( + "errors" + "fmt" + "net" + "syscall" + "testing" + + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/deployhq/deployhq-cli/pkg/sdk" +) + +func TestClassifyHelloValidateErr(t *testing.T) { + tests := []struct { + name string + in error + wantNil bool + wantTyp string // class of returned typed error: "auth", "user", "network" + }{ + {"401 invalid credentials", &sdk.APIError{StatusCode: 401, Message: "Unauthorized"}, false, "auth"}, + {"403 access denied", &sdk.APIError{StatusCode: 403, Message: "AccessDenied"}, false, "auth"}, + {"403 api_access_restricted", &sdk.APIError{StatusCode: 403, Message: "api_access_restricted"}, false, "auth"}, + {"404 not found", &sdk.APIError{StatusCode: 404, Message: "Not found"}, false, "user"}, + {"500 server error returns nil so caller passes through", &sdk.APIError{StatusCode: 500, Message: "boom"}, true, ""}, + {"422 validation returns nil so caller passes through", &sdk.APIError{StatusCode: 422, Message: "bad"}, true, ""}, + {"network OpError classifies as network", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}, false, "network"}, + {"wrapped 403 via fmt.Errorf", fmt.Errorf("outer: %w", &sdk.APIError{StatusCode: 403}), false, "auth"}, + {"generic error returns nil", errors.New("???"), true, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := classifyHelloValidateErr(tt.in) + if tt.wantNil { + if got != nil { + t.Fatalf("expected nil, got %T %v", got, got) + } + return + } + switch tt.wantTyp { + case "auth": + if _, ok := got.(*output.AuthError); !ok { + t.Errorf("expected *AuthError, got %T %v", got, got) + } + case "user": + if _, ok := got.(*output.UserError); !ok { + t.Errorf("expected *UserError, got %T %v", got, got) + } + case "network": + if _, ok := got.(*output.NetworkError); !ok { + t.Errorf("expected *NetworkError, got %T %v", got, got) + } + } + }) + } +}