From de68c9dc2aedfd4511276659981bb31aaed7b992 Mon Sep 17 00:00:00 2001 From: "zhaoyukun.yk" Date: Tue, 2 Jun 2026 19:21:42 +0800 Subject: [PATCH] feat(task): emit typed error envelopes across the task domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task commands now return structured, typed errors instead of the legacy exit-code envelope: every failure carries a stable category, subtype, and recovery hint, so callers can branch on the error class instead of parsing messages. Exit codes derive from the error category — input validation exits 2, a permission denial exits 3, other API errors exit 1. Batch operations (adding tasks to a tasklist, creating a tasklist with tasks) now report partial failure honestly: the per-item successes and failures stay on stdout and the command exits non-zero instead of masking failures as a success. --- .golangci.yml | 6 +- shortcuts/task/shortcuts.go | 31 +-- shortcuts/task/task_assign.go | 41 +--- shortcuts/task/task_assign_test.go | 86 +++++++ shortcuts/task/task_body_test.go | 74 +++--- shortcuts/task/task_comment.go | 22 +- shortcuts/task/task_complete.go | 41 +--- shortcuts/task/task_errors.go | 42 ++++ shortcuts/task/task_errors_test.go | 90 +++++++ shortcuts/task/task_followers.go | 41 +--- shortcuts/task/task_followers_test.go | 25 ++ shortcuts/task/task_get_my_tasks.go | 45 ++-- shortcuts/task/task_get_my_tasks_test.go | 50 ++++ shortcuts/task/task_get_related_tasks.go | 29 +-- shortcuts/task/task_query_helpers.go | 16 +- shortcuts/task/task_query_helpers_test.go | 32 ++- shortcuts/task/task_reminder.go | 76 +----- shortcuts/task/task_reminder_test.go | 55 +++++ shortcuts/task/task_reopen.go | 23 +- shortcuts/task/task_search.go | 37 +-- shortcuts/task/task_search_test.go | 131 +++++++++++ shortcuts/task/task_set_ancestor.go | 20 +- shortcuts/task/task_subscribe_event.go | 22 +- shortcuts/task/task_subscribe_event_test.go | 32 +++ shortcuts/task/task_tasklist_search.go | 37 +-- shortcuts/task/task_tasklist_search_test.go | 35 +++ shortcuts/task/task_update.go | 33 +-- shortcuts/task/task_upload_attachment.go | 36 ++- shortcuts/task/task_upload_attachment_test.go | 169 ++++++++++++-- shortcuts/task/task_util.go | 110 ++++----- shortcuts/task/task_util_test.go | 219 ++++++++++++++++-- shortcuts/task/tasklist_add_task.go | 41 ++-- shortcuts/task/tasklist_add_task_test.go | 76 ++++++ shortcuts/task/tasklist_create.go | 56 ++--- shortcuts/task/tasklist_create_test.go | 159 +++++++++++++ shortcuts/task/tasklist_members.go | 103 +------- shortcuts/task/tasklist_members_test.go | 181 +++++++++++++++ 37 files changed, 1560 insertions(+), 762 deletions(-) create mode 100644 shortcuts/task/task_errors.go create mode 100644 shortcuts/task/task_errors_test.go create mode 100644 shortcuts/task/task_reminder_test.go create mode 100644 shortcuts/task/tasklist_create_test.go diff --git a/.golangci.yml b/.golangci.yml index f9a51eae8..272ceb166 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,20 +65,20 @@ linters: - forbidigo # errs-typed-only enforced on paths already migrated to errs.NewXxxError. # Add a path when its migration is complete. - - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go|shortcuts/drive/) + - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go|shortcuts/drive/|shortcuts/task/) text: errs-typed-only linters: - forbidigo # errs-no-bare-wrap enforced on paths fully migrated to typed final # errors. Scoped separately from errs-typed-only because cmd/auth/, # cmd/config/ still have residual fmt.Errorf and must not be caught. - - path-except: (shortcuts/drive/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) + - path-except: (shortcuts/drive/|shortcuts/task/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper is drive-only: the shared helpers it bans are # still used by other domains until their later migration phase. - - path-except: (shortcuts/drive/) + - path-except: (shortcuts/drive/|shortcuts/task/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/shortcuts/task/shortcuts.go b/shortcuts/task/shortcuts.go index caae868f8..c6d694ec6 100644 --- a/shortcuts/task/shortcuts.go +++ b/shortcuts/task/shortcuts.go @@ -13,9 +13,8 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) func inferTaskMemberType(id string) string { @@ -107,7 +106,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{} // Handle generic JSON payload if provided if dataStr := runtime.Str("data"); dataStr != "" { if err := json.Unmarshal([]byte(dataStr), &body); err != nil { - return nil, output.ErrValidation("--data must be a valid JSON object: %v", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be a valid JSON object: %v", err).WithParam("--data") } } @@ -143,7 +142,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{} if dueStr := runtime.Str("due"); dueStr != "" { dueObj, err := parseTaskTime(dueStr) if err != nil { - return nil, output.ErrValidation("failed to parse due time: %v", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "failed to parse due time: %v", err).WithParam("--due") } body["due"] = dueObj } @@ -154,7 +153,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{} summary, _ := body["summary"].(string) if strings.TrimSpace(summary) == "" { - return nil, output.ErrValidation("task summary is required") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "task summary is required").WithParam("--summary") } return body, nil @@ -194,27 +193,11 @@ var CreateTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { body, err := buildTaskCreateBody(runtime) if err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, err.Error(), "create task") - } - - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return output.Errorf(output.ExitAPI, "api_error", "failed to parse response: %v", parseErr) - } + return err } - data, err := HandleTaskApiResult(result, err, "create task") + params := map[string]interface{}{"user_id_type": "open_id"} + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks", params, body) if err != nil { return err } diff --git a/shortcuts/task/task_assign.go b/shortcuts/task/task_assign.go index 02d5c119c..d0134a518 100644 --- a/shortcuts/task/task_assign.go +++ b/shortcuts/task/task_assign.go @@ -5,15 +5,13 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,7 +33,7 @@ var AssignTask = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if runtime.Str("add") == "" && runtime.Str("remove") == "" { - return WrapTaskError(ErrCodeTaskInvalidParams, "must specify either --add or --remove", "validate assign") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "must specify either --add or --remove") } return nil }, @@ -62,28 +60,13 @@ var AssignTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { taskId := url.PathEscape(runtime.Str("task-id")) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} var lastData map[string]interface{} if addStr := runtime.Str("add"); addStr != "" { body := buildMembersBody(addStr, "assignee", runtime.Str("idempotency-key")) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/add_members", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add members") - } - } - - data, err := HandleTaskApiResult(result, err, "add task members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/add_members", params, body) if err != nil { return err } @@ -92,21 +75,7 @@ var AssignTask = common.Shortcut{ if removeStr := runtime.Str("remove"); removeStr != "" { body := buildMembersBody(removeStr, "assignee", "") - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/remove_members", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove members") - } - } - - data, err := HandleTaskApiResult(result, err, "remove task members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/remove_members", params, body) if err != nil { return err } diff --git a/shortcuts/task/task_assign_test.go b/shortcuts/task/task_assign_test.go index 1a56965e3..5ea15cbe1 100644 --- a/shortcuts/task/task_assign_test.go +++ b/shortcuts/task/task_assign_test.go @@ -4,14 +4,100 @@ package task import ( + "errors" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" "github.com/smartystreets/goconvey/convey" ) +// TestAssignTask_RequiresAddOrRemove covers the Validate guard: neither --add +// nor --remove yields a typed validation error (exit 2) before any API call. +func TestAssignTask_RequiresAddOrRemove(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := AssignTask + args := []string{"+assign", "--task-id", "task-1", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } +} + +// TestAssignTask_MalformedResponse covers the Execute parse-response arm: a +// 200 with an unparseable body surfaces a typed internal invalid_response +// error (exit 5). +func TestAssignTask_MalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/task-1/add_members", + Status: 200, + RawBody: []byte("{not-json"), + }) + + s := AssignTask + args := []string{"+assign", "--task-id", "task-1", "--add", "ou_user_1", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } +} + +// TestAssignTask_MalformedResponse_RemoveArm covers the Execute remove-members +// parse arm: with only --remove set, the add arm is skipped and the +// remove_members POST returns a 200 with an unparseable body, which must +// surface a typed internal invalid_response error (exit 5). +func TestAssignTask_MalformedResponse_RemoveArm(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/task-1/remove_members", + Status: 200, + RawBody: []byte("{not-json"), + }) + + s := AssignTask + args := []string{"+assign", "--task-id", "task-1", "--remove", "ou_user_1", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } +} + func TestBuildMembersBody(t *testing.T) { convey.Convey("Build with ids and token", t, func() { body := buildMembersBody("u1, u2 , ", "assignee", "token1") diff --git a/shortcuts/task/task_body_test.go b/shortcuts/task/task_body_test.go index b6cad5002..e8a73fabd 100644 --- a/shortcuts/task/task_body_test.go +++ b/shortcuts/task/task_body_test.go @@ -5,8 +5,10 @@ package task import ( "errors" + "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" @@ -19,33 +21,25 @@ func TestBuildTaskCreateBody_StructuredErrors(t *testing.T) { data string summary string due string - wantCode int - wantType string wantSubstr string }{ { - name: "invalid JSON data returns ErrValidation", + name: "invalid JSON data returns validation error", data: "not-json", summary: "test", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "--data must be a valid JSON object", }, { - name: "missing summary returns ErrValidation", + name: "missing summary returns validation error", data: "", summary: "", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "task summary is required", }, { - name: "invalid due time returns ErrValidation", + name: "invalid due time returns validation error", data: "", summary: "test task", due: "not-a-valid-time", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "failed to parse due time", }, } @@ -68,18 +62,22 @@ func TestBuildTaskCreateBody_StructuredErrors(t *testing.T) { t.Fatal("expected error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) } - if exitErr.Code != tt.wantCode { - t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("ProblemOf(%T) returned !ok", err) } - if exitErr.Detail == nil { - t.Fatal("expected non-nil error detail") + if p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidArgument) } - if exitErr.Detail.Type != tt.wantType { - t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType) + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } + if !strings.Contains(err.Error(), tt.wantSubstr) { + t.Errorf("message = %q, want substring %q", err.Error(), tt.wantSubstr) } }) } @@ -91,35 +89,27 @@ func TestBuildTaskUpdateBody_StructuredErrors(t *testing.T) { data string summary string due string - wantCode int - wantType string wantSubstr string }{ { - name: "invalid JSON data returns ErrValidation", + name: "invalid JSON data returns validation error", data: "not-json", summary: "", due: "", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "--data must be a valid JSON object", }, { - name: "no fields to update returns ErrValidation", + name: "no fields to update returns validation error", data: "", summary: "", due: "", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "no fields to update", }, { - name: "invalid due time returns ErrValidation", + name: "invalid due time returns validation error", data: "", summary: "", due: "not-a-valid-time", - wantCode: output.ExitValidation, - wantType: "validation", wantSubstr: "failed to parse due time", }, } @@ -138,18 +128,22 @@ func TestBuildTaskUpdateBody_StructuredErrors(t *testing.T) { t.Fatal("expected error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("ProblemOf(%T) returned !ok", err) } - if exitErr.Code != tt.wantCode { - t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode) + if p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidArgument) } - if exitErr.Detail == nil { - t.Fatal("expected non-nil error detail") + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) } - if exitErr.Detail.Type != tt.wantType { - t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType) + if !strings.Contains(err.Error(), tt.wantSubstr) { + t.Errorf("message = %q, want substring %q", err.Error(), tt.wantSubstr) } }) } diff --git a/shortcuts/task/task_comment.go b/shortcuts/task/task_comment.go index 04f1612f6..576c959c6 100644 --- a/shortcuts/task/task_comment.go +++ b/shortcuts/task/task_comment.go @@ -5,13 +5,10 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/shortcuts/common" ) @@ -48,24 +45,9 @@ var CommentTask = common.Shortcut{ "resource_type": "task", } - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/comments", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse comment response") - } - } + params := map[string]interface{}{"user_id_type": "open_id"} - data, err := HandleTaskApiResult(result, err, "add task comment") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/comments", params, body) if err != nil { return err } diff --git a/shortcuts/task/task_complete.go b/shortcuts/task/task_complete.go index 8f58c3602..8f6d81951 100644 --- a/shortcuts/task/task_complete.go +++ b/shortcuts/task/task_complete.go @@ -5,15 +5,12 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" "time" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/shortcuts/common" ) @@ -47,28 +44,14 @@ var CompleteTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { taskId := url.PathEscape(runtime.Str("task-id")) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} var data map[string]interface{} // 1. Get current task status - getResp, getErr := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasks/" + taskId, - QueryParams: queryParams, - }) - - var getResult map[string]interface{} - if getErr == nil { - if parseErr := json.Unmarshal(getResp.RawBody, &getResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse get response: %v", parseErr), "parse get response") - } - } - - getData, getErr := HandleTaskApiResult(getResult, getErr, "get task") - if getErr != nil { - return getErr + getData, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasks/"+taskId, params, nil) + if err != nil { + return err } taskData, _ := getData["task"].(map[string]interface{}) @@ -80,21 +63,7 @@ var CompleteTask = common.Shortcut{ } else { // 3. Complete the task body := buildCompleteBody() - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPatch, - ApiPath: "/open-apis/task/v2/tasks/" + taskId, - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse complete response") - } - } - - data, err = HandleTaskApiResult(result, err, "complete task") + data, err = callTaskAPITyped(runtime, http.MethodPatch, "/open-apis/task/v2/tasks/"+taskId, params, body) if err != nil { return err } diff --git a/shortcuts/task/task_errors.go b/shortcuts/task/task_errors.go new file mode 100644 index 000000000..70b5c9103 --- /dev/null +++ b/shortcuts/task/task_errors.go @@ -0,0 +1,42 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package task + +import ( + "errors" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +// wrapTaskNetworkErr returns err unchanged when it is already a typed errs.* +// error (preserving its subtype / code / log_id from the runtime boundary), +// and only wraps a raw, unclassified error as a transport-level network error. +func wrapTaskNetworkErr(err error, format string, args ...any) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) +} + +// taskInputStatError maps a FileIO.Stat/Open error for input file validation +// to a typed validation error: +// - Path validation failures → "unsafe file path: ..." +// - Other errors → readMsg prefix (default "cannot read file") +// +// Pass an optional readMsg to override the non-path-validation message prefix, +// mirroring the shared input-stat helper so call-site context is preserved. +func taskInputStatError(err error, readMsg ...string) error { + if err == nil { + return nil + } + if errors.Is(err, fileio.ErrPathValidation) { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).WithCause(err) + } + msg := "cannot read file" + if len(readMsg) > 0 && readMsg[0] != "" { + msg = readMsg[0] + } + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: %s", msg, err).WithCause(err) +} diff --git a/shortcuts/task/task_errors_test.go b/shortcuts/task/task_errors_test.go new file mode 100644 index 000000000..ceb9be55e --- /dev/null +++ b/shortcuts/task/task_errors_test.go @@ -0,0 +1,90 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package task + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/output" +) + +func TestTaskInputStatError(t *testing.T) { + t.Run("nil error returns nil", func(t *testing.T) { + if err := taskInputStatError(nil); err != nil { + t.Errorf("taskInputStatError(nil) = %v, want nil", err) + } + }) + + t.Run("path validation failure maps to unsafe file path", func(t *testing.T) { + err := taskInputStatError(fmt.Errorf("bad: %w", fileio.ErrPathValidation)) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if output.ExitCodeOf(err) != output.ExitValidation { + t.Errorf("exit = %d, want %d", output.ExitCodeOf(err), output.ExitValidation) + } + if !strings.Contains(err.Error(), "unsafe file path") { + t.Errorf("message = %q, want 'unsafe file path'", err.Error()) + } + }) + + t.Run("generic error uses readMsg prefix", func(t *testing.T) { + err := taskInputStatError(errors.New("permission denied"), "cannot access file") + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError", err) + } + if !strings.Contains(err.Error(), "cannot access file") { + t.Errorf("message = %q, want 'cannot access file' prefix", err.Error()) + } + }) + + t.Run("default prefix when no readMsg", func(t *testing.T) { + err := taskInputStatError(errors.New("boom")) + if !strings.Contains(err.Error(), "cannot read file") { + t.Errorf("message = %q, want default 'cannot read file'", err.Error()) + } + }) +} + +func TestWrapTaskNetworkErr(t *testing.T) { + // wrapTaskNetworkErr is only ever called inside an `if err != nil` guard + // (DoAPIStream failure), mirroring drive's wrapDriveNetworkErr, so it does + // not special-case a nil cause. + t.Run("untyped cause becomes typed network error wrapping the cause", func(t *testing.T) { + cause := errors.New("dial timeout") + err := wrapTaskNetworkErr(cause, "upload failed") + var ne *errs.NetworkError + if !errors.As(err, &ne) { + t.Fatalf("err = %T, want *errs.NetworkError", err) + } + if ne.Subtype != errs.SubtypeNetworkTransport { + t.Errorf("subtype = %q, want %q", ne.Subtype, errs.SubtypeNetworkTransport) + } + if !errors.Is(err, cause) { + t.Error("expected the original cause to be wrapped (errors.Is)") + } + }) + + t.Run("already-typed cause is passed through unchanged", func(t *testing.T) { + typed := errs.NewAPIError(errs.SubtypeNotFound, "missing") + err := wrapTaskNetworkErr(typed, "upload failed") + var ae *errs.APIError + if !errors.As(err, &ae) { + t.Fatalf("err = %T, want the original *errs.APIError passed through", err) + } + if ae.Subtype != errs.SubtypeNotFound { + t.Errorf("subtype = %q, want %q (not re-wrapped as network)", ae.Subtype, errs.SubtypeNotFound) + } + }) +} diff --git a/shortcuts/task/task_followers.go b/shortcuts/task/task_followers.go index 0e99e42ae..3016ad110 100644 --- a/shortcuts/task/task_followers.go +++ b/shortcuts/task/task_followers.go @@ -5,15 +5,13 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,7 +33,7 @@ var FollowersTask = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if runtime.Str("add") == "" && runtime.Str("remove") == "" { - return WrapTaskError(ErrCodeTaskInvalidParams, "must specify either --add or --remove", "validate followers") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "must specify either --add or --remove") } return nil }, @@ -63,28 +61,13 @@ var FollowersTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { taskId := url.PathEscape(runtime.Str("task-id")) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} var lastData map[string]interface{} if addStr := runtime.Str("add"); addStr != "" { body := buildFollowersBody(addStr, runtime.Str("idempotency-key")) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/add_members", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add followers") - } - } - - data, err := HandleTaskApiResult(result, err, "add task followers") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/add_members", params, body) if err != nil { return err } @@ -93,21 +76,7 @@ var FollowersTask = common.Shortcut{ if removeStr := runtime.Str("remove"); removeStr != "" { body := buildFollowersBody(removeStr, "") - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/remove_members", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove followers") - } - } - - data, err := HandleTaskApiResult(result, err, "remove task followers") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/remove_members", params, body) if err != nil { return err } diff --git a/shortcuts/task/task_followers_test.go b/shortcuts/task/task_followers_test.go index 3f0833dc9..4eeacf9c4 100644 --- a/shortcuts/task/task_followers_test.go +++ b/shortcuts/task/task_followers_test.go @@ -4,11 +4,36 @@ package task import ( + "errors" "testing" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/output" "github.com/smartystreets/goconvey/convey" ) +// TestFollowersTask_RequiresAddOrRemove covers the Validate guard: neither +// --add nor --remove yields a typed validation error (exit 2) before any API +// call. +func TestFollowersTask_RequiresAddOrRemove(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := FollowersTask + args := []string{"+followers", "--task-id", "task-1", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } +} + func TestBuildFollowersBody(t *testing.T) { convey.Convey("Build with ids and token", t, func() { body := buildFollowersBody("u1, u2 , ", "token1") diff --git a/shortcuts/task/task_get_my_tasks.go b/shortcuts/task/task_get_my_tasks.go index 453b8f98e..8badce078 100644 --- a/shortcuts/task/task_get_my_tasks.go +++ b/shortcuts/task/task_get_my_tasks.go @@ -5,15 +5,14 @@ package task import ( "context" - "encoding/json" "fmt" "io" + "net/http" "strconv" "strings" "time" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -59,19 +58,20 @@ var GetMyTasks = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { startTime := time.Now() - queryParams := make(larkcore.QueryParams) - queryParams.Set("type", "my_tasks") - queryParams.Set("user_id_type", "open_id") - queryParams.Set("page_size", "50") + params := map[string]interface{}{ + "type": "my_tasks", + "user_id_type": "open_id", + "page_size": 50, + } if runtime.Cmd.Flags().Changed("complete") { if runtime.Bool("complete") { - queryParams.Set("completed", "true") + params["completed"] = "true" } else { - queryParams.Set("completed", "false") + params["completed"] = "false" } } if pageToken := runtime.Str("page-token"); pageToken != "" { - queryParams.Set("page_token", pageToken) + params["page_token"] = pageToken } // parse time flags to ms timestamp if provided @@ -79,7 +79,7 @@ var GetMyTasks = common.Shortcut{ if createdStr := runtime.Str("created_at"); createdStr != "" { tStr, err := parseTimeFlagSec(createdStr, "start") if err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("invalid created_at: %v", err), "parse created_at") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid created_at: %v", err).WithParam("--created_at") } createdAfterMs, _ = strconv.ParseInt(tStr, 10, 64) createdAfterMs *= 1000 // Convert sec to ms @@ -88,7 +88,7 @@ var GetMyTasks = common.Shortcut{ if dueStartStr := runtime.Str("due-start"); dueStartStr != "" { tStr, err := parseTimeFlagSec(dueStartStr, "start") if err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("invalid due-start: %v", err), "parse due-start") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid due-start: %v", err).WithParam("--due-start") } dueStartMs, _ = strconv.ParseInt(tStr, 10, 64) dueStartMs *= 1000 @@ -97,7 +97,7 @@ var GetMyTasks = common.Shortcut{ if dueEndStr := runtime.Str("due-end"); dueEndStr != "" { tStr, err := parseTimeFlagSec(dueEndStr, "end") if err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("invalid due-end: %v", err), "parse due-end") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid due-end: %v", err).WithParam("--due-end") } dueEndMs, _ = strconv.ParseInt(tStr, 10, 64) dueEndMs *= 1000 @@ -114,22 +114,7 @@ var GetMyTasks = common.Shortcut{ for { pageCount++ - apiReq := &larkcore.ApiReq{ - HttpMethod: "GET", - ApiPath: "/open-apis/task/v2/tasks", - QueryParams: queryParams, - } - - apiResp, err := runtime.DoAPI(apiReq) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse my tasks") - } - } - - data, err := HandleTaskApiResult(result, err, "list tasks") + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasks", params, nil) if err != nil { return err } @@ -150,7 +135,7 @@ var GetMyTasks = common.Shortcut{ } // Set page_token for next iteration - queryParams.Set("page_token", lastPageToken) + params["page_token"] = lastPageToken } var filteredItems []map[string]interface{} diff --git a/shortcuts/task/task_get_my_tasks_test.go b/shortcuts/task/task_get_my_tasks_test.go index d49800912..81fc8841e 100644 --- a/shortcuts/task/task_get_my_tasks_test.go +++ b/shortcuts/task/task_get_my_tasks_test.go @@ -4,12 +4,15 @@ package task import ( + "errors" "strconv" "strings" "testing" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" ) func TestGetMyTasks_LocalTimeFormatting(t *testing.T) { @@ -106,3 +109,50 @@ func TestGetMyTasks_LocalTimeFormatting(t *testing.T) { }) } } + +// TestGetMyTasks_InvalidTimeFlags locks the three time-flag validation arms in +// Execute (--created_at / --due-start / --due-end). The parse runs before any +// API call, so a malformed value deterministically surfaces a typed +// *errs.ValidationError (exit 2) regardless of credentials — the command runs +// as user with a throwaway token. Each error carries the corresponding --flag +// param so the caller can point at the offending input. +func TestGetMyTasks_InvalidTimeFlags(t *testing.T) { + tests := []struct { + name string + flag string + wantParam string + }{ + {name: "created_at", flag: "--created_at", wantParam: "--created_at"}, + {name: "due-start", flag: "--due-start", wantParam: "--due-start"}, + {name: "due-end", flag: "--due-end", wantParam: "--due-end"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := GetMyTasks + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+get-my-tasks", tt.flag, "not-a-time", "--as", "user"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + if err == nil { + t.Fatalf("expected validation error for %s, got nil", tt.flag) + } + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } + if ve.Param != tt.wantParam { + t.Errorf("param = %q, want %q", ve.Param, tt.wantParam) + } + }) + } +} diff --git a/shortcuts/task/task_get_related_tasks.go b/shortcuts/task/task_get_related_tasks.go index 8a9bf2957..d1725e682 100644 --- a/shortcuts/task/task_get_related_tasks.go +++ b/shortcuts/task/task_get_related_tasks.go @@ -5,14 +5,11 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -55,14 +52,15 @@ var GetRelatedTasks = common.Shortcut{ Params(params) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - queryParams.Set("page_size", fmt.Sprintf("%d", relatedTasksPageSize)) + params := map[string]interface{}{ + "user_id_type": "open_id", + "page_size": relatedTasksPageSize, + } if runtime.Cmd.Flags().Changed("include-complete") && !runtime.Bool("include-complete") { - queryParams.Set("completed", "false") + params["completed"] = "false" } if pageToken := runtime.Str("page-token"); pageToken != "" { - queryParams.Set("page_token", pageToken) + params["page_token"] = pageToken } pageLimit := runtime.Int("page-limit") @@ -80,18 +78,7 @@ var GetRelatedTasks = common.Shortcut{ var lastPageToken string var lastHasMore bool for page := 0; page < pageLimit; page++ { - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/task_v2/list_related_task", - QueryParams: queryParams, - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse related tasks") - } - } - data, err := HandleTaskApiResult(result, err, "list related tasks") + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/task_v2/list_related_task", params, nil) if err != nil { return err } @@ -103,7 +90,7 @@ var GetRelatedTasks = common.Shortcut{ if !lastHasMore || lastPageToken == "" { break } - queryParams.Set("page_token", lastPageToken) + params["page_token"] = lastPageToken } userOpenID := runtime.UserOpenId() diff --git a/shortcuts/task/task_query_helpers.go b/shortcuts/task/task_query_helpers.go index 32bc66ee1..affdd1d89 100644 --- a/shortcuts/task/task_query_helpers.go +++ b/shortcuts/task/task_query_helpers.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" ) func splitAndTrimCSV(input string) []string { @@ -46,7 +46,7 @@ func parseTimeRangeMillis(input string) (string, string, error) { } startSecInt, err = strconv.ParseInt(startSec, 10, 64) if err != nil { - return "", "", output.ErrValidation("invalid start timestamp: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid start timestamp: %v", err) } hasStart = true startMillis = startSec + "000" @@ -58,13 +58,13 @@ func parseTimeRangeMillis(input string) (string, string, error) { } endSecInt, err = strconv.ParseInt(endSec, 10, 64) if err != nil { - return "", "", output.ErrValidation("invalid end timestamp: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid end timestamp: %v", err) } hasEnd = true endMillis = endSec + "000" } if hasStart && hasEnd && startSecInt > endSecInt { - return "", "", output.ErrValidation("start time must be earlier than or equal to end time") + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "start time must be earlier than or equal to end time") } return startMillis, endMillis, nil } @@ -91,7 +91,7 @@ func parseTimeRangeRFC3339(input string) (string, string, error) { } startSecInt, err = strconv.ParseInt(startSec, 10, 64) if err != nil { - return "", "", output.ErrValidation("invalid start timestamp: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid start timestamp: %v", err) } hasStart = true startTime = time.Unix(startSecInt, 0).Local().Format(time.RFC3339) @@ -103,13 +103,13 @@ func parseTimeRangeRFC3339(input string) (string, string, error) { } endSecInt, err = strconv.ParseInt(endSec, 10, 64) if err != nil { - return "", "", output.ErrValidation("invalid end timestamp: %v", err) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid end timestamp: %v", err) } hasEnd = true endTime = time.Unix(endSecInt, 0).Local().Format(time.RFC3339) } if hasStart && hasEnd && startSecInt > endSecInt { - return "", "", output.ErrValidation("start time must be earlier than or equal to end time") + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "start time must be earlier than or equal to end time") } return startTime, endTime, nil } @@ -220,7 +220,7 @@ func requireSearchFilter(query string, filter map[string]interface{}, action str if len(filter) > 0 { return nil } - return WrapTaskError(ErrCodeTaskInvalidParams, "query is empty and no filter is provided", action) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s: query is empty and no filter is provided", action) } func renderRelatedTasksPretty(items []map[string]interface{}, hasMore bool, pageToken string) string { diff --git a/shortcuts/task/task_query_helpers_test.go b/shortcuts/task/task_query_helpers_test.go index eba934056..50a199860 100644 --- a/shortcuts/task/task_query_helpers_test.go +++ b/shortcuts/task/task_query_helpers_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" ) @@ -88,6 +89,7 @@ func TestParseTimeRangeMillisAndRequireSearchFilter(t *testing.T) { }{ {name: "empty input", input: "", wantStart: "", wantEnd: ""}, {name: "invalid input", input: "bad-time", wantErr: true}, + {name: "invalid end input", input: "-1d,bad-time", wantErr: true}, {name: "range input", input: "-1d,+1d", wantStart: "non-empty", wantEnd: "non-empty"}, {name: "reversed range fails fast", input: "+1d,-1d", wantErr: true}, } @@ -99,15 +101,16 @@ func TestParseTimeRangeMillisAndRequireSearchFilter(t *testing.T) { t.Fatalf("parseTimeRangeMillis(%q) expected error, got nil", tt.input) } if tt.name == "reversed range fails fast" { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) } - if exitErr.Code != output.ExitValidation { - t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + p, ok := errs.ProblemOf(err) + if !ok || p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidArgument) } - if exitErr.Detail == nil || exitErr.Detail.Type != "validation" { - t.Errorf("error detail type = %q, want %q", exitErr.Detail.Type, "validation") + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) } } return @@ -264,6 +267,7 @@ func TestRenderRelatedTasksPretty(t *testing.T) { }{ {name: "empty input", input: "", wantStart: "", wantEnd: ""}, {name: "invalid input", input: "bad-time", wantErr: true}, + {name: "invalid end input", input: "-1d,bad-time", wantErr: true}, {name: "range input", input: "-1d,+1d", wantStart: "rfc3339", wantEnd: "rfc3339"}, {name: "reversed range fails fast", input: "+1d,-1d", wantErr: true}, } @@ -276,12 +280,16 @@ func TestRenderRelatedTasksPretty(t *testing.T) { t.Fatal("expected error, got nil") } if tt.name == "reversed range fails fast" { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) } - if exitErr.Code != output.ExitValidation { - t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + p, ok := errs.ProblemOf(err) + if !ok || p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) } } return diff --git a/shortcuts/task/task_reminder.go b/shortcuts/task/task_reminder.go index f7e10c51d..029ab21c6 100644 --- a/shortcuts/task/task_reminder.go +++ b/shortcuts/task/task_reminder.go @@ -5,7 +5,6 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" @@ -13,8 +12,7 @@ import ( "strconv" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,10 +33,10 @@ var ReminderTask = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if runtime.Str("set") == "" && !runtime.Bool("remove") { - return WrapTaskError(ErrCodeTaskInvalidParams, "must specify either --set or --remove", "validate reminder") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "must specify either --set or --remove") } if runtime.Str("set") != "" && runtime.Bool("remove") { - return WrapTaskError(ErrCodeTaskInvalidParams, "cannot specify both --set and --remove", "validate reminder") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot specify both --set and --remove") } return nil }, @@ -66,24 +64,10 @@ var ReminderTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { taskId := url.PathEscape(runtime.Str("task-id")) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} // First, get the task to find existing reminders - getResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasks/" + taskId, - QueryParams: queryParams, - }) - - var getResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(getResp.RawBody, &getResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse task details: %v", parseErr), "parse task details") - } - } - - data, err := HandleTaskApiResult(getResult, err, "get task reminders") + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasks/"+taskId, params, nil) if err != nil { return err } @@ -112,21 +96,7 @@ var ReminderTask = common.Shortcut{ body := map[string]interface{}{ "reminder_ids": reminderIds, } - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/remove_reminders", - QueryParams: queryParams, - Body: body, - }) - - var removeResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &removeResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove response") - } - } - - if _, err := HandleTaskApiResult(removeResult, err, "remove task reminders"); err != nil { + if _, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/remove_reminders", params, body); err != nil { return err } } @@ -155,7 +125,7 @@ var ReminderTask = common.Shortcut{ } if parseErr != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, parseErr.Error(), "set reminder") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", parseErr) } // If any reminders exist, remove them first @@ -173,21 +143,7 @@ var ReminderTask = common.Shortcut{ body := map[string]interface{}{ "reminder_ids": reminderIds, } - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/remove_reminders", - QueryParams: queryParams, - Body: body, - }) - - var removeResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &removeResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove response") - } - } - - if _, err := HandleTaskApiResult(removeResult, err, "remove existing task reminders before setting new one"); err != nil { + if _, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/remove_reminders", params, body); err != nil { return err } } @@ -200,21 +156,7 @@ var ReminderTask = common.Shortcut{ }, }, } - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + taskId + "/add_reminders", - QueryParams: queryParams, - Body: body, - }) - - var addResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &addResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add response") - } - } - - if _, err := HandleTaskApiResult(addResult, err, "add task reminder"); err != nil { + if _, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+taskId+"/add_reminders", params, body); err != nil { return err } } diff --git a/shortcuts/task/task_reminder_test.go b/shortcuts/task/task_reminder_test.go new file mode 100644 index 000000000..0d8319c06 --- /dev/null +++ b/shortcuts/task/task_reminder_test.go @@ -0,0 +1,55 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package task + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/output" +) + +// TestReminderTask_RequiresSetOrRemove covers the first Validate guard: neither +// --set nor --remove yields a typed validation error (exit 2) before any API +// call. +func TestReminderTask_RequiresSetOrRemove(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := ReminderTask + args := []string{"+reminder", "--task-id", "task-1", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } +} + +// TestReminderTask_CannotSpecifyBoth covers the second Validate guard: passing +// both --set and --remove yields a typed validation error (exit 2). +func TestReminderTask_CannotSpecifyBoth(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := ReminderTask + args := []string{"+reminder", "--task-id", "task-1", "--set", "15m", "--remove", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } +} diff --git a/shortcuts/task/task_reopen.go b/shortcuts/task/task_reopen.go index 5ea685016..0790ee0d8 100644 --- a/shortcuts/task/task_reopen.go +++ b/shortcuts/task/task_reopen.go @@ -5,14 +5,11 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/shortcuts/common" ) @@ -42,24 +39,8 @@ var ReopenTask = common.Shortcut{ taskId := url.PathEscape(runtime.Str("task-id")) body := buildReopenBody() - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPatch, - ApiPath: "/open-apis/task/v2/tasks/" + taskId, - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse reopen response") - } - } - - data, err := HandleTaskApiResult(result, err, "reopen task") + params := map[string]interface{}{"user_id_type": "open_id"} + data, err := callTaskAPITyped(runtime, http.MethodPatch, "/open-apis/task/v2/tasks/"+taskId, params, body) if err != nil { return err } diff --git a/shortcuts/task/task_search.go b/shortcuts/task/task_search.go index 0d30b02cf..f614b6b49 100644 --- a/shortcuts/task/task_search.go +++ b/shortcuts/task/task_search.go @@ -5,14 +5,12 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -77,18 +75,7 @@ var SearchTask = common.Shortcut{ var lastHasMore bool currentBody := body for page := 0; page < pageLimit; page++ { - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/search", - Body: currentBody, - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse task search") - } - } - data, err := HandleTaskApiResult(result, err, "search tasks") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/search", nil, currentBody) if err != nil { return err } @@ -173,7 +160,7 @@ func buildTaskSearchBody(runtime *common.RuntimeContext) (map[string]interface{} if dueRange := runtime.Str("due"); dueRange != "" { start, end, err := parseTimeRangeRFC3339(dueRange) if err != nil { - return nil, WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("invalid due: %v", err), "build task search") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid due: %v", err).WithParam("--due") } if dueFilter := buildTimeRangeFilter("due_time", start, end); dueFilter != nil { mergeIntoFilter(filter, dueFilter) @@ -196,27 +183,15 @@ func buildTaskSearchBody(runtime *common.RuntimeContext) (map[string]interface{} } func getTaskDetail(runtime *common.RuntimeContext, taskID string) (map[string]interface{}, error) { - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasks/" + url.PathEscape(taskID), - QueryParams: queryParams, - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return nil, WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse task detail response: %v", parseErr), "parse task detail") - } - } - data, err := HandleTaskApiResult(result, err, "get task detail "+taskID) + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasks/"+url.PathEscape(taskID), params, nil) if err != nil { return nil, err } task, _ := data["task"].(map[string]interface{}) if task == nil { - return nil, WrapTaskError(ErrCodeTaskInternalError, "task detail response missing task object", "get task detail") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "task detail response missing task object") } return task, nil } diff --git a/shortcuts/task/task_search_test.go b/shortcuts/task/task_search_test.go index d2bb1384a..96a130eba 100644 --- a/shortcuts/task/task_search_test.go +++ b/shortcuts/task/task_search_test.go @@ -4,12 +4,17 @@ package task import ( + "context" + "errors" "strings" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -298,3 +303,129 @@ func TestSearchTask_Execute(t *testing.T) { }) } } + +// TestSearchTask_InvalidDue_Validation drives the --due validation arm through +// the mounted command. buildTaskSearchBody runs before any API call, so a +// malformed range deterministically surfaces a typed *errs.ValidationError +// (invalid_argument, exit 2) carrying the --due param. +func TestSearchTask_InvalidDue_Validation(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + s := SearchTask + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+search", "--query", "release", "--due", "not-a-time", "--as", "user"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + if err == nil { + t.Fatal("expected validation error for malformed --due, got nil") + } + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } + if ve.Param != "--due" { + t.Errorf("param = %q, want %q", ve.Param, "--due") + } +} + +// TestSearchTask_MalformedSearchResponse covers the search raw-body parse arm: +// the SDK returns a 200 with a non-JSON body and nil error, so the shortcut's +// own json.Unmarshal fails and must surface a typed *errs.InternalError +// (invalid_response, exit 5). +func TestSearchTask_MalformedSearchResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/search", + RawBody: []byte("{not-json"), + }) + + s := SearchTask + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+search", "--query", "release", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + if err == nil { + t.Fatal("expected internal error for malformed response, got nil") + } + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("error type = %T, want *errs.InternalError; error = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } +} + +// TestGetTaskDetail_MalformedResponse exercises getTaskDetail directly. In the +// +search Execute loop a detail-fetch error is intentionally swallowed (the hit +// falls back to its app_link), so the only way to lock the helper's two +// internal arms — a non-JSON body and a code-0 response missing the task object +// — is to call it directly. Both must surface a typed *errs.InternalError +// (invalid_response, exit 5). +func TestGetTaskDetail_MalformedResponse(t *testing.T) { + tests := []struct { + name string + stub *httpmock.Stub + }{ + { + name: "body not json", + stub: &httpmock.Stub{ + Method: "GET", + URL: "/open-apis/task/v2/tasks/task-123", + RawBody: []byte("{not-json"), + }, + }, + { + name: "missing task object", + stub: &httpmock.Stub{ + Method: "GET", + URL: "/open-apis/task/v2/tasks/task-123", + Body: map[string]interface{}{ + "code": 0, + "msg": "success", + "data": map[string]interface{}{}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, _, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + reg.Register(tt.stub) + + runtime := common.TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "test"}, taskTestConfig(t), f, core.AsBot) + + _, err := getTaskDetail(runtime, "task-123") + if err == nil { + t.Fatal("expected internal error, got nil") + } + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("error type = %T, want *errs.InternalError; error = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } + }) + } +} diff --git a/shortcuts/task/task_set_ancestor.go b/shortcuts/task/task_set_ancestor.go index ecc27813c..0eb666efc 100644 --- a/shortcuts/task/task_set_ancestor.go +++ b/shortcuts/task/task_set_ancestor.go @@ -5,14 +5,11 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/shortcuts/common" ) @@ -37,22 +34,9 @@ var SetAncestorTask = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { taskID := runtime.Str("task-id") - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + url.PathEscape(taskID) + "/set_ancestor_task", - QueryParams: queryParams, - Body: buildSetAncestorBody(runtime.Str("ancestor-id")), - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "set ancestor task") - } - } - if _, err = HandleTaskApiResult(result, err, "set ancestor task"); err != nil { + if _, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+url.PathEscape(taskID)+"/set_ancestor_task", params, buildSetAncestorBody(runtime.Str("ancestor-id"))); err != nil { return err } diff --git a/shortcuts/task/task_subscribe_event.go b/shortcuts/task/task_subscribe_event.go index f8afe6c26..44e0bd873 100644 --- a/shortcuts/task/task_subscribe_event.go +++ b/shortcuts/task/task_subscribe_event.go @@ -5,13 +5,10 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/shortcuts/common" ) @@ -29,23 +26,8 @@ var SubscribeTaskEvent = common.Shortcut{ Params(map[string]interface{}{"user_id_type": "open_id"}) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/task_v2/task_subscription", - QueryParams: queryParams, - }) - - // DoAPI may return HTTP 200 while the JSON body contains a non-zero business "code". - // Parse and validate the envelope to avoid false-success output. - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "subscribe task events") - } - } - if _, err := HandleTaskApiResult(result, err, "subscribe task events"); err != nil { + params := map[string]interface{}{"user_id_type": "open_id"} + if _, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/task_v2/task_subscription", params, nil); err != nil { return err } diff --git a/shortcuts/task/task_subscribe_event_test.go b/shortcuts/task/task_subscribe_event_test.go index 2cd4323ec..42b3b7a4b 100644 --- a/shortcuts/task/task_subscribe_event_test.go +++ b/shortcuts/task/task_subscribe_event_test.go @@ -4,12 +4,15 @@ package task import ( + "errors" "strings" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -129,3 +132,32 @@ func TestSubscribeTaskEvent(t *testing.T) { }) } } + +// TestSubscribeTaskEvent_MalformedResponse covers the parse-response arm: a 200 +// with an unparseable body surfaces a typed internal invalid_response error +// (exit 5). +func TestSubscribeTaskEvent_MalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/task_v2/task_subscription", + Status: 200, + RawBody: []byte("{not-json"), + }) + + args := []string{"+subscribe-event", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, SubscribeTaskEvent, args, f, stdout) + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } +} diff --git a/shortcuts/task/task_tasklist_search.go b/shortcuts/task/task_tasklist_search.go index 17d126aba..cc8628b5a 100644 --- a/shortcuts/task/task_tasklist_search.go +++ b/shortcuts/task/task_tasklist_search.go @@ -5,14 +5,12 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -74,18 +72,7 @@ var SearchTasklist = common.Shortcut{ var lastHasMore bool currentBody := body for page := 0; page < pageLimit; page++ { - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists/search", - Body: currentBody, - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse tasklist search") - } - } - data, err := HandleTaskApiResult(result, err, "search tasklists") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists/search", nil, currentBody) if err != nil { return err } @@ -160,7 +147,7 @@ func buildTasklistSearchBody(runtime *common.RuntimeContext) (map[string]interfa if createTime := runtime.Str("create-time"); createTime != "" { start, end, err := parseTimeRangeRFC3339(createTime) if err != nil { - return nil, WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("invalid create-time: %v", err), "build tasklist search") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid create-time: %v", err).WithParam("--create-time") } if timeFilter := buildTimeRangeFilter("create_time", start, end); timeFilter != nil { mergeIntoFilter(filter, timeFilter) @@ -183,27 +170,15 @@ func buildTasklistSearchBody(runtime *common.RuntimeContext) (map[string]interfa } func getTasklistDetail(runtime *common.RuntimeContext, tasklistID string) (map[string]interface{}, error) { - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasklists/" + url.PathEscape(tasklistID), - QueryParams: queryParams, - }) - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return nil, WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse tasklist detail response: %v", parseErr), "parse tasklist detail") - } - } - data, err := HandleTaskApiResult(result, err, "get tasklist detail "+tasklistID) + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasklists/"+url.PathEscape(tasklistID), params, nil) if err != nil { return nil, err } tasklist, _ := data["tasklist"].(map[string]interface{}) if tasklist == nil { - return nil, WrapTaskError(ErrCodeTaskInternalError, "tasklist detail response missing tasklist object", "get tasklist detail") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "tasklist detail response missing tasklist object") } return tasklist, nil } diff --git a/shortcuts/task/task_tasklist_search_test.go b/shortcuts/task/task_tasklist_search_test.go index 288793f68..e285a99b7 100644 --- a/shortcuts/task/task_tasklist_search_test.go +++ b/shortcuts/task/task_tasklist_search_test.go @@ -4,12 +4,15 @@ package task import ( + "errors" "strings" "testing" "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -261,3 +264,35 @@ func TestSearchTasklist_Execute(t *testing.T) { }) } } + +// TestSearchTasklist_MalformedResponse covers the search parse arm: a 200 with +// an unparseable search body surfaces a typed internal invalid_response error +// (exit 5). The detail parse arm is swallowed into the fallback path, so only +// the top-level search parse propagates. +func TestSearchTasklist_MalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists/search", + Status: 200, + RawBody: []byte("{not-json"), + }) + + s := SearchTasklist + s.AuthTypes = []string{"bot", "user"} + args := []string{"+tasklist-search", "--query", "Q2", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) + } +} diff --git a/shortcuts/task/task_update.go b/shortcuts/task/task_update.go index 4ec1b0575..e12c33b10 100644 --- a/shortcuts/task/task_update.go +++ b/shortcuts/task/task_update.go @@ -12,8 +12,7 @@ import ( "net/url" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -51,7 +50,9 @@ var UpdateTask = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { body, err := buildTaskUpdateBody(runtime) if err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, err.Error(), "update task") + // buildTaskUpdateBody already returns a typed validation error; + // propagate it directly instead of re-wrapping as an API error. + return err } taskIds := strings.Split(runtime.Str("task-id"), ",") @@ -63,24 +64,8 @@ var UpdateTask = common.Shortcut{ continue } - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPatch, - ApiPath: "/open-apis/task/v2/tasks/" + url.PathEscape(taskId), - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return output.Errorf(output.ExitAPI, "api_error", "failed to parse response for task %s: %v", taskId, parseErr) - } - } - - data, err := HandleTaskApiResult(result, err, "update task "+taskId) + params := map[string]interface{}{"user_id_type": "open_id"} + data, err := callTaskAPITyped(runtime, http.MethodPatch, "/open-apis/task/v2/tasks/"+url.PathEscape(taskId), params, body) if err != nil { return err } @@ -133,7 +118,7 @@ func buildTaskUpdateBody(runtime *common.RuntimeContext) (map[string]interface{} if dataStr := runtime.Str("data"); dataStr != "" { if err := json.Unmarshal([]byte(dataStr), &taskObj); err != nil { - return nil, output.ErrValidation("--data must be a valid JSON object: %v", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be a valid JSON object: %v", err).WithParam("--data") } // If data is provided, assume keys are update fields for k := range taskObj { @@ -158,7 +143,7 @@ func buildTaskUpdateBody(runtime *common.RuntimeContext) (map[string]interface{} if dueStr := runtime.Str("due"); dueStr != "" { dueObj, err := parseTaskTime(dueStr) if err != nil { - return nil, output.ErrValidation("failed to parse due time: %v", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "failed to parse due time: %v", err).WithParam("--due") } taskObj["due"] = dueObj if !contains(updateFields, "due") { @@ -167,7 +152,7 @@ func buildTaskUpdateBody(runtime *common.RuntimeContext) (map[string]interface{} } if len(updateFields) == 0 { - return nil, output.ErrValidation("no fields to update") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "no fields to update") } return map[string]interface{}{ diff --git a/shortcuts/task/task_upload_attachment.go b/shortcuts/task/task_upload_attachment.go index d752e85c5..f9a187471 100644 --- a/shortcuts/task/task_upload_attachment.go +++ b/shortcuts/task/task_upload_attachment.go @@ -14,8 +14,8 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/client" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -91,20 +91,22 @@ var UploadAttachmentTask = common.Shortcut{ fio := runtime.FileIO() if fio == nil { - return output.ErrValidation("file operations require a FileIO provider") + // A nil FileIO is a runtime wiring fault, not user input. + return errs.NewInternalError(errs.SubtypeUnknown, "file operations require a FileIO provider") } stat, err := fio.Stat(filePath) if err != nil { - return common.WrapInputStatError(err, "file not found") + return taskInputStatError(err, "cannot access file") } if !stat.Mode().IsRegular() { - return output.ErrValidation("file must be a regular file: %s", filePath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", filePath).WithParam("--file") } if stat.Size() > taskAttachmentUploadMaxSize { - return output.ErrValidation( + return errs.NewValidationError( + errs.SubtypeInvalidArgument, "attachment %s exceeds the 50MB per-file limit", common.FormatSize(stat.Size()), - ) + ).WithParam("--file") } fileName := filepath.Base(filePath) @@ -118,7 +120,7 @@ var UploadAttachmentTask = common.Shortcut{ f, err := fio.Open(filePath) if err != nil { - return common.WrapInputStatError(err, "cannot open file") + return taskInputStatError(err, "cannot open file") } defer f.Close() @@ -129,20 +131,20 @@ var UploadAttachmentTask = common.Shortcut{ var bodyBuf bytes.Buffer mw := common.NewMultipartWriter(&bodyBuf) if err := mw.WriteField("resource_type", resourceType); err != nil { - return output.Errorf(output.ExitInternal, "internal", "build multipart body: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "build multipart body: %s", err) } if err := mw.WriteField("resource_id", resourceID); err != nil { - return output.Errorf(output.ExitInternal, "internal", "build multipart body: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "build multipart body: %s", err) } filePart, err := mw.CreateFormFile("file", fileName) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "build multipart body: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "build multipart body: %s", err) } if _, err := io.Copy(filePart, f); err != nil { - return output.Errorf(output.ExitInternal, "internal", "write file to multipart body: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "write file to multipart body: %s", err) } if err := mw.Close(); err != nil { - return output.Errorf(output.ExitInternal, "internal", "finalize multipart body: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "finalize multipart body: %s", err) } queryParams := make(larkcore.QueryParams) @@ -167,7 +169,7 @@ var UploadAttachmentTask = common.Shortcut{ if err != nil { fmt.Fprintf(runtime.IO().ErrOut, "[+upload-attachment] http response: error=%v\n", err) - return err + return wrapTaskNetworkErr(err, "upload attachment request failed") } defer httpResp.Body.Close() @@ -175,18 +177,14 @@ var UploadAttachmentTask = common.Shortcut{ if readErr != nil { fmt.Fprintf(runtime.IO().ErrOut, "[+upload-attachment] http response: read_error=%v\n", readErr) - return WrapTaskError(ErrCodeTaskInternalError, - fmt.Sprintf("failed to read response: %v", readErr), - "upload task attachment") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to read response: %v", readErr) } var result map[string]interface{} if parseErr := json.Unmarshal(rawBody, &result); parseErr != nil { fmt.Fprintf(runtime.IO().ErrOut, "[+upload-attachment] http response: parse_error=%v\n", parseErr) - return WrapTaskError(ErrCodeTaskInternalError, - fmt.Sprintf("failed to parse response: %v", parseErr), - "upload task attachment") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr) } data, err := HandleTaskApiResult(result, nil, "upload task attachment") diff --git a/shortcuts/task/task_upload_attachment_test.go b/shortcuts/task/task_upload_attachment_test.go index a2565fb12..b8c08c840 100644 --- a/shortcuts/task/task_upload_attachment_test.go +++ b/shortcuts/task/task_upload_attachment_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/output" @@ -246,12 +247,15 @@ func TestUploadAttachmentTask_SizeLimit(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected ExitError, got %T: %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) } - if exitErr.Code != output.ExitValidation { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) } if !strings.Contains(err.Error(), "50MB") { t.Fatalf("error message should mention 50MB limit, got: %v", err) @@ -274,12 +278,136 @@ func TestUploadAttachmentTask_FileMissing(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected ExitError, got %T: %v", err, err) + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) + } +} + +func TestUploadAttachmentTask_NotRegularFile(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + + // A directory stats fine but is not a regular file; we must reject it as + // invalid --file input before any HTTP call (no stub registered). + if err := os.Mkdir("a-dir", 0o755); err != nil { + t.Fatalf("Mkdir error: %v", err) + } + + err := runMountedTaskShortcut(t, UploadAttachmentTask, []string{ + "+upload-attachment", + "--resource-id", "task-guid-123", + "--file", "a-dir", + "--as", "bot", + "--format", "json", + }, f, stdout) + if err == nil { + t.Fatal("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--file" { + t.Fatalf("param = %q, want %q", ve.Param, "--file") + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) + } + if !strings.Contains(err.Error(), "regular file") { + t.Fatalf("error message should mention regular file, got: %v", err) + } +} + +func TestUploadAttachmentTask_StatErrorMessage(t *testing.T) { + f, stdout, _, _ := taskShortcutTestFactory(t) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + + // A missing path fails Stat → taskInputStatError surfaces "cannot access + // file" (no longer "file not found"). No HTTP stub: must fail before any call. + err := runMountedTaskShortcut(t, UploadAttachmentTask, []string{ + "+upload-attachment", + "--resource-id", "task-guid-123", + "--file", "missing.bin", + "--as", "bot", + "--format", "json", + }, f, stdout) + if err == nil { + t.Fatal("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want %d", got, output.ExitValidation) + } + if !strings.Contains(err.Error(), "cannot access file") { + t.Fatalf("error message should contain %q, got: %v", "cannot access file", err) + } +} + +func TestUploadAttachmentTask_MalformedResponse(t *testing.T) { + f, stdout, stderr, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + + filePath := writeTestFile(t, "note.txt", 4) + + // A 200 response whose body is not valid JSON: the parse failure must + // surface a typed internal invalid_response error (exit 5), not a panic + // or a silent success. + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/attachments/upload", + RawBody: []byte("this is not json{"), + }) + + err := runMountedTaskShortcut(t, UploadAttachmentTask, []string{ + "+upload-attachment", + "--resource-id", "task-guid-123", + "--file", filePath, + "--as", "bot", + "--format", "json", + }, f, stdout) + if err == nil { + t.Fatal("expected error, got nil") + } + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("expected *errs.InternalError, got %T: %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) } - if exitErr.Code != output.ExitValidation { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitValidation) + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Fatalf("exit code = %d, want %d", got, output.ExitInternal) + } + if !strings.Contains(err.Error(), "parse response") { + t.Fatalf("error message should mention parse response, got: %v", err) + } + + // The parse-error observability log should be emitted on stderr. + if errOut := stderr.String(); !strings.Contains(errOut, "parse_error") { + t.Errorf("stderr missing parse_error log; got:\n%s", errOut) } } @@ -311,12 +439,23 @@ func TestUploadAttachmentTask_APIError(t *testing.T) { if err == nil { t.Fatal("expected error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected ExitError, got %T: %v", err, err) + var pe *errs.PermissionError + if !errors.As(err, &pe) { + t.Fatalf("expected *errs.PermissionError, got %T: %v", err, err) + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("ProblemOf(err) = !ok, want typed errs.* error; err = %v", err) + } + if p.Subtype != errs.SubtypePermissionDenied { + t.Fatalf("subtype = %q, want %q", p.Subtype, errs.SubtypePermissionDenied) + } + if p.Code != ErrCodeTaskPermissionDenied { + t.Fatalf("code = %d, want %d", p.Code, ErrCodeTaskPermissionDenied) } - if exitErr.Detail == nil || exitErr.Detail.Code != ErrCodeTaskPermissionDenied { - t.Fatalf("expected task permission denied code %d, got: %+v", ErrCodeTaskPermissionDenied, exitErr.Detail) + // permission_denied maps to CategoryAuthorization → exit 3 (was exit 1 under legacy). + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Fatalf("exit code = %d, want %d", got, output.ExitAuth) } // Key-path log should still be emitted on failure. diff --git a/shortcuts/task/task_util.go b/shortcuts/task/task_util.go index f4af630d3..726387b00 100644 --- a/shortcuts/task/task_util.go +++ b/shortcuts/task/task_util.go @@ -10,7 +10,8 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/shortcuts/common" ) @@ -24,7 +25,7 @@ func isRelativeTime(s string) bool { func parseRelativeTime(s string) (time.Time, error) { matches := relativeTimeRe.FindStringSubmatch(s) if len(matches) == 0 { - return time.Time{}, output.ErrValidation("invalid relative time format: %s", s) + return time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid relative time format: %s", s) } sign := matches[1] @@ -75,74 +76,44 @@ const ( ErrCodeTaskReminderExists = 1470613 ) -// TaskErrorCode maps Lark error codes to standardized error info. -type TaskErrorInfo struct { - Type string - Message string - Hint string - ExitCode int +// taskAPIHints carries the task-specific recovery hint for each known Lark API +// code, layered onto the typed error after errclass.BuildAPIError classifies +// it. errclass.APIHint only covers context-free subtypes (e.g. conflict); these +// hints carry the resource context APIHint intentionally leaves to the caller. +// Authorization (1470403) is omitted: BuildAPIError already attaches the +// canonical permission hint. +var taskAPIHints = map[int]string{ + ErrCodeTaskInvalidParams: "Please check required fields, field lengths, or parameter logic (e.g., reminders require a due date).", + ErrCodeTaskNotFound: "Please verify if the task, tasklist, or group ID is correct and has not been deleted.", + ErrCodeTaskConflict: "Avoid making concurrent API calls using the same client_token.", + ErrCodeTaskInternalError: "Please try again. If the error persists, check the content validity or contact support.", + ErrCodeTaskAssigneeLimit: "The current task has reached the maximum number of assignees.", + ErrCodeTaskFollowerLimit: "The current task has reached the maximum number of followers.", + ErrCodeTasklistMemberLimit: "The current tasklist has reached the maximum number of members.", + ErrCodeTaskReminderExists: "The task already has a reminder set. Remove the existing reminder before adding a new one.", } -var taskErrorMap = map[int]TaskErrorInfo{ - // Generic Task errors from docs - ErrCodeTaskInvalidParams: {"validation_error", "Invalid request parameters", "Please check required fields, field lengths, or parameter logic (e.g., reminders require a due date).", output.ExitValidation}, - ErrCodeTaskNotFound: {"not_found", "Resource not found", "Please verify if the task, tasklist, or group ID is correct and has not been deleted.", output.ExitAPI}, - ErrCodeTaskPermissionDenied: {"permission_error", "Permission denied", "Please check if the calling identity has the necessary edit or read permissions for the resource (task/tasklist).", output.ExitAPI}, - ErrCodeTaskInternalError: {"api_error", "Internal server error", "Please try again. If the error persists, check the content validity or contact support.", output.ExitAPI}, - ErrCodeTaskConflict: {"conflict", "Concurrent call conflict", "Avoid making concurrent API calls using the same client_token.", output.ExitAPI}, - ErrCodeTaskAssigneeLimit: {"api_error", "Assignee limit exceeded", "The current task has reached the maximum number of assignees.", output.ExitAPI}, - ErrCodeTaskFollowerLimit: {"api_error", "Follower limit exceeded", "The current task has reached the maximum number of followers.", output.ExitAPI}, - ErrCodeTasklistMemberLimit: {"api_error", "Tasklist member limit exceeded", "The current tasklist has reached the maximum number of members.", output.ExitAPI}, - ErrCodeTaskReminderExists: {"api_error", "Reminder already exists", "The task already has a reminder set. Remove the existing reminder before adding a new one.", output.ExitAPI}, +func callTaskAPITyped(runtime *common.RuntimeContext, method, url string, params map[string]interface{}, body interface{}) (map[string]interface{}, error) { + data, err := runtime.CallAPITyped(method, url, params, body) + return data, applyTaskAPIHint(err) } -// WrapTaskError wraps a Lark API error into a standardized ExitError based on task-specific rules. -func WrapTaskError(larkCode int, rawMsg string, action string) error { - info, ok := taskErrorMap[larkCode] - if !ok { - // Fallback to generic classification if not in task-specific map - exitCode, errType, hint := output.ClassifyLarkError(larkCode, rawMsg) - - // Generic message based on type - genericMsg := "" - switch errType { - case "permission": - genericMsg = "Permission denied" - case "auth": - genericMsg = "Authentication failed" - case "config": - genericMsg = "Configuration error" - case "rate_limit": - genericMsg = "Rate limit exceeded" - default: - genericMsg = "API error" - } - - displayMsg := fmt.Sprintf("%s: %s [%d] (Details: %s)", action, genericMsg, larkCode, rawMsg) - - return &output.ExitError{ - Code: exitCode, - Detail: &output.ErrDetail{ - Type: errType, - Code: larkCode, - Message: displayMsg, - Hint: hint, - }, - } +func applyTaskAPIHint(err error) error { + if err == nil { + return nil } - - return &output.ExitError{ - Code: info.ExitCode, - Detail: &output.ErrDetail{ - Type: info.Type, - Code: larkCode, - Message: fmt.Sprintf("%s: %s (Details: %s)", action, info.Message, rawMsg), - Hint: info.Hint, - }, + if p, ok := errs.ProblemOf(err); ok { + if hint := taskAPIHints[p.Code]; hint != "" { + p.Hint = hint + } } + return err } -// HandleTaskApiResult is a wrapper around common.HandleApiResult that applies task-specific error mapping. +// HandleTaskApiResult interprets a parsed Lark API response. A non-zero code is +// classified into a typed errs.* error by errclass.BuildAPIError — Category, +// Subtype, Code, and log_id are sourced from internal/errclass/codemeta_task.go +// — with the task-specific recovery hint (taskAPIHints) layered on top. func HandleTaskApiResult(result interface{}, err error, action string) (map[string]interface{}, error) { if err != nil { return nil, err @@ -151,16 +122,19 @@ func HandleTaskApiResult(result interface{}, err error, action string) (map[stri resultMap, _ := result.(map[string]interface{}) codeVal, hasCode := resultMap["code"] if !hasCode { - // Try to see if it's already an error from common.HandleApiResult (e.g. network error) - data, err := common.HandleApiResult(result, err, action) - return data, err + // A Lark response always carries a top-level code; its absence (with no + // transport error) means a malformed or unexpected body. + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: unexpected response (missing code field)", action) } - code, _ := util.ToFloat64(codeVal) + code, ok := util.ToFloat64(codeVal) + if !ok { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: malformed response (non-numeric code %v)", action, codeVal) + } larkCode := int(code) if larkCode != 0 { - rawMsg, _ := resultMap["msg"].(string) - return nil, WrapTaskError(larkCode, rawMsg, action) + typedErr := errclass.BuildAPIError(resultMap, errclass.ClassifyContext{}) + return nil, applyTaskAPIHint(typedErr) } data, _ := resultMap["data"].(map[string]interface{}) diff --git a/shortcuts/task/task_util_test.go b/shortcuts/task/task_util_test.go index d83e32398..6c8c12030 100644 --- a/shortcuts/task/task_util_test.go +++ b/shortcuts/task/task_util_test.go @@ -4,10 +4,20 @@ package task import ( + "context" "errors" + "net/http" + "strings" "testing" + "github.com/spf13/cobra" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/shortcuts/common" "github.com/smartystreets/goconvey/convey" ) @@ -20,43 +30,202 @@ func TestContains(t *testing.T) { }) } -func TestParseRelativeTime_StructuredErrors(t *testing.T) { +func TestParseRelativeTime_TypedError(t *testing.T) { + _, err := parseRelativeTime("not-relative") + if err == nil { + t.Fatal("parseRelativeTime(\"not-relative\") expected error, got nil") + } + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError; error = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d", got, output.ExitValidation) + } + if !strings.Contains(err.Error(), "invalid relative time format") { + t.Errorf("message = %q, want substring %q", err.Error(), "invalid relative time format") + } +} + +// apiResult builds a parsed Lark API response with a non-zero code, as +// HandleTaskApiResult receives it after json.Unmarshal. +func apiResult(code int, msg string) map[string]interface{} { + return map[string]interface{}{"code": float64(code), "msg": msg} +} + +// TestHandleTaskApiResult_TypedMapping locks the API code → typed +// category/subtype/exit mapping. Classification is sourced from +// internal/errclass/codemeta_task.go via errclass.BuildAPIError; the +// task-specific recovery hint is layered on from taskAPIHints. 1470400 surfaces +// exit 1 (API-side parameter rejection, was exit 2 under legacy); 1470403 +// routes to CategoryAuthorization and surfaces exit 3 (was exit 1). +func TestHandleTaskApiResult_TypedMapping(t *testing.T) { tests := []struct { - name string - input string - wantCode int - wantType string - wantSubstr string + name string + code int + wantSubtype errs.Subtype + wantExit int + wantRetry bool }{ - { - name: "invalid format returns ErrValidation", - input: "not-relative", - wantCode: output.ExitValidation, - wantType: "validation", - wantSubstr: "invalid relative time format", - }, + {"invalid_params", ErrCodeTaskInvalidParams, errs.SubtypeInvalidParameters, output.ExitAPI, false}, + {"not_found", ErrCodeTaskNotFound, errs.SubtypeNotFound, output.ExitAPI, false}, + {"conflict", ErrCodeTaskConflict, errs.SubtypeConflict, output.ExitAPI, true}, + {"internal", ErrCodeTaskInternalError, errs.SubtypeServerError, output.ExitAPI, true}, + {"assignee_limit", ErrCodeTaskAssigneeLimit, errs.SubtypeQuotaExceeded, output.ExitAPI, false}, + {"follower_limit", ErrCodeTaskFollowerLimit, errs.SubtypeQuotaExceeded, output.ExitAPI, false}, + {"member_limit", ErrCodeTasklistMemberLimit, errs.SubtypeQuotaExceeded, output.ExitAPI, false}, + {"reminder_exists", ErrCodeTaskReminderExists, errs.SubtypeAlreadyExists, output.ExitAPI, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := parseRelativeTime(tt.input) - if err == nil { - t.Fatalf("parseRelativeTime(%q) expected error, got nil", tt.input) + data, err := HandleTaskApiResult(apiResult(tt.code, "raw upstream detail"), nil, "do thing") + if data != nil { + t.Errorf("data = %v, want nil on error", data) } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %T, want typed errs.* error", err) + } + if p.Subtype != tt.wantSubtype { + t.Errorf("subtype = %q, want %q", p.Subtype, tt.wantSubtype) + } + if p.Code != tt.code { + t.Errorf("code = %d, want %d", p.Code, tt.code) + } + if got := output.ExitCodeOf(err); got != tt.wantExit { + t.Errorf("exit code = %d, want %d", got, tt.wantExit) + } + if p.Retryable != tt.wantRetry { + t.Errorf("retryable = %v, want %v", p.Retryable, tt.wantRetry) + } + // These CategoryAPI codes carry the task-specific recovery hint. + if p.Hint != taskAPIHints[tt.code] { + t.Errorf("hint = %q, want %q", p.Hint, taskAPIHints[tt.code]) + } + // BuildAPIError uses the raw upstream msg as the message. + if !strings.Contains(err.Error(), "raw upstream detail") { + t.Errorf("message = %q, want raw upstream detail", err.Error()) + } + }) + } +} + +// TestHandleTaskApiResult_PermissionDenied verifies 1470403 routes to a typed +// *errs.PermissionError (exit 3) carrying the canonical permission hint from +// BuildAPIError — taskAPIHints intentionally omits it so the canonical hint +// stands. +func TestHandleTaskApiResult_PermissionDenied(t *testing.T) { + _, err := HandleTaskApiResult(apiResult(ErrCodeTaskPermissionDenied, "no permission"), nil, "do thing") + var pe *errs.PermissionError + if !errors.As(err, &pe) { + t.Fatalf("err = %T, want *errs.PermissionError", err) + } + if pe.Subtype != errs.SubtypePermissionDenied { + t.Errorf("subtype = %q, want %q", pe.Subtype, errs.SubtypePermissionDenied) + } + if got := output.ExitCodeOf(err); got != output.ExitAuth { + t.Errorf("exit code = %d, want %d", got, output.ExitAuth) + } + if strings.TrimSpace(pe.Hint) == "" { + t.Error("expected a canonical permission hint, got empty") + } +} + +func TestCallTaskAPITyped_TaskHint(t *testing.T) { + cfg := taskTestConfig(t) + f, _, _, reg := cmdutil.TestFactory(t, cfg) + rt := common.TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "+x"}, cfg, f, core.AsUser) + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/task/v2/tasks/t-1", + Body: map[string]interface{}{ + "code": float64(ErrCodeTaskReminderExists), + "msg": "reminder exists", + }, + }) + + _, err := callTaskAPITyped(rt, http.MethodGet, "/open-apis/task/v2/tasks/t-1", nil, nil) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %T, want typed errs.* error", err) + } + if p.Hint != taskAPIHints[ErrCodeTaskReminderExists] { + t.Errorf("hint = %q, want %q", p.Hint, taskAPIHints[ErrCodeTaskReminderExists]) + } +} - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err) +// TestHandleTaskApiResult_MalformedResponse covers the two malformed-body arms: +// a response with no top-level code, and one whose code is non-numeric. Both +// must surface a typed internal invalid_response error (exit 5) rather than +// silently passing through as a success. +func TestHandleTaskApiResult_MalformedResponse(t *testing.T) { + cases := []struct { + name string + result map[string]interface{} + }{ + {"missing code field", map[string]interface{}{"msg": "weird", "data": map[string]interface{}{}}}, + {"non-numeric code", map[string]interface{}{"code": "oops", "msg": "weird"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + data, err := HandleTaskApiResult(tc.result, nil, "do thing") + if data != nil { + t.Errorf("data = %v, want nil", data) } - if exitErr.Code != tt.wantCode { - t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode) + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError", err) } - if exitErr.Detail == nil { - t.Fatal("expected non-nil error detail") + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) } - if exitErr.Detail.Type != tt.wantType { - t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType) + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d", got, output.ExitInternal) } }) } } + +// TestHandleTaskApiResult_Success returns the data map unchanged when code == 0. +func TestHandleTaskApiResult_Success(t *testing.T) { + want := map[string]interface{}{"guid": "t-1"} + data, err := HandleTaskApiResult(map[string]interface{}{ + "code": float64(0), + "data": want, + }, nil, "do thing") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if data["guid"] != "t-1" { + t.Errorf("data = %v, want guid=t-1", data) + } +} + +// TestHandleTaskApiResult_UnknownCode covers the fallback arm: an uncatalogued +// code becomes a generic CategoryAPI error with SubtypeUnknown and no layered +// hint. +func TestHandleTaskApiResult_UnknownCode(t *testing.T) { + _, err := HandleTaskApiResult(apiResult(9999999, "weird"), nil, "do thing") + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("err = %T, want typed errs.* error", err) + } + if p.Subtype != errs.SubtypeUnknown { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeUnknown) + } + if p.Code != 9999999 { + t.Errorf("code = %d, want 9999999", p.Code) + } + if got := output.ExitCodeOf(err); got != output.ExitAPI { + t.Errorf("exit code = %d, want %d", got, output.ExitAPI) + } + var ae *errs.APIError + if !errors.As(err, &ae) { + t.Errorf("error type = %T, want *errs.APIError", err) + } +} diff --git a/shortcuts/task/tasklist_add_task.go b/shortcuts/task/tasklist_add_task.go index 2f41ff784..6f868b7ed 100644 --- a/shortcuts/task/tasklist_add_task.go +++ b/shortcuts/task/tasklist_add_task.go @@ -5,16 +5,13 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -55,8 +52,7 @@ var AddTaskToTasklist = common.Shortcut{ tasklistGuid := extractTasklistGuid(runtime.Str("tasklist-id")) taskIds := strings.Split(runtime.Str("task-id"), ",") - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} body := map[string]interface{}{ "tasklist_guid": tasklistGuid, @@ -75,30 +71,16 @@ var AddTaskToTasklist = common.Shortcut{ continue } - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks/" + url.PathEscape(taskId) + "/add_tasklist", - QueryParams: queryParams, - Body: body, - }) - - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - err = WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add task response") - } - } - - data, err := HandleTaskApiResult(result, err, "add task to tasklist") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks/"+url.PathEscape(taskId)+"/add_tasklist", params, body) if err != nil { failDetail := map[string]interface{}{ "guid": taskId, } - if exitErr, ok := err.(*output.ExitError); ok && exitErr.Detail != nil { - failDetail["type"] = exitErr.Detail.Type - failDetail["code"] = exitErr.Detail.Code - failDetail["message"] = exitErr.Detail.Message - failDetail["hint"] = exitErr.Detail.Hint + if p, ok := errs.ProblemOf(err); ok { + failDetail["type"] = string(p.Subtype) + failDetail["code"] = p.Code + failDetail["message"] = p.Message + failDetail["hint"] = p.Hint } else { failDetail["type"] = "api_error" failDetail["message"] = err.Error() @@ -123,6 +105,13 @@ var AddTaskToTasklist = common.Shortcut{ "tasklist_guid": tasklistGuid, } + // Item-level failures surface as a non-zero exit (ok:false) so callers + // don't have to inspect failed_tasks to detect a partial add; the full + // payload (successful + failed) stays on stdout either way. + if len(failed) > 0 { + return runtime.OutPartialFailure(resultData, nil) + } + runtime.OutFormat(resultData, nil, func(w io.Writer) { fmt.Fprintf(w, "✅ Tasks added to tasklist %s!\n", tasklistGuid) fmt.Fprintf(w, "Successful: %d, Failed: %d\n", len(successful), len(failed)) diff --git a/shortcuts/task/tasklist_add_task_test.go b/shortcuts/task/tasklist_add_task_test.go index af9edfd7c..c300e438d 100644 --- a/shortcuts/task/tasklist_add_task_test.go +++ b/shortcuts/task/tasklist_add_task_test.go @@ -4,10 +4,13 @@ package task import ( + "errors" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" ) func TestAddTaskToTasklist_Success(t *testing.T) { @@ -41,3 +44,76 @@ func TestAddTaskToTasklist_Success(t *testing.T) { t.Errorf("expected tasklist_guid in output, got: %s", out) } } + +// TestAddTaskToTasklist_PartialFailure exercises the batch path: some tasks +// succeed, others fail with typed API errors. Successful and failed tasks both +// land in stdout as an ok:false envelope, and the command returns the typed +// partial-failure exit signal (exit 1) via runtime.OutPartialFailure. The +// failed_tasks[].type carries the typed subtype (e.g. "permission_denied", +// "not_found") read off errs.ProblemOf, not the legacy +// *output.ExitError.Detail.Type ("permission_error" etc). +func TestAddTaskToTasklist_PartialFailure(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/task-ok/add_tasklist", + Body: map[string]interface{}{ + "code": 0, "msg": "success", + "data": map[string]interface{}{ + "task": map[string]interface{}{ + "guid": "task-ok", + }, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/task-perm/add_tasklist", + Body: map[string]interface{}{ + "code": ErrCodeTaskPermissionDenied, "msg": "no permission", + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks/task-missing/add_tasklist", + Body: map[string]interface{}{ + "code": ErrCodeTaskNotFound, "msg": "task not found", + }, + }) + + s := AddTaskToTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-task-add", "--tasklist-id", "tl-123", "--task-id", "task-ok,task-perm,task-missing", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + // Partial failure now surfaces as a non-zero exit (ok:false), not nil. + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError on partial failure, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } + + out := stdout.String() + + // Successful task is in stdout. + if !strings.Contains(out, "task-ok") { + t.Errorf("expected successful task-ok in output, got: %s", out) + } + + // Failed tasks carry the typed subtype, not the legacy Detail.Type. + if !strings.Contains(out, string(errs.SubtypePermissionDenied)) { + t.Errorf("expected typed subtype %q in failed_tasks, got: %s", errs.SubtypePermissionDenied, out) + } + if !strings.Contains(out, string(errs.SubtypeNotFound)) { + t.Errorf("expected typed subtype %q in failed_tasks, got: %s", errs.SubtypeNotFound, out) + } + + // The legacy shapes must not leak. + if strings.Contains(out, "permission_error") { + t.Errorf("legacy type \"permission_error\" leaked into output: %s", out) + } +} diff --git a/shortcuts/task/tasklist_create.go b/shortcuts/task/tasklist_create.go index 849000337..89f137c00 100644 --- a/shortcuts/task/tasklist_create.go +++ b/shortcuts/task/tasklist_create.go @@ -12,8 +12,7 @@ import ( "strings" "sync" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -50,24 +49,18 @@ var CreateTasklist = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { body := buildTasklistCreateBody(runtime) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") - - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists", - QueryParams: queryParams, - Body: body, - }) + params := map[string]interface{}{"user_id_type": "open_id"} - var result map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse create tasklist") + // Validate --data (client input) before any remote write, so a malformed + // payload fails fast without creating an orphan tasklist. + var tasks []map[string]interface{} + if dataStr := runtime.Str("data"); dataStr != "" { + if err := json.Unmarshal([]byte(dataStr), &tasks); err != nil { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "failed to parse --data as JSON array: %v", err).WithParam("--data") } } - data, err := HandleTaskApiResult(result, err, "create tasklist") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists", params, body) if err != nil { return err } @@ -78,16 +71,10 @@ var CreateTasklist = common.Shortcut{ tasklistUrl, _ := tasklist["url"].(string) tasklistUrl = truncateTaskURL(tasklistUrl) - // Create tasks if data is provided - var tasks []map[string]interface{} var createdTasks []map[string]interface{} var failedTasks []string - if dataStr := runtime.Str("data"); dataStr != "" { - if err := json.Unmarshal([]byte(dataStr), &tasks); err != nil { - return WrapTaskError(ErrCodeTaskInvalidParams, fmt.Sprintf("failed to parse --data as JSON array: %v", err), "parse data") - } - + if len(tasks) > 0 { var wg sync.WaitGroup var mu sync.Mutex @@ -120,24 +107,11 @@ var CreateTasklist = common.Shortcut{ delete(tDef, "assignee") } - tResp, tErr := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasks", - QueryParams: queryParams, - Body: tDef, - }) + tData, tErr := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasks", params, tDef) mu.Lock() defer mu.Unlock() - var tResult map[string]interface{} - if tErr == nil { - if json.Unmarshal(tResp.RawBody, &tResult) != nil { - tErr = WrapTaskError(ErrCodeTaskInternalError, "failed to parse task response", "parse task") - } - } - - tData, tErr := HandleTaskApiResult(tResult, tErr, "create task in tasklist") if tErr != nil { summary, _ := tDef["summary"].(string) failedTasks = append(failedTasks, fmt.Sprintf("Index %d (%s): %v", idx, summary, tErr)) @@ -163,6 +137,14 @@ var CreateTasklist = common.Shortcut{ "guid": tasklistGuid, "url": tasklistUrl, "created_tasks": createdTasks, + "failed_tasks": failedTasks, + } + + // Sub-task creation failures surface as a non-zero exit (ok:false) so the + // partial result is detectable without inspecting failed_tasks; the + // tasklist itself was created and stays in the payload. + if len(failedTasks) > 0 { + return runtime.OutPartialFailure(outData, nil) } runtime.OutFormat(outData, nil, func(w io.Writer) { diff --git a/shortcuts/task/tasklist_create_test.go b/shortcuts/task/tasklist_create_test.go new file mode 100644 index 000000000..2b99f7201 --- /dev/null +++ b/shortcuts/task/tasklist_create_test.go @@ -0,0 +1,159 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package task + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" +) + +// TestCreateTasklist_PartialFailure exercises the batch sub-task path: the +// tasklist is created (code 0), then two sub-tasks are created concurrently — +// one succeeds, one fails with a typed API error. The command returns the typed +// partial-failure exit signal (*output.PartialFailureError, ExitAPI) via +// runtime.OutPartialFailure, and stdout carries both created_tasks (the +// success) and failed_tasks (the failure) so the partial result is inspectable. +// Sub-tasks are routed by summary via BodyFilter because both POST the same +// /tasks URL and run on separate goroutines. +func TestCreateTasklist_PartialFailure(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists", + Body: map[string]interface{}{ + "code": 0, "msg": "success", + "data": map[string]interface{}{ + "tasklist": map[string]interface{}{ + "guid": "tl-new", + "name": "My List", + "url": "https://example.feishu.cn/tl-new", + }, + }, + }, + }) + + // Succeeding sub-task (summary "ok-task"). + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks", + BodyFilter: func(b []byte) bool { return bytes.Contains(b, []byte("ok-task")) }, + Body: map[string]interface{}{ + "code": 0, "msg": "success", + "data": map[string]interface{}{ + "task": map[string]interface{}{ + "guid": "task-ok", + "url": "https://example.feishu.cn/task-ok", + }, + }, + }, + }) + + // Failing sub-task (summary "bad-task") → typed permission_denied. + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasks", + BodyFilter: func(b []byte) bool { return bytes.Contains(b, []byte("bad-task")) }, + Body: map[string]interface{}{ + "code": ErrCodeTaskPermissionDenied, "msg": "no permission", + }, + }) + + s := CreateTasklist + s.AuthTypes = []string{"bot", "user"} + + data := `[{"summary":"ok-task"},{"summary":"bad-task"}]` + args := []string{"+tasklist-create", "--name", "My List", "--data", data, "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("err = %T, want *output.PartialFailureError; err = %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } + + out := stdout.String() + + // The tasklist itself is created and stays in the payload. + if !strings.Contains(out, "tl-new") { + t.Errorf("expected created tasklist guid tl-new in output, got: %s", out) + } + // Success lands in created_tasks. + if !strings.Contains(out, "task-ok") { + t.Errorf("expected created sub-task task-ok in output, got: %s", out) + } + // Failure lands in failed_tasks (keyed by index + summary). + if !strings.Contains(out, "bad-task") { + t.Errorf("expected failed sub-task bad-task in output, got: %s", out) + } +} + +// TestCreateTasklist_InvalidDataJSON covers the --data validation arm: a string +// that is not a JSON array must surface a typed *errs.ValidationError +// (invalid_argument, exit 2) after the tasklist create succeeds. +func TestCreateTasklist_InvalidDataJSON(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + // No POST /tasklists stub is registered on purpose: invalid --data must be + // rejected before any remote write, leaving no orphan tasklist. If the + // ordering regressed (create first), the POST would hit no stub and surface + // as a non-validation transport error, failing the assertion below. + s := CreateTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-create", "--name", "My List", "--data", "{not-an-array", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d (ExitValidation)", got, output.ExitValidation) + } +} + +// TestCreateTasklist_MalformedResponse covers the create-tasklist parse arm: a +// 200 with a non-JSON body must surface a typed +// *errs.InternalError(invalid_response) (exit 5) from the json.Unmarshal guard. +func TestCreateTasklist_MalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists", + RawBody: []byte("not json"), + }) + + s := CreateTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-create", "--name", "My List", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d (ExitInternal)", got, output.ExitInternal) + } +} diff --git a/shortcuts/task/tasklist_members.go b/shortcuts/task/tasklist_members.go index 3dd644037..2d3603362 100644 --- a/shortcuts/task/tasklist_members.go +++ b/shortcuts/task/tasklist_members.go @@ -5,15 +5,13 @@ package task import ( "context" - "encoding/json" "fmt" "io" "net/http" "net/url" "strings" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -39,7 +37,7 @@ var MembersTasklist = common.Shortcut{ hasRemove := runtime.Str("remove") != "" if hasSet && (hasAdd || hasRemove) { - return WrapTaskError(ErrCodeTaskInvalidParams, "cannot combine --set with --add or --remove", "validate tasklist members") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot combine --set with --add or --remove") } return nil }, @@ -74,8 +72,7 @@ var MembersTasklist = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { tlId := url.PathEscape(extractTasklistGuid(runtime.Str("tasklist-id"))) - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", "open_id") + params := map[string]interface{}{"user_id_type": "open_id"} setStr := runtime.Str("set") addStr := runtime.Str("add") @@ -83,20 +80,7 @@ var MembersTasklist = common.Shortcut{ // If no modifications, just list if setStr == "" && addStr == "" && removeStr == "" { - getResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId, - QueryParams: queryParams, - }) - - var getResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(getResp.RawBody, &getResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse tasklist details") - } - } - - data, err := HandleTaskApiResult(getResult, err, "get tasklist members") + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasklists/"+tlId, params, nil) if err != nil { return err } @@ -142,20 +126,7 @@ var MembersTasklist = common.Shortcut{ var lastTasklist map[string]interface{} if setStr != "" { // Query existing to diff for "set" behavior - getResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId, - QueryParams: queryParams, - }) - - var getResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(getResp.RawBody, &getResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse tasklist details") - } - } - - data, err := HandleTaskApiResult(getResult, err, "get tasklist details for set") + data, err := callTaskAPITyped(runtime, http.MethodGet, "/open-apis/task/v2/tasklists/"+tlId, params, nil) if err != nil { return err } @@ -198,21 +169,7 @@ var MembersTasklist = common.Shortcut{ if len(toAdd) > 0 { body := buildTlMembersBody(strings.Join(toAdd, ",")) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId + "/add_members", - QueryParams: queryParams, - Body: body, - }) - - var addResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &addResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add members") - } - } - - data, err := HandleTaskApiResult(addResult, err, "add tasklist members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists/"+tlId+"/add_members", params, body) if err != nil { return err } @@ -221,21 +178,7 @@ var MembersTasklist = common.Shortcut{ if len(toRemove) > 0 { body := buildTlMembersBody(strings.Join(toRemove, ",")) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId + "/remove_members", - QueryParams: queryParams, - Body: body, - }) - - var removeResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &removeResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove members") - } - } - - data, err := HandleTaskApiResult(removeResult, err, "remove tasklist members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists/"+tlId+"/remove_members", params, body) if err != nil { return err } @@ -246,21 +189,7 @@ var MembersTasklist = common.Shortcut{ // Add / Remove mode if addStr != "" { body := buildTlMembersBody(addStr) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId + "/add_members", - QueryParams: queryParams, - Body: body, - }) - - var addResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &addResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse add members") - } - } - - data, err := HandleTaskApiResult(addResult, err, "add tasklist members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists/"+tlId+"/add_members", params, body) if err != nil { return err } @@ -269,21 +198,7 @@ var MembersTasklist = common.Shortcut{ if removeStr != "" { body := buildTlMembersBody(removeStr) - apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: "/open-apis/task/v2/tasklists/" + tlId + "/remove_members", - QueryParams: queryParams, - Body: body, - }) - - var removeResult map[string]interface{} - if err == nil { - if parseErr := json.Unmarshal(apiResp.RawBody, &removeResult); parseErr != nil { - return WrapTaskError(ErrCodeTaskInternalError, fmt.Sprintf("failed to parse response: %v", parseErr), "parse remove members") - } - } - - data, err := HandleTaskApiResult(removeResult, err, "remove tasklist members") + data, err := callTaskAPITyped(runtime, http.MethodPost, "/open-apis/task/v2/tasklists/"+tlId+"/remove_members", params, body) if err != nil { return err } diff --git a/shortcuts/task/tasklist_members_test.go b/shortcuts/task/tasklist_members_test.go index e32ee052d..4c1a696a3 100644 --- a/shortcuts/task/tasklist_members_test.go +++ b/shortcuts/task/tasklist_members_test.go @@ -4,9 +4,14 @@ package task import ( + "errors" "testing" "github.com/smartystreets/goconvey/convey" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" ) func TestBuildTlMembersBody(t *testing.T) { @@ -16,3 +21,179 @@ func TestBuildTlMembersBody(t *testing.T) { convey.So(len(members), convey.ShouldEqual, 2) }) } + +// TestMembersTasklist_SetCombinedWithAddRejected covers the Validate guard: +// --set is mutually exclusive with --add/--remove. It must surface a typed +// *errs.ValidationError (exit 2) before any API call is made. +func TestMembersTasklist_SetCombinedWithAddRejected(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--set", "ou_a", "--add", "ou_b", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("err = %T, want *errs.ValidationError; err = %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Errorf("exit code = %d, want %d (ExitValidation)", got, output.ExitValidation) + } +} + +// TestMembersTasklist_ListMalformedResponse covers the list arm (no +// set/add/remove): a 200 with a non-JSON body must surface a typed +// *errs.InternalError(invalid_response) (exit 5) from the json.Unmarshal guard, +// not a silent success. +func TestMembersTasklist_ListMalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/task/v2/tasklists/tl-123", + RawBody: []byte("not json"), + }) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + assertInvalidResponse(t, err) +} + +// TestMembersTasklist_SetMalformedResponse covers the --set arm: the diff path +// first GETs the tasklist; a non-JSON body there must surface the typed +// internal invalid_response error. +func TestMembersTasklist_SetMalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/task/v2/tasklists/tl-123", + RawBody: []byte("not json"), + }) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--set", "ou_a", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + assertInvalidResponse(t, err) +} + +// TestMembersTasklist_AddMalformedResponse covers the add/remove arm: the POST +// to add_members returns a non-JSON body, which must surface the typed internal +// invalid_response error. +func TestMembersTasklist_AddMalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists/tl-123/add_members", + RawBody: []byte("not json"), + }) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--add", "ou_a", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + assertInvalidResponse(t, err) +} + +// TestMembersTasklist_SetRemoveDiffMalformedResponse covers the --set diff's +// remove_members arm: the GET returns an existing member absent from the target +// set, so the shortcut issues a remove_members POST whose 200 carries a +// non-JSON body, which must surface the typed internal invalid_response error. +// The target equals one existing member, so no add_members call precedes it. +func TestMembersTasklist_SetRemoveDiffMalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/task/v2/tasklists/tl-123", + Status: 200, + Body: map[string]interface{}{ + "code": 0, + "msg": "success", + "data": map[string]interface{}{ + "tasklist": map[string]interface{}{ + "url": "https://example.com/tl-123", + "members": []interface{}{ + map[string]interface{}{"id": "ou_keep"}, + map[string]interface{}{"id": "ou_drop"}, + }, + }, + }, + }, + }) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists/tl-123/remove_members", + Status: 200, + RawBody: []byte("not json"), + }) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--set", "ou_keep", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + assertInvalidResponse(t, err) +} + +// TestMembersTasklist_RemoveMalformedResponse covers the add/remove mode's +// remove_members arm: with only --remove set, the add arm is skipped and the +// remove_members POST returns a non-JSON body, which must surface the typed +// internal invalid_response error. +func TestMembersTasklist_RemoveMalformedResponse(t *testing.T) { + f, stdout, _, reg := taskShortcutTestFactory(t) + warmTenantToken(t, f, reg) + + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/task/v2/tasklists/tl-123/remove_members", + Status: 200, + RawBody: []byte("not json"), + }) + + s := MembersTasklist + s.AuthTypes = []string{"bot", "user"} + + args := []string{"+tasklist-members", "--tasklist-id", "tl-123", "--remove", "ou_a", "--as", "bot", "--format", "json"} + err := runMountedTaskShortcut(t, s, args, f, stdout) + + assertInvalidResponse(t, err) +} + +// assertInvalidResponse asserts a typed *errs.InternalError(invalid_response) +// with exit 5 — the contract for a parse-response failure across the members +// arms. +func assertInvalidResponse(t *testing.T, err error) { + t.Helper() + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("err = %T, want *errs.InternalError; err = %v", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Errorf("exit code = %d, want %d (ExitInternal)", got, output.ExitInternal) + } +}