fix: migrate task shortcut errors from bare fmt.Errorf to structured output.Errorf/ErrValidation (#740)

This commit is contained in:
Zhang-986
2026-05-06 21:45:37 +08:00
committed by GitHub
parent c3756f3642
commit b65147f208
7 changed files with 242 additions and 17 deletions

View File

@@ -13,6 +13,7 @@ import (
"strings"
"time"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/shortcuts/common"
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
)
@@ -106,7 +107,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, fmt.Errorf("--data must be a valid JSON object: %v", err)
return nil, output.ErrValidation("--data must be a valid JSON object: %v", err)
}
}
@@ -142,7 +143,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{}
if dueStr := runtime.Str("due"); dueStr != "" {
dueObj, err := parseTaskTime(dueStr)
if err != nil {
return nil, fmt.Errorf("failed to parse due time: %v", err)
return nil, output.ErrValidation("failed to parse due time: %v", err)
}
body["due"] = dueObj
}
@@ -153,7 +154,7 @@ func buildTaskCreateBody(runtime *common.RuntimeContext) (map[string]interface{}
summary, _ := body["summary"].(string)
if strings.TrimSpace(summary) == "" {
return nil, fmt.Errorf("task summary is required")
return nil, output.ErrValidation("task summary is required")
}
return body, nil
@@ -209,7 +210,7 @@ var CreateTask = common.Shortcut{
var result map[string]interface{}
if err == nil {
if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil {
return fmt.Errorf("failed to parse response: %v", parseErr)
return output.Errorf(output.ExitAPI, "api_error", "failed to parse response: %v", parseErr)
}
}

View File

@@ -0,0 +1,156 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package task
import (
"errors"
"testing"
"github.com/larksuite/cli/internal/output"
"github.com/spf13/cobra"
"github.com/larksuite/cli/shortcuts/common"
)
func TestBuildTaskCreateBody_StructuredErrors(t *testing.T) {
tests := []struct {
name string
data string
summary string
due string
wantCode int
wantType string
wantSubstr string
}{
{
name: "invalid JSON data returns ErrValidation",
data: "not-json",
summary: "test",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "--data must be a valid JSON object",
},
{
name: "missing summary returns ErrValidation",
data: "",
summary: "",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "task summary is required",
},
{
name: "invalid due time returns ErrValidation",
data: "",
summary: "test task",
due: "not-a-valid-time",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "failed to parse due time",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := &cobra.Command{}
cmd.Flags().String("data", tt.data, "")
cmd.Flags().String("summary", tt.summary, "")
cmd.Flags().String("description", "", "")
cmd.Flags().String("assignee", "", "")
cmd.Flags().String("follower", "", "")
cmd.Flags().String("due", tt.due, "")
cmd.Flags().String("tasklist-id", "", "")
cmd.Flags().String("idempotency-key", "", "")
runtime := &common.RuntimeContext{Cmd: cmd}
_, err := buildTaskCreateBody(runtime)
if err == nil {
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)
}
if exitErr.Code != tt.wantCode {
t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode)
}
if exitErr.Detail == nil {
t.Fatal("expected non-nil error detail")
}
if exitErr.Detail.Type != tt.wantType {
t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType)
}
})
}
}
func TestBuildTaskUpdateBody_StructuredErrors(t *testing.T) {
tests := []struct {
name string
data string
summary string
due string
wantCode int
wantType string
wantSubstr string
}{
{
name: "invalid JSON data returns ErrValidation",
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",
data: "",
summary: "",
due: "",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "no fields to update",
},
{
name: "invalid due time returns ErrValidation",
data: "",
summary: "",
due: "not-a-valid-time",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "failed to parse due time",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := &cobra.Command{}
cmd.Flags().String("data", tt.data, "")
cmd.Flags().String("summary", tt.summary, "")
cmd.Flags().String("description", "", "")
cmd.Flags().String("due", tt.due, "")
runtime := &common.RuntimeContext{Cmd: cmd}
_, err := buildTaskUpdateBody(runtime)
if err == nil {
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)
}
if exitErr.Code != tt.wantCode {
t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode)
}
if exitErr.Detail == nil {
t.Fatal("expected non-nil error detail")
}
if exitErr.Detail.Type != tt.wantType {
t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType)
}
})
}
}

View File

@@ -8,6 +8,8 @@ import (
"strconv"
"strings"
"time"
"github.com/larksuite/cli/internal/output"
)
func splitAndTrimCSV(input string) []string {
@@ -44,7 +46,7 @@ func parseTimeRangeMillis(input string) (string, string, error) {
}
startSecInt, err = strconv.ParseInt(startSec, 10, 64)
if err != nil {
return "", "", fmt.Errorf("invalid start timestamp: %w", err)
return "", "", output.ErrValidation("invalid start timestamp: %v", err)
}
hasStart = true
startMillis = startSec + "000"
@@ -56,13 +58,13 @@ func parseTimeRangeMillis(input string) (string, string, error) {
}
endSecInt, err = strconv.ParseInt(endSec, 10, 64)
if err != nil {
return "", "", fmt.Errorf("invalid end timestamp: %w", err)
return "", "", output.ErrValidation("invalid end timestamp: %v", err)
}
hasEnd = true
endMillis = endSec + "000"
}
if hasStart && hasEnd && startSecInt > endSecInt {
return "", "", fmt.Errorf("start time must be earlier than or equal to end time")
return "", "", output.ErrValidation("start time must be earlier than or equal to end time")
}
return startMillis, endMillis, nil
}
@@ -89,7 +91,7 @@ func parseTimeRangeRFC3339(input string) (string, string, error) {
}
startSecInt, err = strconv.ParseInt(startSec, 10, 64)
if err != nil {
return "", "", fmt.Errorf("invalid start timestamp: %w", err)
return "", "", output.ErrValidation("invalid start timestamp: %v", err)
}
hasStart = true
startTime = time.Unix(startSecInt, 0).Local().Format(time.RFC3339)
@@ -101,13 +103,13 @@ func parseTimeRangeRFC3339(input string) (string, string, error) {
}
endSecInt, err = strconv.ParseInt(endSec, 10, 64)
if err != nil {
return "", "", fmt.Errorf("invalid end timestamp: %w", err)
return "", "", output.ErrValidation("invalid end timestamp: %v", err)
}
hasEnd = true
endTime = time.Unix(endSecInt, 0).Local().Format(time.RFC3339)
}
if hasStart && hasEnd && startSecInt > endSecInt {
return "", "", fmt.Errorf("start time must be earlier than or equal to end time")
return "", "", output.ErrValidation("start time must be earlier than or equal to end time")
}
return startTime, endTime, nil
}

View File

@@ -4,8 +4,11 @@
package task
import (
"errors"
"strings"
"testing"
"github.com/larksuite/cli/internal/output"
)
func TestSplitAndTrimCSV(t *testing.T) {
@@ -95,6 +98,18 @@ func TestParseTimeRangeMillisAndRequireSearchFilter(t *testing.T) {
if err == nil {
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)
}
if exitErr.Code != output.ExitValidation {
t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation)
}
if exitErr.Detail == nil || exitErr.Detail.Type != "validation" {
t.Errorf("error detail type = %q, want %q", exitErr.Detail.Type, "validation")
}
}
return
}
if err != nil {
@@ -260,6 +275,15 @@ func TestRenderRelatedTasksPretty(t *testing.T) {
if err == nil {
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)
}
if exitErr.Code != output.ExitValidation {
t.Errorf("exit code = %d, want %d", exitErr.Code, output.ExitValidation)
}
}
return
}
if err != nil {

View File

@@ -76,7 +76,7 @@ var UpdateTask = common.Shortcut{
var result map[string]interface{}
if err == nil {
if parseErr := json.Unmarshal(apiResp.RawBody, &result); parseErr != nil {
return fmt.Errorf("failed to parse response for task %s: %v", taskId, parseErr)
return output.Errorf(output.ExitAPI, "api_error", "failed to parse response for task %s: %v", taskId, parseErr)
}
}
@@ -133,7 +133,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, fmt.Errorf("--data must be a valid JSON object: %v", err)
return nil, output.ErrValidation("--data must be a valid JSON object: %v", err)
}
// If data is provided, assume keys are update fields
for k := range taskObj {
@@ -158,7 +158,7 @@ func buildTaskUpdateBody(runtime *common.RuntimeContext) (map[string]interface{}
if dueStr := runtime.Str("due"); dueStr != "" {
dueObj, err := parseTaskTime(dueStr)
if err != nil {
return nil, fmt.Errorf("failed to parse due time: %v", err)
return nil, output.ErrValidation("failed to parse due time: %v", err)
}
taskObj["due"] = dueObj
if !contains(updateFields, "due") {
@@ -167,7 +167,7 @@ func buildTaskUpdateBody(runtime *common.RuntimeContext) (map[string]interface{}
}
if len(updateFields) == 0 {
return nil, fmt.Errorf("no fields to update")
return nil, output.ErrValidation("no fields to update")
}
return map[string]interface{}{

View File

@@ -24,7 +24,7 @@ func isRelativeTime(s string) bool {
func parseRelativeTime(s string) (time.Time, error) {
matches := relativeTimeRe.FindStringSubmatch(s)
if len(matches) == 0 {
return time.Time{}, fmt.Errorf("invalid relative time format: %s", s)
return time.Time{}, output.ErrValidation("invalid relative time format: %s", s)
}
sign := matches[1]
@@ -50,9 +50,8 @@ func parseRelativeTime(s string) (time.Time, error) {
return now.Add(time.Duration(amount) * time.Minute), nil
case "h":
return now.Add(time.Duration(amount) * time.Hour), nil
default:
return time.Time{}, fmt.Errorf("unknown unit: %s", unit)
}
panic(fmt.Sprintf("unreachable: relativeTimeRe matched unexpected unit %q", unit))
}
const (

View File

@@ -4,8 +4,10 @@
package task
import (
"errors"
"testing"
"github.com/larksuite/cli/internal/output"
"github.com/smartystreets/goconvey/convey"
)
@@ -17,3 +19,44 @@ func TestContains(t *testing.T) {
convey.So(contains([]string{}, "a"), convey.ShouldBeFalse)
})
}
func TestParseRelativeTime_StructuredErrors(t *testing.T) {
tests := []struct {
name string
input string
wantCode int
wantType string
wantSubstr string
}{
{
name: "invalid format returns ErrValidation",
input: "not-relative",
wantCode: output.ExitValidation,
wantType: "validation",
wantSubstr: "invalid relative time format",
},
}
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)
}
var exitErr *output.ExitError
if !errors.As(err, &exitErr) {
t.Fatalf("error type = %T, want *output.ExitError; error = %v", err, err)
}
if exitErr.Code != tt.wantCode {
t.Errorf("exit code = %d, want %d", exitErr.Code, tt.wantCode)
}
if exitErr.Detail == nil {
t.Fatal("expected non-nil error detail")
}
if exitErr.Detail.Type != tt.wantType {
t.Errorf("error type = %q, want %q", exitErr.Detail.Type, tt.wantType)
}
})
}
}