diff --git a/cli/pkg/client.go b/cli/pkg/client.go index 8d41c148e..78f06b050 100644 --- a/cli/pkg/client.go +++ b/cli/pkg/client.go @@ -70,6 +70,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 path-segment + // mismatch between the CLI's configured api.name and the deployed + // server's actual path). + Hint string } func (e *APIError) Error() string { @@ -77,13 +83,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 { @@ -266,12 +276,48 @@ func (c *Client) do(method, pathTemplate string, pathParams, queryParams map[str } apiErr.Data = errResp.Data } + if hint := apiNameMismatchHint(resp.StatusCode, path); hint != "" { + apiErr.Hint = hint + } return nil, nil, apiErr } return respBody, resp.Header, nil } +// apiNameMismatchHint returns a suggestion for users hitting 404 on an +// org-scoped API path whose API-name segment likely does not match what the +// server actually serves. Older (pre-rename) deployments respond on +// `/v2/org/{org}/carbide/...`; current deployments respond on +// `/v2/org/{org}/nico/...`. When a 404 lands on either of those segments, +// the most common cause is an api.name config mismatch, not a missing +// resource. Returns "" for unrelated 4xx, non-404 statuses, or paths that do +// not look API-name-segmented. +func apiNameMismatchHint(statusCode int, path string) string { + if statusCode != http.StatusNotFound { + return "" + } + m := orgScopedAPIPathPattern.FindStringSubmatch(path) + if len(m) < 3 { + return "" + } + segment := m[2] + var alternate string + switch segment { + case "nico": + alternate = "carbide" + case "carbide": + alternate = "nico" + default: + return "" + } + return fmt.Sprintf( + "the server returned 404 for path segment %q. Older deployments serve on %q. "+ + "If this resource exists, set api.name to %q in your config (~/.nico/config.yaml).", + segment, alternate, alternate, + ) +} + 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 f72f335b3..cc2f0adbd 100644 --- a/cli/pkg/client_test.go +++ b/cli/pkg/client_test.go @@ -201,3 +201,88 @@ 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 + statusCode int + path string + wantHas string + wantEmpty bool + }{ + { + name: "404 on nico segment suggests carbide", + statusCode: http.StatusNotFound, + path: "/v2/org/ncx/nico/site", + wantHas: `set api.name to "carbide"`, + }, + { + name: "404 on carbide segment suggests nico", + statusCode: http.StatusNotFound, + path: "/v2/org/ncx/carbide/site", + wantHas: `set api.name to "nico"`, + }, + { + name: "non-404 status returns empty hint", + statusCode: http.StatusInternalServerError, + path: "/v2/org/ncx/nico/site", + wantEmpty: true, + }, + { + name: "404 on non-API-name path returns empty hint", + statusCode: http.StatusNotFound, + path: "/healthz", + wantEmpty: true, + }, + { + name: "404 on unrelated org-scoped segment returns empty hint", + statusCode: http.StatusNotFound, + path: "/v2/org/ncx/something-else/site", + wantEmpty: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := apiNameMismatchHint(tc.statusCode, tc.path) + if tc.wantEmpty { + require.Empty(t, got) + return + } + require.Contains(t, got, tc.wantHas) + }) + } +} + +func TestClientDoAddsAPINameHintOn404(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, `{"message":"not found"}`, http.StatusNotFound) + })) + 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 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:") +}