From ffaf36c1d09f802e70b88bc424262db77cdef0f3 Mon Sep 17 00:00:00 2001 From: prankur-sh Date: Mon, 5 Oct 2026 11:43:45 +0530 Subject: [PATCH] fix: clarify the error when an API key is no longer valid --- pkg/store/http.go | 10 ++++ pkg/store/http_response_error_test.go | 68 +++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 pkg/store/http_response_error_test.go diff --git a/pkg/store/http.go b/pkg/store/http.go index 0a227078..ed77c159 100644 --- a/pkg/store/http.go +++ b/pkg/store/http.go @@ -11,6 +11,7 @@ import ( "slices" "strings" + "github.com/brevdev/brev-cli/pkg/auth" "github.com/brevdev/brev-cli/pkg/cmd/version" "github.com/brevdev/brev-cli/pkg/entity" breverrors "github.com/brevdev/brev-cli/pkg/errors" @@ -317,7 +318,16 @@ func NewHTTPResponseError(response *resty.Response) *HTTPResponseError { } } +const expiredAPIKeyMessage = "Your API key is invalid or has expired. Generate a new API key and run 'brev login --api-key ' (or pass --api-key )" + func (e HTTPResponseError) Error() string { + if e.Response.StatusCode() == http.StatusUnauthorized && e.Response.Request != nil && auth.IsBrevAPIKey(e.Response.Request.Token) { + return expiredAPIKeyMessage + "\n" + e.baseError() + } + return e.baseError() +} + +func (e HTTPResponseError) baseError() string { body := e.Response.Body() if featureflag.Debug() { return fmt.Sprintf("%s %s %s", e.Response.Request.URL, e.Response.Status(), body) diff --git a/pkg/store/http_response_error_test.go b/pkg/store/http_response_error_test.go new file mode 100644 index 00000000..83e0adce --- /dev/null +++ b/pkg/store/http_response_error_test.go @@ -0,0 +1,68 @@ +package store + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + resty "github.com/go-resty/resty/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const unauthorizedBody = `{"errors":[{"type":"UnauthorizedError"}]}` + +func doRequestWithToken(t *testing.T, status int, body, token string) *HTTPResponseError { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + defer server.Close() + + req := resty.New().R() + if token != "" { + req.SetAuthToken(token) + } + res, err := req.Get(server.URL) + require.NoError(t, err) + require.True(t, res.IsError()) + return NewHTTPResponseError(res) +} + +func TestHTTPResponseError_UnauthorizedWithAPIKeyShowsHintThenOriginalMessage(t *testing.T) { + err := doRequestWithToken(t, http.StatusUnauthorized, unauthorizedBody, "bak-expired") + + msg := err.Error() + assert.True(t, strings.HasPrefix(msg, expiredAPIKeyMessage+"\n"), "hint should come first: %q", msg) + assert.Contains(t, msg, "brev login --api-key") + // the original error is preserved after the hint + assert.True(t, strings.HasSuffix(msg, err.baseError())) + assert.Contains(t, msg, "401 Unauthorized") + assert.Contains(t, msg, "UnauthorizedError") +} + +func TestHTTPResponseError_UnauthorizedWithoutAPIKeyKeepsOriginalMessage(t *testing.T) { + // A non-API-key (e.g. browser login JWT) 401 must not claim the API key is bad. + err := doRequestWithToken(t, http.StatusUnauthorized, unauthorizedBody, "eyJhbGciOiJIUzI1NiJ9.e30.sig") + + msg := err.Error() + assert.NotContains(t, msg, expiredAPIKeyMessage) + assert.Contains(t, msg, "401") + assert.Contains(t, msg, "UnauthorizedError") +} + +func TestHTTPResponseError_UnauthorizedWithNoTokenKeepsOriginalMessage(t *testing.T) { + err := doRequestWithToken(t, http.StatusUnauthorized, unauthorizedBody, "") + + assert.NotContains(t, err.Error(), expiredAPIKeyMessage) +} + +func TestHTTPResponseError_NonUnauthorizedWithAPIKeyKeepsOriginalMessage(t *testing.T) { + // Only a 401 means the key is expired; other failures must surface as-is. + err := doRequestWithToken(t, http.StatusForbidden, `{"errors":[{"message":"forbidden"}]}`, "bak-valid") + + assert.Equal(t, "forbidden\n", err.Error()) +}