Skip to content

Commit c9034d5

Browse files
somanshreddyclaude
andcommitted
cli: reclassify parse/read failures + add source/http_status telemetry
Reclassify the remaining opaque `error` sites: a mid-response read failure -> network_error; JSON-parse and poll field-extraction failures -> response_parse_error. Capture the request id from x-amzn-request-id / x-amzn-trace-id (the app does not set X-Request-Id; the ALB does), falling back to X-Request-Id. Add telemetry-only origin signals to COMMAND_RUN_COMPLETE: source (api = from the API envelope, cli = CLI-generated; defaults to cli) and http_status (upstream status when known). Both are omitted on success and are NOT in the user-facing error envelope. Dashboards can then group by (source, error_code) collision-safely and separate an app 5xx from a no-envelope 5xx via source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a010fc9 commit c9034d5

7 files changed

Lines changed: 202 additions & 27 deletions

File tree

cmd/heygen/main.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,25 +26,33 @@ func main() {
2626

2727
exitCode := 0
2828
errorCode := ""
29+
source := ""
30+
httpStatus := 0
2931
if err != nil {
3032
var cliErr *clierrors.CLIError
3133
if errors.As(err, &cliErr) {
3234
enrichAuthHint(cliErr, credSourceFromCmd(executedCmd))
3335
formatter.Error(cliErr)
3436
exitCode = cliErr.ExitCode
3537
errorCode = cliErr.Code
38+
source = cliErr.Source
39+
if source == "" {
40+
source = "cli" // any error the CLI raised without an explicit origin
41+
}
42+
httpStatus = cliErr.HTTPStatus
3643
} else {
3744
// Cobra returns plain errors for unknown commands and arg validation.
3845
// Detect these and wrap as usage errors (exit 2).
3946
wrapped := classifyError(err)
4047
formatter.Error(wrapped)
4148
exitCode = wrapped.ExitCode
4249
errorCode = wrapped.Code
50+
source = "cli"
4351
}
4452
}
4553

4654
if analyticsClient.Started() && executedCmd != nil {
47-
analyticsClient.CommandRunComplete(executedCmd.CommandPath(), exitCode, time.Since(start), errorCode)
55+
analyticsClient.CommandRunComplete(executedCmd.CommandPath(), exitCode, time.Since(start), errorCode, source, httpStatus)
4856
}
4957
analyticsClient.Close()
5058
os.Exit(exitCode)

internal/analytics/analytics.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,28 @@ func (c *Client) CommandRun(command string) {
7070
})
7171
}
7272

73-
func (c *Client) CommandRunComplete(command string, exitCode int, duration time.Duration, errorCode string) {
73+
// CommandRunComplete records a completed command. On error, source is "api" (the
74+
// error came from an API response envelope) or "cli" (CLI-generated), and httpStatus
75+
// is the upstream status when known (0 otherwise); both are omitted on success.
76+
func (c *Client) CommandRunComplete(command string, exitCode int, duration time.Duration, errorCode, source string, httpStatus int) {
7477
if !c.enabled || c.ph == nil {
7578
return
7679
}
80+
props := c.baseProperties(command).
81+
Set("exit_code", exitCode).
82+
Set("duration_ms", duration.Milliseconds()).
83+
Set("success", exitCode == 0).
84+
Set("error_code", errorCode)
85+
if source != "" {
86+
props = props.Set("source", source)
87+
}
88+
if httpStatus > 0 {
89+
props = props.Set("http_status", httpStatus)
90+
}
7791
_ = c.ph.Enqueue(posthog.Capture{
7892
DistinctId: c.distinctID,
7993
Event: "COMMAND_RUN_COMPLETE",
80-
Properties: c.baseProperties(command).
81-
Set("exit_code", exitCode).
82-
Set("duration_ms", duration.Milliseconds()).
83-
Set("success", exitCode == 0).
84-
Set("error_code", errorCode),
94+
Properties: props,
8595
})
8696
}
8797

internal/analytics/analytics_test.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestCommandRunComplete_Properties(t *testing.T) {
5959
client := newWithCapture("v1.2.3", stub)
6060
client.distinctID = "anon-id"
6161

62-
client.CommandRunComplete("heygen video create", 4, 1500*time.Millisecond, "timeout")
62+
client.CommandRunComplete("heygen video create", 4, 1500*time.Millisecond, "timeout", "cli", 0)
6363

6464
if len(stub.messages) != 1 {
6565
t.Fatalf("messages = %d, want 1", len(stub.messages))
@@ -107,7 +107,7 @@ func TestCommandRunComplete_IncludesClientOrigin(t *testing.T) {
107107
client := newWithCapture("v1.2.3", stub)
108108
client.clientOrigin = "claude_code"
109109

110-
client.CommandRunComplete("heygen video create", 0, time.Second, "")
110+
client.CommandRunComplete("heygen video create", 0, time.Second, "", "", 0)
111111

112112
msg := stub.messages[0].(posthog.Capture)
113113
if got := msg.Properties["client_origin"]; got != "claude_code" {
@@ -225,3 +225,39 @@ func TestDistinctID_Persists(t *testing.T) {
225225
t.Fatalf("second distinct ID = %q, want %q", second, first)
226226
}
227227
}
228+
229+
func TestCommandRunComplete_SourceAndHTTPStatus(t *testing.T) {
230+
t.Run("api source carries http_status", func(t *testing.T) {
231+
stub := &stubCaptureClient{}
232+
client := newWithCapture("v1.2.3", stub)
233+
client.CommandRunComplete("heygen video create", 1, time.Second, "insufficient_credit", "api", 402)
234+
msg := stub.messages[0].(posthog.Capture)
235+
if got := msg.Properties["source"]; got != "api" {
236+
t.Fatalf("source = %v, want api", got)
237+
}
238+
if got := msg.Properties["http_status"]; got != 402 {
239+
t.Fatalf("http_status = %v, want 402", got)
240+
}
241+
})
242+
t.Run("cli source omits http_status when 0", func(t *testing.T) {
243+
stub := &stubCaptureClient{}
244+
client := newWithCapture("v1.2.3", stub)
245+
client.CommandRunComplete("heygen video create", 1, time.Second, "file_io_error", "cli", 0)
246+
msg := stub.messages[0].(posthog.Capture)
247+
if got := msg.Properties["source"]; got != "cli" {
248+
t.Fatalf("source = %v, want cli", got)
249+
}
250+
if _, ok := msg.Properties["http_status"]; ok {
251+
t.Fatalf("http_status should be omitted when 0")
252+
}
253+
})
254+
t.Run("success omits source", func(t *testing.T) {
255+
stub := &stubCaptureClient{}
256+
client := newWithCapture("v1.2.3", stub)
257+
client.CommandRunComplete("heygen video list", 0, time.Second, "", "", 0)
258+
msg := stub.messages[0].(posthog.Capture)
259+
if _, ok := msg.Properties["source"]; ok {
260+
t.Fatalf("source should be omitted on success")
261+
}
262+
})
263+
}

internal/client/executor.go

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,13 @@ func (c *Client) ExecuteAndPoll(
8181

8282
resourceID, err := extractJSONPath(createResp, spec.PollConfig.IDField)
8383
if err != nil {
84-
return nil, clierrors.New(fmt.Sprintf(
85-
"failed to extract resource ID from %q: %v. This command may require manual polling for batch responses",
86-
spec.PollConfig.IDField, err,
87-
))
84+
return nil, &clierrors.CLIError{
85+
Code: "response_parse_error",
86+
Message: fmt.Sprintf(
87+
"failed to extract resource ID from %q: %v", spec.PollConfig.IDField, err),
88+
Hint: "The create response did not have the expected shape. This command may require manual polling for batch responses.",
89+
ExitCode: clierrors.ExitGeneral,
90+
}
8891
}
8992

9093
// If IDField uses an array index (e.g., "data.ids.0"), reject batch
@@ -131,10 +134,13 @@ func (c *Client) ExecuteAndPoll(
131134

132135
status, err := extractJSONPath(statusResp, spec.PollConfig.StatusField)
133136
if err != nil {
134-
return nil, clierrors.New(fmt.Sprintf(
135-
"failed to extract status from %q: %v",
136-
spec.PollConfig.StatusField, err,
137-
))
137+
return nil, &clierrors.CLIError{
138+
Code: "response_parse_error",
139+
Message: fmt.Sprintf(
140+
"failed to extract status from %q: %v", spec.PollConfig.StatusField, err),
141+
Hint: "The status response did not have the expected shape. Retry; if it persists, report it.",
142+
ExitCode: clierrors.ExitGeneral,
143+
}
138144
}
139145

140146
if slices.Contains(spec.PollConfig.TerminalOK, status) {
@@ -212,11 +218,17 @@ func (c *Client) executeWithContext(ctx context.Context, spec *command.Spec, inv
212218

213219
respBody, err := io.ReadAll(resp.Body)
214220
if err != nil {
215-
return nil, clierrors.New(fmt.Sprintf("failed to read response body: %v", err))
221+
// Mid-response read failure is a dropped connection, not a parse issue.
222+
return nil, &clierrors.CLIError{
223+
Code: "network_error",
224+
Message: fmt.Sprintf("failed to read response body: %v", err),
225+
Hint: "This is usually a temporary network issue. Check your connection and retry.",
226+
ExitCode: clierrors.ExitGeneral,
227+
}
216228
}
217229

218230
if resp.StatusCode >= 400 {
219-
return nil, parseErrorResponse(resp.StatusCode, respBody, resp.Header.Get("X-Request-Id"))
231+
return nil, parseErrorResponse(resp.StatusCode, respBody, requestIDFromHeaders(resp.Header))
220232
}
221233

222234
return json.RawMessage(respBody), nil
@@ -394,7 +406,12 @@ func extractJSONPath(raw json.RawMessage, path string) (string, error) {
394406

395407
var current any
396408
if err := json.Unmarshal(raw, &current); err != nil {
397-
return "", clierrors.New(fmt.Sprintf("failed to parse JSON response: %v", err))
409+
return "", &clierrors.CLIError{
410+
Code: "response_parse_error",
411+
Message: fmt.Sprintf("failed to parse JSON response: %v", err),
412+
Hint: "The API response could not be parsed. Retry; if it persists, report it (possible CLI/API mismatch).",
413+
ExitCode: clierrors.ExitGeneral,
414+
}
398415
}
399416

400417
for _, part := range strings.Split(path, ".") {
@@ -515,3 +532,15 @@ func parseErrorResponse(statusCode int, body []byte, requestID string) *clierror
515532

516533
return clierrors.FromAPIError(statusCode, &clierrors.APIError{}, requestID)
517534
}
535+
536+
// requestIDFromHeaders returns the best available request/trace id for support
537+
// correlation. The v3 app does not set X-Request-Id; the AWS ALB sets x-amzn-*, so
538+
// prefer the app header when present, then fall back to the ALB's.
539+
func requestIDFromHeaders(h http.Header) string {
540+
for _, k := range []string{"X-Request-Id", "X-Amzn-Request-Id", "X-Amzn-Trace-Id"} {
541+
if v := h.Get(k); v != "" {
542+
return v
543+
}
544+
}
545+
return ""
546+
}

internal/client/executor_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,3 +997,61 @@ func TestParseErrorResponse_SurfacesParamAndDocURL(t *testing.T) {
997997
t.Errorf("DocURL = %q, want the doc URL", err.DocURL)
998998
}
999999
}
1000+
1001+
func TestRequestIDFromHeaders(t *testing.T) {
1002+
tests := []struct {
1003+
name string
1004+
h http.Header
1005+
want string
1006+
}{
1007+
{"app X-Request-Id preferred", http.Header{"X-Request-Id": {"app-1"}, "X-Amzn-Request-Id": {"amzn-1"}}, "app-1"},
1008+
{"falls back to x-amzn-request-id", http.Header{"X-Amzn-Request-Id": {"amzn-1"}}, "amzn-1"},
1009+
{"falls back to x-amzn-trace-id", http.Header{"X-Amzn-Trace-Id": {"trace-1"}}, "trace-1"},
1010+
{"none present", http.Header{}, ""},
1011+
}
1012+
for _, tt := range tests {
1013+
t.Run(tt.name, func(t *testing.T) {
1014+
if got := requestIDFromHeaders(tt.h); got != tt.want {
1015+
t.Errorf("requestIDFromHeaders = %q, want %q", got, tt.want)
1016+
}
1017+
})
1018+
}
1019+
}
1020+
1021+
func TestExtractJSONPath_ParseError(t *testing.T) {
1022+
_, err := extractJSONPath([]byte("{not valid json"), "data.id")
1023+
var cliErr *clierrors.CLIError
1024+
if !errors.As(err, &cliErr) || cliErr.Code != "response_parse_error" {
1025+
t.Fatalf("err = %v, want a response_parse_error CLIError", err)
1026+
}
1027+
}
1028+
1029+
// errReadCloser fails on Read, simulating a connection dropped mid-response.
1030+
type errReadCloser struct{}
1031+
1032+
func (errReadCloser) Read([]byte) (int, error) { return 0, errors.New("connection reset mid-body") }
1033+
func (errReadCloser) Close() error { return nil }
1034+
1035+
type errBodyTransport struct{}
1036+
1037+
func (errBodyTransport) RoundTrip(*http.Request) (*http.Response, error) {
1038+
return &http.Response{StatusCode: 200, Body: errReadCloser{}, Header: make(http.Header)}, nil
1039+
}
1040+
1041+
func TestExecute_ResponseBodyReadError_NetworkError(t *testing.T) {
1042+
c := New("key", WithBaseURL("http://example.invalid"), WithHTTPClient(&http.Client{Transport: errBodyTransport{}}))
1043+
_, err := c.Execute(&command.Spec{Endpoint: "/v3/videos", Method: "GET"}, &command.Invocation{PathParams: map[string]string{}})
1044+
var cliErr *clierrors.CLIError
1045+
if !errors.As(err, &cliErr) || cliErr.Code != "network_error" {
1046+
t.Fatalf("err = %v, want a network_error CLIError", err)
1047+
}
1048+
}
1049+
1050+
func TestRequestIDFromHeaders_CaseInsensitive(t *testing.T) {
1051+
// A lowercase ALB header (as parsed/canonicalized by net/http) is still found.
1052+
h := http.Header{}
1053+
h.Set("x-amzn-request-id", "amzn-lower")
1054+
if got := requestIDFromHeaders(h); got != "amzn-lower" {
1055+
t.Errorf("requestIDFromHeaders = %q, want amzn-lower", got)
1056+
}
1057+
}

internal/errors/errors.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ type CLIError struct {
2323
DocURL string // API: link to this error code's documentation
2424
RequestID string // from API X-Request-Id header (if applicable)
2525
ExitCode int // process exit code (0/1/2/3/4)
26+
27+
// Telemetry-only (NOT serialized in ToErrorEnvelope — internal origin signal):
28+
Source string // "api" (from the API envelope) or "cli" (CLI-generated). Empty defaults to "cli".
29+
HTTPStatus int // upstream HTTP status when the error came from an API response (0 otherwise)
2630
}
2731

2832
// Error implements the error interface.
@@ -184,14 +188,23 @@ func FromAPIError(statusCode int, apiErr *APIError, requestID string) *CLIError
184188
docURL = *apiErr.DocURL
185189
}
186190

191+
// Origin for telemetry: a preserved (specific) API code came from the API
192+
// envelope; a status-derived code was generated by the CLI (non-envelope / edge).
193+
source := "cli"
194+
if !isNonSpecific(apiErr.Code) {
195+
source = "api"
196+
}
197+
187198
return &CLIError{
188-
Code: code,
189-
Message: message,
190-
Hint: hint,
191-
Param: param,
192-
DocURL: docURL,
193-
RequestID: requestID,
194-
ExitCode: exitCode,
199+
Code: code,
200+
Message: message,
201+
Hint: hint,
202+
Param: param,
203+
DocURL: docURL,
204+
RequestID: requestID,
205+
ExitCode: exitCode,
206+
Source: source,
207+
HTTPStatus: statusCode,
195208
}
196209
}
197210

internal/errors/errors_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,24 @@ func TestToErrorEnvelope_OmitsDocURLAndParamWhenEmpty(t *testing.T) {
362362
t.Error("doc_url should be omitted when empty")
363363
}
364364
}
365+
366+
func TestFromAPIError_SourceAndHTTPStatus(t *testing.T) {
367+
// A preserved (specific) API code originates from the API envelope.
368+
got := FromAPIError(402, &APIError{Code: "insufficient_credit", Message: "x"}, "")
369+
if got.Source != "api" || got.HTTPStatus != 402 {
370+
t.Errorf("Source/HTTPStatus = %q/%d, want api/402", got.Source, got.HTTPStatus)
371+
}
372+
// A status-derived (non-specific) code is CLI-generated.
373+
derived := FromAPIError(500, &APIError{Message: "x"}, "")
374+
if derived.Source != "cli" || derived.HTTPStatus != 500 {
375+
t.Errorf("Source/HTTPStatus = %q/%d, want cli/500", derived.Source, derived.HTTPStatus)
376+
}
377+
// Source/HTTPStatus are telemetry-only — never serialized in the envelope.
378+
inner := got.ToErrorEnvelope()["error"].(map[string]any)
379+
if _, ok := inner["source"]; ok {
380+
t.Error("source must not appear in the error envelope")
381+
}
382+
if _, ok := inner["http_status"]; ok {
383+
t.Error("http_status must not appear in the error envelope")
384+
}
385+
}

0 commit comments

Comments
 (0)