From 96c5d8d743de5c94592982005cd97f40cde0c5d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cvuth-dogo=E2=80=9D?= Date: Fri, 3 Apr 2026 22:03:23 +0700 Subject: [PATCH 1/2] fix: improve raw API diagnostics for invalid or empty JSON responses Wrap JSON decode failures from DoAPI and HandleResponse in api_error exit details with a hint to use --output when the endpoint may return non-JSON payloads. Co-Authored-By: Paperclip Made-with: Cursor --- cmd/api/api.go | 2 +- cmd/api/api_test.go | 43 ++++++++++++++++++++++ internal/client/api_errors.go | 62 ++++++++++++++++++++++++++++++++ internal/client/response.go | 2 +- internal/client/response_test.go | 31 ++++++++++++++++ 5 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 internal/client/api_errors.go diff --git a/cmd/api/api.go b/cmd/api/api.go index 89661b36..01feb51e 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -195,7 +195,7 @@ func apiRun(opts *APIOptions) error { resp, err := ac.DoAPI(opts.Ctx, request) if err != nil { - return output.MarkRaw(output.ErrNetwork("API call failed: %v", err)) + return output.MarkRaw(client.WrapDoAPIError(err)) } err = client.HandleResponse(resp, client.ResponseOptions{ OutputPath: opts.Output, diff --git a/cmd/api/api_test.go b/cmd/api/api_test.go index 0b01d250..e06e1c01 100644 --- a/cmd/api/api_test.go +++ b/cmd/api/api_test.go @@ -500,6 +500,49 @@ func TestApiCmd_APIError_PreservesOriginalMessage(t *testing.T) { } } +func TestApiCmd_InvalidJSONResponse_ShowsDiagnostic(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app-invalidjson", AppSecret: "test-secret-invalidjson", Brand: core.BrandFeishu, + }) + + reg.Register(&httpmock.Stub{ + URL: "/open-apis/auth/v3/tenant_access_token/internal", + Body: map[string]interface{}{ + "code": 0, "msg": "ok", + "tenant_access_token": "t-test-token-invalidjson", "expire": 7200, + }, + }) + reg.Register(&httpmock.Stub{ + URL: "/open-apis/test/invalidjson", + RawBody: []byte{}, + ContentType: "application/json", + }) + + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"GET", "/open-apis/test/invalidjson", "--as", "bot"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *output.ExitError, got %T", err) + } + if exitErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI, got %d", exitErr.Code) + } + if exitErr.Detail == nil { + t.Fatal("expected detail on exit error") + } + if !strings.Contains(exitErr.Detail.Message, "invalid JSON response") { + t.Fatalf("expected invalid JSON diagnostic, got %q", exitErr.Detail.Message) + } + if !strings.Contains(exitErr.Detail.Hint, "--output") { + t.Fatalf("expected hint to mention --output, got %q", exitErr.Detail.Hint) + } +} + func TestApiCmd_PageAll_APIError_IsRaw(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app-rawpage", AppSecret: "test-secret-rawpage", Brand: core.BrandFeishu, diff --git a/internal/client/api_errors.go b/internal/client/api_errors.go new file mode 100644 index 00000000..7014ed6b --- /dev/null +++ b/internal/client/api_errors.go @@ -0,0 +1,62 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package client + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "strings" + + "github.com/larksuite/cli/internal/output" +) + +const rawAPIJSONHint = "The endpoint may have returned an empty or non-standard JSON body. If it returns a file, rerun with --output." + +// WrapDoAPIError upgrades malformed JSON decode errors from the SDK into +// actionable API errors for raw `lark-cli api` calls. All other failures +// remain network errors. +func WrapDoAPIError(err error) error { + if err == nil { + return nil + } + if isJSONDecodeError(err) { + return output.ErrWithHint(output.ExitAPI, "api_error", + fmt.Sprintf("API returned an invalid JSON response: %v", err), rawAPIJSONHint) + } + return output.ErrNetwork("API call failed: %v", err) +} + +// WrapJSONResponseParseError upgrades empty or malformed JSON response bodies +// into API errors with hints instead of generic parse failures. +func WrapJSONResponseParseError(err error, body []byte) error { + if err == nil { + return nil + } + if len(bytes.TrimSpace(body)) == 0 { + return output.ErrWithHint(output.ExitAPI, "api_error", + "API returned an empty JSON response body", rawAPIJSONHint) + } + if isJSONDecodeError(err) { + return output.ErrWithHint(output.ExitAPI, "api_error", + fmt.Sprintf("API returned an invalid JSON response: %v", err), rawAPIJSONHint) + } + return output.ErrNetwork("API call failed: %v", err) +} + +func isJSONDecodeError(err error) bool { + var syntaxErr *json.SyntaxError + var unmarshalTypeErr *json.UnmarshalTypeError + + if errors.Is(err, io.EOF) || errors.As(err, &syntaxErr) || errors.As(err, &unmarshalTypeErr) { + return true + } + + msg := err.Error() + return strings.Contains(msg, "unexpected end of JSON input") || + strings.Contains(msg, "invalid character") || + strings.Contains(msg, "cannot unmarshal") +} diff --git a/internal/client/response.go b/internal/client/response.go index db34400b..a4e92bde 100644 --- a/internal/client/response.go +++ b/internal/client/response.go @@ -55,7 +55,7 @@ func HandleResponse(resp *larkcore.ApiResp, opts ResponseOptions) error { if IsJSONContentType(ct) || ct == "" { result, err := ParseJSONResponse(resp) if err != nil { - return output.ErrNetwork("API call failed: %v", err) + return WrapJSONResponseParseError(err, resp.RawBody) } if apiErr := check(result); apiErr != nil { return apiErr diff --git a/internal/client/response_test.go b/internal/client/response_test.go index 0de09f97..593b45c2 100644 --- a/internal/client/response_test.go +++ b/internal/client/response_test.go @@ -219,6 +219,37 @@ func TestHandleResponse_JSONWithError(t *testing.T) { } } +func TestHandleResponse_EmptyJSONBody_ShowsDiagnostic(t *testing.T) { + resp := newApiResp([]byte{}, map[string]string{"Content-Type": "application/json"}) + + var out bytes.Buffer + var errOut bytes.Buffer + err := HandleResponse(resp, ResponseOptions{ + Out: &out, + ErrOut: &errOut, + }) + if err == nil { + t.Fatal("expected error for empty JSON body") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected ExitError, got %T", err) + } + if exitErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI, got %d", exitErr.Code) + } + if exitErr.Detail == nil { + t.Fatal("expected detail on exit error") + } + if exitErr.Detail.Message != "API returned an empty JSON response body" { + t.Fatalf("unexpected message: %q", exitErr.Detail.Message) + } + if !strings.Contains(exitErr.Detail.Hint, "--output") { + t.Fatalf("expected hint to mention --output, got %q", exitErr.Detail.Hint) + } +} + func TestHandleResponse_BinaryAutoSave(t *testing.T) { dir := t.TempDir() origWd, _ := os.Getwd() From a40a62872bb3c59ac31aa4546d88a714332cb2e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cvuth-dogo=E2=80=9D?= Date: Fri, 3 Apr 2026 22:18:29 +0700 Subject: [PATCH 2/2] fix: tighten API JSON decode error classification after review - Do not treat bare io.EOF from DoAPI as JSON parse path - Treat ErrUnexpectedEOF / partial decode as JSON diagnostics in response path - Wrap decode errors with %w in ParseJSONResponse for errors.Is - Adjust tests and add api_errors unit coverage Co-Authored-By: Paperclip Made-with: Cursor --- cmd/api/api_test.go | 5 ++- internal/client/api_errors.go | 14 ++++-- internal/client/api_errors_test.go | 68 ++++++++++++++++++++++++++++++ internal/client/response.go | 2 +- internal/client/response_test.go | 12 ++++++ 5 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 internal/client/api_errors_test.go diff --git a/cmd/api/api_test.go b/cmd/api/api_test.go index e06e1c01..9630d516 100644 --- a/cmd/api/api_test.go +++ b/cmd/api/api_test.go @@ -535,8 +535,9 @@ func TestApiCmd_InvalidJSONResponse_ShowsDiagnostic(t *testing.T) { if exitErr.Detail == nil { t.Fatal("expected detail on exit error") } - if !strings.Contains(exitErr.Detail.Message, "invalid JSON response") { - t.Fatalf("expected invalid JSON diagnostic, got %q", exitErr.Detail.Message) + if !strings.Contains(exitErr.Detail.Message, "invalid JSON response") && + !strings.Contains(exitErr.Detail.Message, "empty JSON response body") { + t.Fatalf("expected JSON diagnostic, got %q", exitErr.Detail.Message) } if !strings.Contains(exitErr.Detail.Hint, "--output") { t.Fatalf("expected hint to mention --output, got %q", exitErr.Detail.Hint) diff --git a/internal/client/api_errors.go b/internal/client/api_errors.go index 7014ed6b..65cf18ac 100644 --- a/internal/client/api_errors.go +++ b/internal/client/api_errors.go @@ -23,7 +23,7 @@ func WrapDoAPIError(err error) error { if err == nil { return nil } - if isJSONDecodeError(err) { + if isJSONDecodeError(err, false) { return output.ErrWithHint(output.ExitAPI, "api_error", fmt.Sprintf("API returned an invalid JSON response: %v", err), rawAPIJSONHint) } @@ -40,22 +40,28 @@ func WrapJSONResponseParseError(err error, body []byte) error { return output.ErrWithHint(output.ExitAPI, "api_error", "API returned an empty JSON response body", rawAPIJSONHint) } - if isJSONDecodeError(err) { + if isJSONDecodeError(err, true) { return output.ErrWithHint(output.ExitAPI, "api_error", fmt.Sprintf("API returned an invalid JSON response: %v", err), rawAPIJSONHint) } return output.ErrNetwork("API call failed: %v", err) } -func isJSONDecodeError(err error) bool { +func isJSONDecodeError(err error, allowEOF bool) bool { var syntaxErr *json.SyntaxError var unmarshalTypeErr *json.UnmarshalTypeError - if errors.Is(err, io.EOF) || errors.As(err, &syntaxErr) || errors.As(err, &unmarshalTypeErr) { + if errors.As(err, &syntaxErr) || errors.As(err, &unmarshalTypeErr) { + return true + } + if allowEOF && (errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF)) { return true } msg := err.Error() + if allowEOF && strings.Contains(msg, "unexpected EOF") { + return true + } return strings.Contains(msg, "unexpected end of JSON input") || strings.Contains(msg, "invalid character") || strings.Contains(msg, "cannot unmarshal") diff --git a/internal/client/api_errors_test.go b/internal/client/api_errors_test.go new file mode 100644 index 00000000..a8bb0461 --- /dev/null +++ b/internal/client/api_errors_test.go @@ -0,0 +1,68 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package client + +import ( + "encoding/json" + "errors" + "io" + "strings" + "testing" + + "github.com/larksuite/cli/internal/output" +) + +func TestWrapDoAPIError_BareEOFIsNetworkError(t *testing.T) { + err := WrapDoAPIError(io.EOF) + if err == nil { + t.Fatal("expected error") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected ExitError, got %T", err) + } + if exitErr.Code != output.ExitNetwork { + t.Fatalf("expected ExitNetwork, got %d", exitErr.Code) + } + if strings.Contains(exitErr.Error(), "invalid JSON response") { + t.Fatalf("unexpected JSON diagnostic for bare EOF: %q", exitErr.Error()) + } +} + +func TestWrapDoAPIError_SyntaxErrorIsAPIDiagnostic(t *testing.T) { + err := WrapDoAPIError(&json.SyntaxError{Offset: 1}) + if err == nil { + t.Fatal("expected error") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected ExitError, got %T", err) + } + if exitErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI, got %d", exitErr.Code) + } + if exitErr.Detail == nil || !strings.Contains(exitErr.Detail.Message, "invalid JSON response") { + t.Fatalf("expected JSON diagnostic message, got %#v", exitErr.Detail) + } +} + +func TestWrapJSONResponseParseError_UnexpectedEOFIsAPIDiagnostic(t *testing.T) { + err := WrapJSONResponseParseError(io.ErrUnexpectedEOF, []byte("{")) + if err == nil { + t.Fatal("expected error") + } + + var exitErr *output.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected ExitError, got %T", err) + } + if exitErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI, got %d", exitErr.Code) + } + if exitErr.Detail == nil || !strings.Contains(exitErr.Detail.Message, "invalid JSON response") { + t.Fatalf("expected invalid JSON diagnostic, got %#v", exitErr.Detail) + } +} diff --git a/internal/client/response.go b/internal/client/response.go index a4e92bde..2d1944f1 100644 --- a/internal/client/response.go +++ b/internal/client/response.go @@ -111,7 +111,7 @@ func ParseJSONResponse(resp *larkcore.ApiResp) (interface{}, error) { dec := json.NewDecoder(bytes.NewReader(resp.RawBody)) dec.UseNumber() if err := dec.Decode(&result); err != nil { - return nil, fmt.Errorf("response parse error: %v (body: %s)", err, util.TruncateStr(string(resp.RawBody), 500)) + return nil, fmt.Errorf("response parse error: %w (body: %s)", err, util.TruncateStr(string(resp.RawBody), 500)) } return result, nil } diff --git a/internal/client/response_test.go b/internal/client/response_test.go index 593b45c2..6daba183 100644 --- a/internal/client/response_test.go +++ b/internal/client/response_test.go @@ -6,6 +6,7 @@ package client import ( "bytes" "errors" + "io" "net/http" "os" "path/filepath" @@ -75,6 +76,17 @@ func TestParseJSONResponse_Invalid(t *testing.T) { } } +func TestParseJSONResponse_EmptyBody_WrapsEOF(t *testing.T) { + resp := newApiResp([]byte{}, map[string]string{"Content-Type": "application/json"}) + _, err := ParseJSONResponse(resp) + if err == nil { + t.Fatal("expected error for empty body") + } + if !errors.Is(err, io.EOF) { + t.Fatalf("expected wrapped io.EOF, got %v", err) + } +} + func TestResolveFilename(t *testing.T) { tests := []struct { name string