Skip to content

Commit dca155f

Browse files
committed
The problem: My initial isRetryable checked exit codes, but the backend returns
ERR_DB_ASYNC_TABLE_NOT_SETUP as a 5xx, which mapped to ExitGenericError — falsely treated as retryable. The real fix: The OperationalError is now preserved in the error chain (not flattened to a string), so isRetryable can unwrap it and check: if the server returned a structured error with an error_code, it's an intentional response that won't change on retry — so skip the retry prompt. Only offer retry for truly transient errors (network failures, unstructured 5xx).
1 parent 9c57217 commit dca155f

4 files changed

Lines changed: 34 additions & 15 deletions

File tree

internal/apierror/http.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,7 @@ func HandleHTTPErrorFromBytes(statusCode int, body []byte, contextMsg string) er
3131
)
3232
}
3333

34-
// Format user-friendly message
35-
message := opError.Format()
36-
if contextMsg != "" {
37-
message = fmt.Sprintf("%s: %s", contextMsg, message)
38-
}
39-
40-
// Map to appropriate exit code
41-
return errors.FromHTTPStatus(statusCode, message)
34+
// Wrap the OperationalError as the cause so callers can unwrap it
35+
// to inspect error codes, severity, etc.
36+
return errors.FromHTTPStatusWithCause(statusCode, opError, contextMsg)
4237
}

internal/apierror/parser.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,11 @@ func formatField(loc []interface{}) string {
170170
return strings.Join(parts, ".")
171171
}
172172

173+
// Error implements the error interface so OperationalError can be wrapped in error chains.
174+
func (e *OperationalError) Error() string {
175+
return e.Format()
176+
}
177+
173178
// Format returns a user-friendly error message
174179
func (e *OperationalError) Format() string {
175180
var parts []string

internal/commands/auth/login.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@ package auth
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"os"
78
"strings"
89
"time"
910

11+
"github.com/qlustered/qctl/internal/apierror"
1012
"github.com/qlustered/qctl/internal/auth"
1113
"github.com/qlustered/qctl/internal/auth/oauth"
1214
"github.com/qlustered/qctl/internal/cmdutil"
1315
"github.com/qlustered/qctl/internal/config"
14-
"github.com/qlustered/qctl/internal/errors"
1516
"github.com/spf13/cobra"
1617
)
1718

@@ -252,13 +253,14 @@ func exchangeWithRetry(ctx context.Context, authClient *auth.Client, kindeAccess
252253
}
253254

254255
// isRetryable returns true for transient errors worth retrying (network failures,
255-
// server 5xx). Returns false for permanent client errors (4xx) like account setup
256-
// issues, auth failures, or bad requests.
256+
// generic server errors). Returns false when the server returned a structured error
257+
// with an error code — those are intentional responses that won't change on retry.
257258
func isRetryable(err error) bool {
258-
exitCode := errors.GetExitCode(err)
259-
// ExitGenericError (1) covers 5xx and untyped errors (e.g. network failures).
260-
// All other exit codes (bad usage, not found, unauthorized) are permanent.
261-
return exitCode == errors.ExitGenericError
259+
var opErr *apierror.OperationalError
260+
if errors.As(err, &opErr) && opErr.ErrorCode != "" {
261+
return false
262+
}
263+
return true
262264
}
263265

264266
// promptRetry asks the user if they want to retry the operation

internal/errors/errors.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,23 @@ func FromHTTPStatus(statusCode int, message string) *Error {
103103
}
104104
}
105105

106+
// FromHTTPStatusWithCause maps an HTTP status code to an appropriate exit code,
107+
// wrapping the given cause error so it can be unwrapped later.
108+
func FromHTTPStatusWithCause(statusCode int, cause error, contextMsg string) *Error {
109+
switch {
110+
case statusCode == http.StatusNotFound:
111+
return Wrap(ExitNotFound, cause, contextMsg)
112+
case statusCode == http.StatusUnauthorized || statusCode == http.StatusForbidden:
113+
return Wrap(ExitUnauthorized, cause, contextMsg)
114+
case statusCode >= 500:
115+
return Wrap(ExitGenericError, cause, contextMsg)
116+
case statusCode >= 400:
117+
return Wrap(ExitBadUsage, cause, contextMsg)
118+
default:
119+
return Wrap(ExitGenericError, cause, contextMsg)
120+
}
121+
}
122+
106123
// GetExitCode extracts the exit code from an error
107124
// Returns ExitGenericError (1) if the error is not an *Error
108125
func GetExitCode(err error) int {

0 commit comments

Comments
 (0)