Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions cli/pkg/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,30 @@ 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 {
msg := e.Message
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 {
Expand Down Expand Up @@ -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 "<empty>"
Expand Down
85 changes: 85 additions & 0 deletions cli/pkg/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:")
}
Loading