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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions shortcuts/task/shortcuts.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"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"
)
Expand Down Expand Up @@ -107,7 +107,7 @@
// 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")
}
}

Expand Down Expand Up @@ -143,7 +143,7 @@
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
}
Expand All @@ -154,7 +154,7 @@

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
Expand Down Expand Up @@ -194,7 +194,7 @@
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
body, err := buildTaskCreateBody(runtime)
if err != nil {
return WrapTaskError(ErrCodeTaskInvalidParams, err.Error(), "create task")
return err

Check warning on line 197 in shortcuts/task/shortcuts.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/shortcuts.go#L197

Added line #L197 was not covered by tests
}

queryParams := make(larkcore.QueryParams)
Expand All @@ -210,7 +210,7 @@
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 errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr)

Check warning on line 213 in shortcuts/task/shortcuts.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/shortcuts.go#L213

Added line #L213 was not covered by tests
}
}

Expand Down
7 changes: 4 additions & 3 deletions shortcuts/task/task_assign.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

larkcore "github.com/larksuite/oapi-sdk-go/v3/core"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/shortcuts/common"
)

Expand All @@ -35,7 +36,7 @@

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")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return nil
},
Expand Down Expand Up @@ -79,7 +80,7 @@
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")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr)

Check warning on line 83 in shortcuts/task/task_assign.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/task_assign.go#L83

Added line #L83 was not covered by tests
}
}

Expand All @@ -102,7 +103,7 @@
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")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr)

Check warning on line 106 in shortcuts/task/task_assign.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/task_assign.go#L106

Added line #L106 was not covered by tests
}
}

Expand Down
55 changes: 55 additions & 0 deletions shortcuts/task/task_assign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,69 @@
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)
}
}

func TestBuildMembersBody(t *testing.T) {
convey.Convey("Build with ids and token", t, func() {
body := buildMembersBody("u1, u2 , ", "assignee", "token1")
Expand Down
74 changes: 34 additions & 40 deletions shortcuts/task/task_body_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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",
},
}
Expand All @@ -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)
}
})
}
Expand All @@ -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",
},
}
Expand All @@ -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)
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion shortcuts/task/task_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

larkcore "github.com/larksuite/oapi-sdk-go/v3/core"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/shortcuts/common"
)

Expand Down Expand Up @@ -61,8 +62,8 @@
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")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr)
}

Check warning on line 66 in shortcuts/task/task_comment.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/task_comment.go#L65-L66

Added lines #L65 - L66 were not covered by tests
}

data, err := HandleTaskApiResult(result, err, "add task comment")
Expand Down
5 changes: 3 additions & 2 deletions shortcuts/task/task_complete.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

larkcore "github.com/larksuite/oapi-sdk-go/v3/core"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/shortcuts/common"
)

Expand Down Expand Up @@ -62,7 +63,7 @@
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")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse get response: %v", parseErr)
}
}

Expand Down Expand Up @@ -90,7 +91,7 @@
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")
return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to parse response: %v", parseErr)

Check warning on line 94 in shortcuts/task/task_complete.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/task/task_complete.go#L94

Added line #L94 was not covered by tests
}
}

Expand Down
Loading
Loading