From 63ccdeaccb5482719c161a6ddd46a2b6ae996745 Mon Sep 17 00:00:00 2001 From: Kyle Felter Date: Fri, 29 May 2026 13:11:57 -0500 Subject: [PATCH] fix: Hint at api.name config when 404 source mismatches CLI config --- cli/pkg/client.go | 43 +++++++++++++++- cli/pkg/client_test.go | 112 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 2 deletions(-) diff --git a/cli/pkg/client.go b/cli/pkg/client.go index ac3965c25..aed179a6e 100644 --- a/cli/pkg/client.go +++ b/cli/pkg/client.go @@ -56,6 +56,12 @@ type APIError struct { Body string Message string Data interface{} + // Hint is an optional human-readable suggestion appended to the error + // message. Used to surface common-misconfiguration tips that the server + // response itself cannot convey (for example, an api.name mismatch + // between the CLI's configured value and what the deployed server + // reports in its "source" field on a 404). + Hint string } func (e *APIError) Error() string { @@ -63,13 +69,17 @@ func (e *APIError) Error() string { if msg == "" { msg = e.Body } + base := fmt.Sprintf("API error %d: %s", e.StatusCode, msg) if e.Data != nil { dataJSON, err := json.Marshal(e.Data) if err == nil && string(dataJSON) != "null" { - return fmt.Sprintf("API error %d: %s\nDetails: %s", e.StatusCode, msg, string(dataJSON)) + base = fmt.Sprintf("%s\nDetails: %s", base, string(dataJSON)) } } - return fmt.Sprintf("API error %d: %s", e.StatusCode, msg) + if e.Hint != "" { + return base + "\nHint: " + e.Hint + } + return base } func NewClient(baseURL, org, token string, log *logrus.Entry, debug bool) *Client { @@ -251,6 +261,9 @@ func (c *Client) do(method, pathTemplate string, pathParams, queryParams map[str apiErr.Message = errResp.Error } apiErr.Data = errResp.Data + if hint := c.apiNameMismatchHint(resp.StatusCode, errResp.Source); hint != "" { + apiErr.Hint = hint + } } return nil, nil, apiErr } @@ -258,6 +271,32 @@ func (c *Client) do(method, pathTemplate string, pathParams, queryParams map[str return respBody, resp.Header, nil } +// apiNameMismatchHint returns a suggestion when a 404 response indicates the +// CLI's configured api.name does not match what the server serves. The +// server includes its configured API name in the "source" field of +// structured error responses (see common/pkg/util/api.go NewAPIErrorResponse). +// When the CLI is configured for a different api.name than the deployed +// server, requests miss every registered route and land in the not-found +// handler with a source field naming the server's actual api.name. Returns +// "" when the status is not 404, the source is empty (non-structured 404, or +// one from a proxy), or the source already matches the CLI's configuration. +func (c *Client) apiNameMismatchHint(statusCode int, source string) string { + if statusCode != http.StatusNotFound { + return "" + } + if source == "" { + return "" + } + if source == c.APIName { + return "" + } + return fmt.Sprintf( + "the server reports api.name %q but the CLI is configured as %q. "+ + "set api.name to %q in your config (~/.nico/config.yaml) to match.", + source, c.APIName, source, + ) +} + func formatDebugBody(body []byte) string { if len(body) == 0 { return "" diff --git a/cli/pkg/client_test.go b/cli/pkg/client_test.go index c612e1221..0246e33a6 100644 --- a/cli/pkg/client_test.go +++ b/cli/pkg/client_test.go @@ -187,3 +187,115 @@ func TestClientDoDoesNotRefreshOnForbidden(t *testing.T) { require.Equal(t, http.StatusForbidden, apiErr.StatusCode) require.Equal(t, 0, refreshes) } + +func TestAPINameMismatchHint(t *testing.T) { + cases := []struct { + name string + clientName string + statusCode int + source string + wantHas string + wantEmpty bool + }{ + { + name: "404 with mismatched source suggests server value", + clientName: "nico", + statusCode: http.StatusNotFound, + source: "carbide", + wantHas: `set api.name to "carbide"`, + }, + { + name: "404 with forge source suggests forge", + clientName: "nico", + statusCode: http.StatusNotFound, + source: "forge", + wantHas: `set api.name to "forge"`, + }, + { + name: "404 with matching source returns empty hint", + clientName: "nico", + statusCode: http.StatusNotFound, + source: "nico", + wantEmpty: true, + }, + { + name: "404 with empty source returns empty hint", + clientName: "nico", + statusCode: http.StatusNotFound, + source: "", + wantEmpty: true, + }, + { + name: "non-404 status returns empty hint", + clientName: "nico", + statusCode: http.StatusInternalServerError, + source: "carbide", + wantEmpty: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := &Client{APIName: tc.clientName} + got := c.apiNameMismatchHint(tc.statusCode, tc.source) + if tc.wantEmpty { + require.Empty(t, got) + return + } + require.Contains(t, got, tc.wantHas) + require.Contains(t, got, tc.clientName) + }) + } +} + +func TestClientDoAddsAPINameHintOn404SourceMismatch(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"source":"carbide","message":"The requested path could not be found"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-org", "token", nil, false) + _, _, err := client.Do("GET", "/v2/org/{org}/nico/site", nil, nil, nil) + apiErr, ok := err.(*APIError) + require.True(t, ok, "err = %T, want *APIError", err) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode) + require.Contains(t, apiErr.Hint, `set api.name to "carbide"`) + require.Contains(t, apiErr.Error(), "Hint:") +} + +func TestClientDoOmitsAPINameHintWhenSourceMatches(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"source":"nico","message":"site not found"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-org", "token", nil, false) + _, _, err := client.Do("GET", "/v2/org/{org}/nico/site/missing", nil, nil, nil) + apiErr, ok := err.(*APIError) + require.True(t, ok, "err = %T, want *APIError", err) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode) + require.Empty(t, apiErr.Hint) + require.NotContains(t, apiErr.Error(), "Hint:") +} + +func TestAPIErrorErrorIncludesHintAndDetails(t *testing.T) { + e := &APIError{ + StatusCode: 404, + Message: "not found", + Data: map[string]string{"path": "/x"}, + Hint: "try foo", + } + got := e.Error() + require.Contains(t, got, "API error 404") + require.Contains(t, got, "Details:") + require.Contains(t, got, "Hint: try foo") +} + +func TestAPIErrorErrorOmitsHintWhenEmpty(t *testing.T) { + e := &APIError{StatusCode: 500, Message: "boom"} + got := e.Error() + require.NotContains(t, got, "Hint:") +}