From 4b19089c8ec8d0404126066e6e82aa9867a82853 Mon Sep 17 00:00:00 2001 From: "zhaoyukun.yk" Date: Tue, 2 Jun 2026 23:31:47 +0800 Subject: [PATCH] feat(okr,whiteboard): emit typed error envelopes across both domains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The okr and whiteboard commands now report every failure as a typed error envelope. Invalid flags, malformed input, output-file conflicts, and API or transport failures alike carry a stable category, subtype, the offending flag or Lark error code, and a meaningful exit code — so scripts and agents can branch on the error shape instead of scraping message strings. --- .golangci.yml | 10 +- shortcuts/common/runner.go | 21 --- shortcuts/okr/okr_cycle_detail.go | 20 ++- shortcuts/okr/okr_cycle_detail_test.go | 27 ++++ shortcuts/okr/okr_cycle_list.go | 29 ++-- shortcuts/okr/okr_errors.go | 34 +++++ shortcuts/okr/okr_errors_test.go | 47 +++++++ shortcuts/okr/okr_image_upload.go | 37 ++--- shortcuts/okr/okr_image_upload_test.go | 20 +++ shortcuts/okr/okr_progress_create.go | 31 ++--- shortcuts/okr/okr_progress_delete.go | 8 +- shortcuts/okr/okr_progress_get.go | 13 +- shortcuts/okr/okr_progress_list.go | 25 ++-- shortcuts/okr/okr_progress_update.go | 29 ++-- shortcuts/whiteboard/whiteboard_errors.go | 39 ++++++ .../whiteboard/whiteboard_errors_test.go | 45 ++++++ shortcuts/whiteboard/whiteboard_query.go | 61 ++++----- shortcuts/whiteboard/whiteboard_query_test.go | 88 +++++++++++- shortcuts/whiteboard/whiteboard_update.go | 128 +++++++----------- .../whiteboard/whiteboard_update_test.go | 71 +++++++++- 20 files changed, 538 insertions(+), 245 deletions(-) create mode 100644 shortcuts/okr/okr_errors.go create mode 100644 shortcuts/okr/okr_errors_test.go create mode 100644 shortcuts/whiteboard/whiteboard_errors.go create mode 100644 shortcuts/whiteboard/whiteboard_errors_test.go diff --git a/.golangci.yml b/.golangci.yml index f9a51eae8..b8042d61b 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/okr/|shortcuts/whiteboard/) 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/okr/|shortcuts/whiteboard/|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/) + # errs-no-legacy-helper bans shared helpers that emit legacy output.Err* + # shapes, on domains that have migrated to typed errs.* builders. + - path-except: (shortcuts/drive/|shortcuts/okr/|shortcuts/whiteboard/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index e091754b8..15ec06aa6 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -586,27 +586,6 @@ func (ctx *RuntimeContext) ResolveSavePath(path string) (string, error) { return resolved, nil } -// WrapSaveError matches a FileIO.Save error against known categories and wraps -// it with the caller-provided message prefix, preserving backward-compatible -// error text per shortcut. -func WrapSaveError(err error, pathMsg, mkdirMsg, writeMsg string) error { - if err == nil { - return nil - } - var me *fileio.MkdirError - var we *fileio.WriteError - switch { - case errors.Is(err, fileio.ErrPathValidation): - return fmt.Errorf("%s: %w", pathMsg, err) - case errors.As(err, &me): - return fmt.Errorf("%s: %w", mkdirMsg, err) - case errors.As(err, &we): - return fmt.Errorf("%s: %w", writeMsg, err) - default: - return fmt.Errorf("%s: %w", writeMsg, err) - } -} - // WrapOpenError matches a FileIO.Open/Stat error and wraps it with the // caller-provided message prefix. func WrapOpenError(err error, pathMsg, readMsg string) error { diff --git a/shortcuts/okr/okr_cycle_detail.go b/shortcuts/okr/okr_cycle_detail.go index 719c22ca4..3839e1d1a 100644 --- a/shortcuts/okr/okr_cycle_detail.go +++ b/shortcuts/okr/okr_cycle_detail.go @@ -11,8 +11,8 @@ import ( "strconv" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // OKRCycleDetail lists all objectives and their key results under a given OKR cycle. @@ -30,10 +30,10 @@ var OKRCycleDetail = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { cycleID := runtime.Str("cycle-id") if cycleID == "" { - return common.FlagErrorf("--cycle-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id is required").WithParam("--cycle-id") } if id, err := strconv.ParseInt(cycleID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--cycle-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id must be a positive int64").WithParam("--cycle-id") } return nil }, @@ -52,8 +52,7 @@ var OKRCycleDetail = common.Shortcut{ cycleID := runtime.Str("cycle-id") // Paginate objectives under the cycle. - queryParams := make(larkcore.QueryParams) - queryParams.Set("page_size", "100") + queryParams := map[string]interface{}{"page_size": "100"} var objectives []Objective page := 0 @@ -71,7 +70,7 @@ var OKRCycleDetail = common.Shortcut{ page++ path := fmt.Sprintf("/open-apis/okr/v2/cycles/%s/objectives", cycleID) - data, err := runtime.DoAPIJSON("GET", path, queryParams, nil) + data, err := runtime.CallAPITyped("GET", path, queryParams, nil) if err != nil { return err } @@ -93,7 +92,7 @@ var OKRCycleDetail = common.Shortcut{ if !hasMore || pageToken == "" { break } - queryParams.Set("page_token", pageToken) + queryParams["page_token"] = pageToken } // For each objective, paginate key results and convert to response format. @@ -104,8 +103,7 @@ var OKRCycleDetail = common.Shortcut{ } obj := &objectives[i] - krQuery := make(larkcore.QueryParams) - krQuery.Set("page_size", "100") + krQuery := map[string]interface{}{"page_size": "100"} var keyResults []KeyResult krPage := 0 @@ -123,7 +121,7 @@ var OKRCycleDetail = common.Shortcut{ krPage++ path := fmt.Sprintf("/open-apis/okr/v2/objectives/%s/key_results", obj.ID) - data, err := runtime.DoAPIJSON("GET", path, krQuery, nil) + data, err := runtime.CallAPITyped("GET", path, krQuery, nil) if err != nil { return err } @@ -145,7 +143,7 @@ var OKRCycleDetail = common.Shortcut{ if !hasMore || pageToken == "" { break } - krQuery.Set("page_token", pageToken) + krQuery["page_token"] = pageToken } respObj := obj.ToResp() diff --git a/shortcuts/okr/okr_cycle_detail_test.go b/shortcuts/okr/okr_cycle_detail_test.go index edb3b8c1b..f1eb759bc 100644 --- a/shortcuts/okr/okr_cycle_detail_test.go +++ b/shortcuts/okr/okr_cycle_detail_test.go @@ -6,11 +6,13 @@ package okr import ( "bytes" "encoding/json" + "errors" "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" @@ -106,6 +108,31 @@ func TestCycleDetailValidate_InvalidCycleID_Negative(t *testing.T) { } } +// TestCycleDetailValidate_TypedError locks the typed-envelope contract shared by +// every okr flag check: an invalid flag surfaces as *errs.ValidationError carrying +// SubtypeInvalidArgument and the offending --flag (readable via errors.As / +// errs.ProblemOf), and maps to the validation exit code rather than a legacy api error. +func TestCycleDetailValidate_TypedError(t *testing.T) { + t.Parallel() + f, stdout, _, _ := cmdutil.TestFactory(t, cycleDetailTestConfig(t)) + err := runCycleDetailShortcut(t, f, stdout, []string{"+cycle-detail", "--cycle-id", "0"}) + + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error is not *errs.ValidationError: %T (%v)", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--cycle-id" { + t.Errorf("Param = %q, want %q", ve.Param, "--cycle-id") + } + p, ok := errs.ProblemOf(err) + if !ok || p.Category != errs.CategoryValidation { + t.Errorf("ProblemOf category = %v (ok=%v), want %q", p, ok, errs.CategoryValidation) + } +} + func TestCycleDetailValidate_ValidCycleID(t *testing.T) { t.Parallel() f, stdout, _, reg := cmdutil.TestFactory(t, cycleDetailTestConfig(t)) diff --git a/shortcuts/okr/okr_cycle_list.go b/shortcuts/okr/okr_cycle_list.go index df9e96587..b78abd962 100644 --- a/shortcuts/okr/okr_cycle_list.go +++ b/shortcuts/okr/okr_cycle_list.go @@ -12,9 +12,9 @@ import ( "strings" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // parseTimeRange parses a "YYYY-MM--YYYY-MM" string into two time.Time values. @@ -22,20 +22,20 @@ import ( func parseTimeRange(s string) (start, end time.Time, err error) { parts := strings.SplitN(s, "--", 2) if len(parts) != 2 { - return time.Time{}, time.Time{}, fmt.Errorf("invalid time-range format %q, expected YYYY-MM--YYYY-MM", s) + return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range format %q, expected YYYY-MM--YYYY-MM", s).WithParam("--time-range") } start, err = time.Parse("2006-01", strings.TrimSpace(parts[0])) if err != nil { - return time.Time{}, time.Time{}, fmt.Errorf("invalid start month %q: %w", parts[0], err) + return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range start month %q: %v", parts[0], err).WithParam("--time-range").WithCause(err) } end, err = time.Parse("2006-01", strings.TrimSpace(parts[1])) if err != nil { - return time.Time{}, time.Time{}, fmt.Errorf("invalid end month %q: %w", parts[1], err) + return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range end month %q: %v", parts[1], err).WithParam("--time-range").WithCause(err) } // end is the last moment of the end month end = end.AddDate(0, 1, 0).Add(-time.Millisecond) if start.After(end) { - return time.Time{}, time.Time{}, fmt.Errorf("start month %s is after end month %s", parts[0], parts[1]) + return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range: start month %s is after end month %s", parts[0], parts[1]).WithParam("--time-range") } return start, end, nil } @@ -69,7 +69,7 @@ var OKRListCycles = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { idType := runtime.Str("user-id-type") if idType != "open_id" && idType != "union_id" && idType != "user_id" { - return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type") } userID := runtime.Str("user-id") if err := validate.RejectControlChars(userID, "user-id"); err != nil { @@ -82,7 +82,7 @@ var OKRListCycles = common.Shortcut{ return err } if _, _, err := parseTimeRange(tr); err != nil { - return common.FlagErrorf("--time-range: %s", err) + return err } } return nil @@ -110,16 +110,17 @@ var OKRListCycles = common.Shortcut{ var err error rangeStart, rangeEnd, err = parseTimeRange(timeRange) if err != nil { - return common.FlagErrorf("--time-range: %s", err) + return err } hasRange = true } // Paginated fetch of all cycles - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id", userID) - queryParams.Set("user_id_type", userIDType) - queryParams.Set("page_size", "100") + queryParams := map[string]interface{}{ + "user_id": userID, + "user_id_type": userIDType, + "page_size": "100", + } var allCycles []Cycle page := 0 @@ -136,7 +137,7 @@ var OKRListCycles = common.Shortcut{ } page++ - data, err := runtime.DoAPIJSON("GET", "/open-apis/okr/v2/cycles", queryParams, nil) + data, err := runtime.CallAPITyped("GET", "/open-apis/okr/v2/cycles", queryParams, nil) if err != nil { return err } @@ -158,7 +159,7 @@ var OKRListCycles = common.Shortcut{ if !hasMore || pageToken == "" { break } - queryParams.Set("page_token", pageToken) + queryParams["page_token"] = pageToken } // Filter by time-range overlap diff --git a/shortcuts/okr/okr_errors.go b/shortcuts/okr/okr_errors.go new file mode 100644 index 000000000..29433c36d --- /dev/null +++ b/shortcuts/okr/okr_errors.go @@ -0,0 +1,34 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package okr + +import ( + "errors" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +// okrInputStatError maps a FileIO.Stat/Open error for input file validation to +// a typed validation error: path validation failures read as "unsafe file +// path", other errors as "cannot read file". +func okrInputStatError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, fileio.ErrPathValidation) { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).WithCause(err) + } + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read file: %s", err).WithCause(err) +} + +// wrapOkrNetworkErr returns err unchanged when it is already a typed errs.* +// error (preserving subtype / code / log_id from the runtime boundary) and only +// wraps a raw, unclassified error as a transport-level network error. +func wrapOkrNetworkErr(err error, format string, args ...any) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) +} diff --git a/shortcuts/okr/okr_errors_test.go b/shortcuts/okr/okr_errors_test.go new file mode 100644 index 000000000..f626fefc1 --- /dev/null +++ b/shortcuts/okr/okr_errors_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package okr + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +func TestOkrInputStatError(t *testing.T) { + if okrInputStatError(nil) != nil { + t.Fatal("nil error should map to nil") + } + + var ve *errs.ValidationError + + pathErr := okrInputStatError(&fileio.PathValidationError{Err: errors.New("traversal")}) + if !errors.As(pathErr, &ve) || ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("path validation: got %T (%v)", pathErr, pathErr) + } + + genericErr := okrInputStatError(errors.New("permission denied")) + if !errors.As(genericErr, &ve) || ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("generic: got %T (%v)", genericErr, genericErr) + } +} + +func TestWrapOkrNetworkErr(t *testing.T) { + typed := errs.NewValidationError(errs.SubtypeInvalidArgument, "already typed") + if got := wrapOkrNetworkErr(typed, "wrap %v", typed); got != error(typed) { + t.Fatalf("typed error must pass through unchanged, got %v", got) + } + + raw := errors.New("dial tcp: i/o timeout") + got := wrapOkrNetworkErr(raw, "upload failed: %v", raw) + var ne *errs.NetworkError + if !errors.As(got, &ne) || ne.Subtype != errs.SubtypeNetworkTransport { + t.Fatalf("raw error: got %T (%v)", got, got) + } + if !errors.Is(got, raw) { + t.Fatal("raw cause should be retained via WithCause") + } +} diff --git a/shortcuts/okr/okr_image_upload.go b/shortcuts/okr/okr_image_upload.go index d81e8d4b6..397145503 100644 --- a/shortcuts/okr/okr_image_upload.go +++ b/shortcuts/okr/okr_image_upload.go @@ -5,7 +5,6 @@ package okr import ( "context" - "encoding/json" "errors" "fmt" "path/filepath" @@ -14,6 +13,7 @@ import ( 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" ) @@ -43,24 +43,24 @@ var OKRUploadImage = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { filePath := runtime.Str("file") if filePath == "" { - return common.FlagErrorf("--file is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file is required").WithParam("--file") } ext := strings.ToLower(filepath.Ext(filePath)) if !allowedImageExts[ext] { - return common.FlagErrorf("--file must be an image (supported: JPG, JPEG, PNG, GIF, BMP), got %q", ext) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file must be an image (supported: JPG, JPEG, PNG, GIF, BMP), got %q", ext).WithParam("--file") } targetID := runtime.Str("target-id") if targetID == "" { - return common.FlagErrorf("--target-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id is required").WithParam("--target-id") } if id, err := strconv.ParseInt(targetID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--target-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id must be a positive int64").WithParam("--target-id") } targetType := runtime.Str("target-type") if _, ok := targetTypeAllowed[targetType]; !ok { - return common.FlagErrorf("--target-type must be one of: objective | key_result") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-type must be one of: objective | key_result").WithParam("--target-type") } return nil }, @@ -87,12 +87,12 @@ var OKRUploadImage = common.Shortcut{ info, err := runtime.FileIO().Stat(filePath) if err != nil { - return common.WrapInputStatError(err) + return okrInputStatError(err) } f, err := runtime.FileIO().Open(filePath) if err != nil { - return common.WrapInputStatError(err) + return okrInputStatError(err) } defer f.Close() @@ -114,26 +114,19 @@ var OKRUploadImage = common.Shortcut{ if errors.As(err, &exitErr) { return err } - return output.ErrNetwork("upload failed: %v", err) + return wrapOkrNetworkErr(err, "upload failed: %v", err) } - var result map[string]interface{} - if err := json.Unmarshal(apiResp.RawBody, &result); err != nil { - return output.Errorf(output.ExitAPI, "api_error", "upload failed: invalid response JSON: %v", err) - } - - if larkCode := int(common.GetFloat(result, "code")); larkCode != 0 { - msg, _ := result["msg"].(string) - return output.ErrAPI(larkCode, fmt.Sprintf("upload failed: [%d] %s", larkCode, msg), result["error"]) + data, err := runtime.ClassifyAPIResponse(apiResp) + if err != nil { + return err } - data, _ := result["data"].(map[string]interface{}) - fileToken, _ := data["file_token"].(string) - url, _ := data["url"].(string) - + fileToken := common.GetString(data, "file_token") if fileToken == "" { - return output.Errorf(output.ExitAPI, "api_error", "upload failed: no file_token returned") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "upload failed: no file_token returned") } + url := common.GetString(data, "url") runtime.Out(map[string]interface{}{ "file_token": fileToken, diff --git a/shortcuts/okr/okr_image_upload_test.go b/shortcuts/okr/okr_image_upload_test.go index 40d34d0fa..ea8b67cef 100644 --- a/shortcuts/okr/okr_image_upload_test.go +++ b/shortcuts/okr/okr_image_upload_test.go @@ -5,6 +5,7 @@ package okr import ( "bytes" + "errors" "mime" "mime/multipart" "os" @@ -13,6 +14,7 @@ import ( "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" @@ -360,6 +362,15 @@ func TestUploadImageExecute_APIError(t *testing.T) { if err == nil { t.Fatal("expected error for API failure") } + // The upload boundary now classifies the Lark error code into a typed + // envelope carrying the numeric code, instead of a flat legacy error. + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error is not a typed errs.* envelope: %T (%v)", err, err) + } + if p.Code != 1001001 { + t.Errorf("Problem.Code = %d, want 1001001", p.Code) + } } func TestUploadImageExecute_FileNotFound(t *testing.T) { @@ -407,6 +418,15 @@ func TestUploadImageExecute_NoFileTokenInResponse(t *testing.T) { if err == nil { t.Fatal("expected error for missing file_token in response") } + // A 2xx response that omits the expected file_token is a malformed response, + // surfaced as a typed internal/invalid_response error. + var ie *errs.InternalError + if !errors.As(err, &ie) { + t.Fatalf("error is not *errs.InternalError: %T (%v)", err, err) + } + if ie.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("Subtype = %q, want %q", ie.Subtype, errs.SubtypeInvalidResponse) + } if !strings.Contains(err.Error(), "no file_token returned") { t.Fatalf("unexpected error: %v", err) } diff --git a/shortcuts/okr/okr_progress_create.go b/shortcuts/okr/okr_progress_create.go index 6f5baac7f..0b0f74e66 100644 --- a/shortcuts/okr/okr_progress_create.go +++ b/shortcuts/okr/okr_progress_create.go @@ -11,10 +11,10 @@ import ( "math" "strconv" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // targetTypeAllowed values for --target-type flag @@ -39,7 +39,7 @@ func parseCreateProgressRecordParams(runtime *common.RuntimeContext) (*createPro content := runtime.Str("content") var cb ContentBlock if err := json.Unmarshal([]byte(content), &cb); err != nil { - return nil, common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content") } contentV1 := cb.ToV1() @@ -60,13 +60,13 @@ func parseCreateProgressRecordParams(runtime *common.RuntimeContext) (*createPro if v := runtime.Str("progress-percent"); v != "" { percent, err := strconv.ParseFloat(v, 64) if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 { - return nil, common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent") } progressRate = &ProgressRateV1{Percent: &percent} if s := runtime.Str("progress-status"); s != "" { status, ok := ParseProgressStatus(s) if !ok { - return nil, common.FlagErrorf("--progress-status must be one of: normal | overdue | done") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status") } progressRate.Status = int32Ptr(int32(status)) } @@ -105,7 +105,7 @@ var OKRCreateProgressRecord = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { content := runtime.Str("content") if content == "" { - return common.FlagErrorf("--content is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content is required").WithParam("--content") } if err := validate.RejectControlChars(content, "content"); err != nil { return err @@ -113,23 +113,23 @@ var OKRCreateProgressRecord = common.Shortcut{ // Validate content is valid JSON and can be parsed as ContentBlock var cb ContentBlock if err := json.Unmarshal([]byte(content), &cb); err != nil { - return common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content") } targetID := runtime.Str("target-id") if targetID == "" { - return common.FlagErrorf("--target-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id is required").WithParam("--target-id") } if err := validate.RejectControlChars(targetID, "target-id"); err != nil { return err } if id, err := strconv.ParseInt(targetID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--target-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id must be a positive int64").WithParam("--target-id") } targetType := runtime.Str("target-type") if _, ok := targetTypeAllowed[targetType]; !ok { - return common.FlagErrorf("--target-type must be one of: objective | key_result") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-type must be one of: objective | key_result").WithParam("--target-type") } if v := runtime.Str("source-title"); v != "" { @@ -146,21 +146,21 @@ var OKRCreateProgressRecord = common.Shortcut{ if v := runtime.Str("progress-percent"); v != "" { percent, err := strconv.ParseFloat(v, 64) if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 { - return common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent") } } if v := runtime.Str("progress-status"); v != "" { if _, ok := ParseProgressStatus(v); !ok { - return common.FlagErrorf("--progress-status must be one of: normal | overdue | done") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status") } if v := runtime.Str("progress-percent"); v == "" { - return common.FlagErrorf("--progress-percent must provided with --progress-status") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must provided with --progress-status").WithParam("--progress-percent") } } idType := runtime.Str("user-id-type") if idType != "open_id" && idType != "union_id" && idType != "user_id" { - return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type") } return nil }, @@ -202,10 +202,9 @@ var OKRCreateProgressRecord = common.Shortcut{ body["progress_rate"] = p.ProgressRate } - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", p.UserIDType) + queryParams := map[string]interface{}{"user_id_type": p.UserIDType} - data, err := runtime.DoAPIJSON("POST", "/open-apis/okr/v1/progress_records/", queryParams, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/okr/v1/progress_records/", queryParams, body) if err != nil { return err } diff --git a/shortcuts/okr/okr_progress_delete.go b/shortcuts/okr/okr_progress_delete.go index 5738a116c..4ae321a40 100644 --- a/shortcuts/okr/okr_progress_delete.go +++ b/shortcuts/okr/okr_progress_delete.go @@ -9,8 +9,8 @@ import ( "io" "strconv" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // OKRDeleteProgressRecord deletes a progress by ID. @@ -28,10 +28,10 @@ var OKRDeleteProgressRecord = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { progressID := runtime.Str("progress-id") if progressID == "" { - return common.FlagErrorf("--progress-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id is required").WithParam("--progress-id") } if id, err := strconv.ParseInt(progressID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--progress-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id must be a positive int64").WithParam("--progress-id") } return nil }, @@ -46,7 +46,7 @@ var OKRDeleteProgressRecord = common.Shortcut{ progressID := runtime.Str("progress-id") path := fmt.Sprintf("/open-apis/okr/v1/progress_records/%s", progressID) - _, err := runtime.DoAPIJSON("DELETE", path, larkcore.QueryParams{}, nil) + _, err := runtime.CallAPITyped("DELETE", path, nil, nil) if err != nil { return err } diff --git a/shortcuts/okr/okr_progress_get.go b/shortcuts/okr/okr_progress_get.go index 0504e484f..6aa87d3c7 100644 --- a/shortcuts/okr/okr_progress_get.go +++ b/shortcuts/okr/okr_progress_get.go @@ -10,8 +10,8 @@ import ( "io" "strconv" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // OKRGetProgressRecord gets a progress by ID. @@ -30,14 +30,14 @@ var OKRGetProgressRecord = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { progressID := runtime.Str("progress-id") if progressID == "" { - return common.FlagErrorf("--progress-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id is required").WithParam("--progress-id") } if id, err := strconv.ParseInt(progressID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--progress-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id must be a positive int64").WithParam("--progress-id") } idType := runtime.Str("user-id-type") if idType != "open_id" && idType != "union_id" && idType != "user_id" { - return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type") } return nil }, @@ -56,11 +56,10 @@ var OKRGetProgressRecord = common.Shortcut{ progressID := runtime.Str("progress-id") userIDType := runtime.Str("user-id-type") - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", userIDType) + queryParams := map[string]interface{}{"user_id_type": userIDType} path := fmt.Sprintf("/open-apis/okr/v1/progress_records/%s", progressID) - data, err := runtime.DoAPIJSON("GET", path, queryParams, nil) + data, err := runtime.CallAPITyped("GET", path, queryParams, nil) if err != nil { return err } diff --git a/shortcuts/okr/okr_progress_list.go b/shortcuts/okr/okr_progress_list.go index dbda3f8e6..4727b4b94 100644 --- a/shortcuts/okr/okr_progress_list.go +++ b/shortcuts/okr/okr_progress_list.go @@ -10,9 +10,9 @@ import ( "io" "strconv" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // OKRListProgress lists progress for an objective or key result. @@ -33,28 +33,28 @@ var OKRListProgress = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { targetID := runtime.Str("target-id") if targetID == "" { - return common.FlagErrorf("--target-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id is required").WithParam("--target-id") } if err := validate.RejectControlChars(targetID, "target-id"); err != nil { return err } if id, err := strconv.ParseInt(targetID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--target-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id must be a positive int64").WithParam("--target-id") } targetType := runtime.Str("target-type") if _, ok := targetTypeAllowed[targetType]; !ok { - return common.FlagErrorf("--target-type must be one of: objective | key_result") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-type must be one of: objective | key_result").WithParam("--target-type") } idType := runtime.Str("user-id-type") if idType != "open_id" && idType != "union_id" && idType != "user_id" { - return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type") } deptIDType := runtime.Str("department-id-type") if deptIDType != "department_id" && deptIDType != "open_department_id" { - return common.FlagErrorf("--department-id-type must be one of: department_id | open_department_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--department-id-type must be one of: department_id | open_department_id").WithParam("--department-id-type") } return nil }, @@ -89,10 +89,11 @@ var OKRListProgress = common.Shortcut{ userIDType := runtime.Str("user-id-type") deptIDType := runtime.Str("department-id-type") - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", userIDType) - queryParams.Set("department_id_type", deptIDType) - queryParams.Set("page_size", "100") + queryParams := map[string]interface{}{ + "user_id_type": userIDType, + "department_id_type": deptIDType, + "page_size": "100", + } var apiPath string switch targetType { @@ -108,7 +109,7 @@ var OKRListProgress = common.Shortcut{ return err } - data, err := runtime.DoAPIJSON("GET", apiPath, queryParams, nil) + data, err := runtime.CallAPITyped("GET", apiPath, queryParams, nil) if err != nil { return err } @@ -130,7 +131,7 @@ var OKRListProgress = common.Shortcut{ if !hasMore || pageToken == "" { break } - queryParams.Set("page_token", pageToken) + queryParams["page_token"] = pageToken } // Convert to response format diff --git a/shortcuts/okr/okr_progress_update.go b/shortcuts/okr/okr_progress_update.go index 6a2cfedb2..0e31afab4 100644 --- a/shortcuts/okr/okr_progress_update.go +++ b/shortcuts/okr/okr_progress_update.go @@ -11,9 +11,9 @@ import ( "math" "strconv" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) // updateProgressRecordParams holds the parsed parameters for updating a progress. @@ -29,7 +29,7 @@ func parseUpdateProgressRecordParams(runtime *common.RuntimeContext) (*updatePro content := runtime.Str("content") var cb ContentBlock if err := json.Unmarshal([]byte(content), &cb); err != nil { - return nil, common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content") } contentV1 := cb.ToV1() @@ -37,13 +37,13 @@ func parseUpdateProgressRecordParams(runtime *common.RuntimeContext) (*updatePro if v := runtime.Str("progress-percent"); v != "" { percent, err := strconv.ParseFloat(v, 64) if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 { - return nil, common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent") } progressRate = &ProgressRateV1{Percent: &percent} if s := runtime.Str("progress-status"); s != "" { status, ok := ParseProgressStatus(s) if !ok { - return nil, common.FlagErrorf("--progress-status must be one of: normal | overdue | done") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status") } progressRate.Status = int32Ptr(int32(status)) } @@ -76,42 +76,42 @@ var OKRUpdateProgressRecord = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { progressID := runtime.Str("progress-id") if progressID == "" { - return common.FlagErrorf("--progress-id is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id is required").WithParam("--progress-id") } if id, err := strconv.ParseInt(progressID, 10, 64); err != nil || id <= 0 { - return common.FlagErrorf("--progress-id must be a positive int64") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id must be a positive int64").WithParam("--progress-id") } content := runtime.Str("content") if content == "" { - return common.FlagErrorf("--content is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content is required").WithParam("--content") } if err := validate.RejectControlChars(content, "content"); err != nil { return err } var cb ContentBlock if err := json.Unmarshal([]byte(content), &cb); err != nil { - return common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content") } if v := runtime.Str("progress-percent"); v != "" { percent, err := strconv.ParseFloat(v, 64) if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 { - return common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent") } } if v := runtime.Str("progress-status"); v != "" { if _, ok := ParseProgressStatus(v); !ok { - return common.FlagErrorf("--progress-status must be one of: normal | overdue | done") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status") } if v := runtime.Str("progress-percent"); v == "" { - return common.FlagErrorf("--progress-percent must provided with --progress-status") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must provided with --progress-status").WithParam("--progress-percent") } } idType := runtime.Str("user-id-type") if idType != "open_id" && idType != "union_id" && idType != "user_id" { - return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type") } return nil }, @@ -146,11 +146,10 @@ var OKRUpdateProgressRecord = common.Shortcut{ body["progress_rate"] = p.ProgressRate } - queryParams := make(larkcore.QueryParams) - queryParams.Set("user_id_type", p.UserIDType) + queryParams := map[string]interface{}{"user_id_type": p.UserIDType} path := fmt.Sprintf("/open-apis/okr/v1/progress_records/%s", p.ProgressID) - data, err := runtime.DoAPIJSON("PUT", path, queryParams, body) + data, err := runtime.CallAPITyped("PUT", path, queryParams, body) if err != nil { return err } diff --git a/shortcuts/whiteboard/whiteboard_errors.go b/shortcuts/whiteboard/whiteboard_errors.go new file mode 100644 index 000000000..4bec7a471 --- /dev/null +++ b/shortcuts/whiteboard/whiteboard_errors.go @@ -0,0 +1,39 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package whiteboard + +import ( + "errors" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +// wrapWbNetworkErr returns err unchanged when it is already a typed errs.* error +// (preserving subtype / code / log_id from the runtime boundary) and only wraps +// a raw, unclassified error as a transport-level network error. +func wrapWbNetworkErr(err error, format string, args ...any) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) +} + +// wbSaveError maps a FileIO.Save error to a typed error. Path validation +// failures are validation errors (exit code 2); mkdir / write failures are +// internal file-I/O errors (exit code 5). +func wbSaveError(err error) error { + if err == nil { + return nil + } + var me *fileio.MkdirError + switch { + case errors.Is(err, fileio.ErrPathValidation): + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithCause(err) + case errors.As(err, &me): + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create parent directory: %s", err).WithCause(err) + default: + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create file: %s", err).WithCause(err) + } +} diff --git a/shortcuts/whiteboard/whiteboard_errors_test.go b/shortcuts/whiteboard/whiteboard_errors_test.go new file mode 100644 index 000000000..da1955fec --- /dev/null +++ b/shortcuts/whiteboard/whiteboard_errors_test.go @@ -0,0 +1,45 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package whiteboard + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +func TestWrapWbNetworkErr(t *testing.T) { + typed := errs.NewAPIError(errs.SubtypeRateLimit, "already typed") + if got := wrapWbNetworkErr(typed, "wrap"); got != error(typed) { + t.Fatalf("typed error must pass through unchanged, got %v", got) + } + + raw := errors.New("connection reset by peer") + got := wrapWbNetworkErr(raw, "fetch failed: %v", raw) + var ne *errs.NetworkError + if !errors.As(got, &ne) || ne.Subtype != errs.SubtypeNetworkTransport { + t.Fatalf("raw error: got %T (%v)", got, got) + } +} + +func TestWbSaveError(t *testing.T) { + if wbSaveError(nil) != nil { + t.Fatal("nil error should map to nil") + } + + var ve *errs.ValidationError + if got := wbSaveError(&fileio.PathValidationError{Err: errors.New("escape")}); !errors.As(got, &ve) || ve.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("path validation: got %T (%v)", got, got) + } + + var ie *errs.InternalError + if got := wbSaveError(&fileio.MkdirError{Err: errors.New("mkdir denied")}); !errors.As(got, &ie) || ie.Subtype != errs.SubtypeFileIO { + t.Fatalf("mkdir: got %T (%v)", got, got) + } + if got := wbSaveError(errors.New("disk full")); !errors.As(got, &ie) || ie.Subtype != errs.SubtypeFileIO { + t.Fatalf("default: got %T (%v)", got, got) + } +} diff --git a/shortcuts/whiteboard/whiteboard_query.go b/shortcuts/whiteboard/whiteboard_query.go index 44b2c1fcc..8e6169055 100644 --- a/shortcuts/whiteboard/whiteboard_query.go +++ b/shortcuts/whiteboard/whiteboard_query.go @@ -14,8 +14,8 @@ import ( "path/filepath" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" larkcore "github.com/larksuite/oapi-sdk-go/v3/core" @@ -79,16 +79,16 @@ var WhiteboardQuery = common.Shortcut{ out := runtime.Str("output") if out != "" { if err := runtime.ValidatePath(out); err != nil { - return output.ErrValidation("invalid output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid output path: %s", err).WithParam("--output") } } if out == "" && runtime.Str("output_as") == WhiteboardQueryAsImage { - return output.ErrValidation("need a output directory to query whiteboard as image") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "need a output directory to query whiteboard as image").WithParam("--output") } as := runtime.Str("output_as") if as != WhiteboardQueryAsImage && as != WhiteboardQueryAsCode && as != WhiteboardQueryAsRaw { - return common.FlagErrorf("--output_as flag must be one of: image | code | raw") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--output_as flag must be one of: image | code | raw").WithParam("--output_as") } return nil }, @@ -125,7 +125,7 @@ var WhiteboardQuery = common.Shortcut{ case WhiteboardQueryAsRaw: return exportWhiteboardRaw(runtime, token, outDir) default: - return output.ErrValidation("--as flag must be one of: image | code | raw") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--as flag must be one of: image | code | raw").WithParam("--output_as") } }, @@ -136,14 +136,19 @@ func exportWhiteboardPreview(ctx context.Context, runtime *common.RuntimeContext HttpMethod: http.MethodGet, ApiPath: fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/download_as_image", url.PathEscape(wbToken)), } - // Execute API request + // Execute API request. The preview endpoint streams raw image bytes (not a + // JSON envelope), so classify by HTTP status: 5xx is a retryable server + // error, any other non-2xx a transport error. resp, err := runtime.DoAPI(req, larkcore.WithFileDownload()) if err != nil { - return output.ErrNetwork(fmt.Sprintf("get whiteboard preview failed: %v", err)) + return wrapWbNetworkErr(err, "get whiteboard preview failed: %v", err) } - // Check response status code - if resp.StatusCode != http.StatusOK { - return output.ErrAPI(resp.StatusCode, string(resp.RawBody), nil) + if resp.StatusCode >= 400 { + subtype := errs.SubtypeNetworkTransport + if resp.StatusCode >= 500 { + subtype = errs.SubtypeNetworkServer + } + return errs.NewNetworkError(subtype, "get whiteboard preview failed: HTTP %d: %s", resp.StatusCode, string(resp.RawBody)).WithCode(resp.StatusCode) } finalPath, size, err := saveOutputFile(outDir, ".png", wbToken, runtime, bytes.NewReader(resp.RawBody)) @@ -162,34 +167,18 @@ func exportWhiteboardPreview(ctx context.Context, runtime *common.RuntimeContext } type wbNodesResp struct { - Code int `json:"code"` - Msg string `json:"msg"` Data struct { Nodes []interface{} `json:"nodes"` } `json:"data"` } func fetchWhiteboardNodes(runtime *common.RuntimeContext, wbToken string) (*wbNodesResp, error) { - req := &larkcore.ApiReq{ - HttpMethod: http.MethodGet, - ApiPath: fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes", wbToken), - } - resp, err := runtime.DoAPI(req) + data, err := runtime.CallAPITyped(http.MethodGet, fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes", url.PathEscape(wbToken)), nil, nil) if err != nil { - return nil, output.ErrNetwork(fmt.Sprintf("get whiteboard nodes failed: %v", err)) - } - // 检查响应状态码 - if resp.StatusCode != http.StatusOK { - return nil, output.ErrAPI(resp.StatusCode, string(resp.RawBody), nil) + return nil, err } var nodes wbNodesResp - err = json.Unmarshal(resp.RawBody, &nodes) - if err != nil { - return nil, output.Errorf(output.ExitInternal, "parsing", fmt.Sprintf("parse whiteboard nodes failed: %v", err)) - } - if nodes.Code != 0 { - return nil, output.ErrAPI(nodes.Code, "get whiteboard nodes failed", fmt.Sprintf("get whiteboard nodes failed: %s", nodes.Msg)) - } + nodes.Data.Nodes, _ = data["nodes"].([]interface{}) return &nodes, nil } @@ -229,6 +218,12 @@ func exportWhiteboardCode(runtime *common.RuntimeContext, wbToken, outDir string code, _ := syntaxMap["code"].(string) var syntaxType SyntaxType switch v := syntaxMap["syntax_type"].(type) { + case json.Number: + // runtime.ClassifyAPIResponse decodes the response with UseNumber, + // so numeric fields arrive as json.Number rather than float64. + if n, err := v.Int64(); err == nil { + syntaxType = SyntaxType(n) + } case float64: syntaxType = SyntaxType(v) case SyntaxType: @@ -299,7 +294,7 @@ func exportWhiteboardRaw(runtime *common.RuntimeContext, wbToken, outDir string) jsonData, err := json.MarshalIndent(wbNodes.Data, "", " ") if err != nil { - return output.Errorf(output.ExitInternal, "json_error", "cannot marshal whiteboard data: %s", err) + return errs.NewInternalError(errs.SubtypeUnknown, "cannot marshal whiteboard data: %s", err) } if outDir == "" { @@ -348,10 +343,10 @@ func saveOutputFile(outPath, ext, token string, runtime *common.RuntimeContext, _, err = runtime.FileIO().Stat(finalPath) if err == nil { if !runtime.Bool("overwrite") { - return "", 0, output.ErrValidation(fmt.Sprintf("file already exists: %s (use --overwrite to overwrite)", finalPath)) + return "", 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "file already exists: %s (use --overwrite to overwrite)", finalPath).WithParam("--overwrite") } } else if !os.IsNotExist(err) { - return "", 0, output.Errorf(output.ExitInternal, "io_error", "cannot check file existence: %s", err) + return "", 0, errs.NewInternalError(errs.SubtypeFileIO, "cannot check file existence: %s", err) } // Step 3: Save file @@ -369,7 +364,7 @@ func saveOutputFile(outPath, ext, token string, runtime *common.RuntimeContext, ContentType: contentType, }, data) if err != nil { - return "", 0, common.WrapSaveError(err, "unsafe file path", "cannot create parent directory", "cannot create file") + return "", 0, wbSaveError(err) } return finalPath, savResult.Size(), nil diff --git a/shortcuts/whiteboard/whiteboard_query_test.go b/shortcuts/whiteboard/whiteboard_query_test.go index 94ed9c063..c29509526 100644 --- a/shortcuts/whiteboard/whiteboard_query_test.go +++ b/shortcuts/whiteboard/whiteboard_query_test.go @@ -6,11 +6,13 @@ package whiteboard import ( "bytes" "context" + "errors" "os" "path/filepath" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -149,6 +151,82 @@ func TestWhiteboardQuery_Validate(t *testing.T) { } } +// TestWhiteboardQuery_Validate_TypedErrors locks the typed-envelope contract: +// input-validation failures surface as *errs.ValidationError carrying +// SubtypeInvalidArgument and the offending --flag, readable via errs.ProblemOf +// and errors.As — the shape downstream consumers (and exit-code mapping) rely on. +func TestWhiteboardQuery_Validate_TypedErrors(t *testing.T) { + ctx := context.Background() + chdirTemp(t) + + tests := []struct { + name string + flags map[string]string + wantParam string + }{ + { + name: "image without output", + flags: map[string]string{"whiteboard-token": "t", "output_as": "image"}, + wantParam: "--output", + }, + { + name: "bad output_as value", + flags: map[string]string{"whiteboard-token": "t", "output_as": "invalid"}, + wantParam: "--output_as", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := WhiteboardQuery.Validate(ctx, newTestRuntime(tt.flags, nil)) + if err == nil { + t.Fatalf("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error is not *errs.ValidationError: %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != tt.wantParam { + t.Errorf("Param = %q, want %q", ve.Param, tt.wantParam) + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("errs.ProblemOf returned false") + } + if p.Category != errs.CategoryValidation { + t.Errorf("Category = %q, want %q", p.Category, errs.CategoryValidation) + } + }) + } +} + +// TestExportWhiteboardPreview_HTTPError locks the download-path failure +// behavior: a failed preview download surfaces as a typed errs.* envelope, not +// a flat legacy error. +func TestExportWhiteboardPreview_HTTPError(t *testing.T) { + factory, stdout, reg := newExecuteFactory(t) + chdirTemp(t) + + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/board/v1/whiteboards/test-token-preview-5xx/download_as_image", + Status: 500, + RawBody: []byte("gateway error"), + }) + + args := []string{"+query", "--whiteboard-token", "test-token-preview-5xx", "--output_as", "image", "--output", "output"} + err := runShortcut(t, WhiteboardQuery, args, factory, stdout) + if err == nil { + t.Fatal("expected error for HTTP 500 download") + } + if _, ok := errs.ProblemOf(err); !ok { + t.Fatalf("error is not a typed errs.* envelope: %T (%v)", err, err) + } +} + func TestWhiteboardQuery_DryRun(t *testing.T) { t.Parallel() @@ -705,10 +783,18 @@ func TestFetchWhiteboardNodes_APIError(t *testing.T) { args := []string{"+query", "--whiteboard-token", "test-token-api-error", "--output_as", "raw"} err := runShortcut(t, WhiteboardQuery, args, factory, stdout) - // We expect an error here, but don't fail the test because it's testing error path if err == nil { t.Fatalf("Expected API error, but got none") } + // The nodes fetch now classifies the Lark error code into a typed envelope + // carrying the numeric code. + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error is not a typed errs.* envelope: %T (%v)", err, err) + } + if p.Code != 10001 { + t.Errorf("Problem.Code = %d, want 10001", p.Code) + } } // newTestRuntime creates a RuntimeContext with string flags for testing. diff --git a/shortcuts/whiteboard/whiteboard_update.go b/shortcuts/whiteboard/whiteboard_update.go index 0ffe6fde2..3fb51c669 100644 --- a/shortcuts/whiteboard/whiteboard_update.go +++ b/shortcuts/whiteboard/whiteboard_update.go @@ -12,10 +12,9 @@ import ( "net/url" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" - larkcore "github.com/larksuite/oapi-sdk-go/v3/core" ) const ( @@ -50,13 +49,13 @@ func wbUpdateValidate(ctx context.Context, runtime *common.RuntimeContext) error return err } if itoken != "" && len(itoken) < 10 { - return common.FlagErrorf("--idempotent-token must be at least 10 characters long.") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--idempotent-token must be at least 10 characters long.").WithParam("--idempotent-token") } // 检查 --input_format 标志 format := getFormat(runtime) if format != FormatRaw && format != FormatPlantUML && format != FormatMermaid { - return common.FlagErrorf("--input_format must be one of: raw | plantuml | mermaid") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--input_format must be one of: raw | plantuml | mermaid").WithParam("--input_format") } return nil } @@ -116,7 +115,7 @@ func wbUpdateExecute(ctx context.Context, runtime *common.RuntimeContext) error input := runtime.Str("source") if input == "" { - return output.ErrValidation("read input failed: source is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "read input failed: source is required").WithParam("--source") } switch format { @@ -125,7 +124,7 @@ func wbUpdateExecute(ctx context.Context, runtime *common.RuntimeContext) error case FormatPlantUML, FormatMermaid: return updateWhiteboardByCode(ctx, runtime, token, []byte(input), format, overwrite, idempotentToken) default: - return output.ErrValidation(fmt.Sprintf("unsupported format: %s", format)) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported format: %s", format).WithParam("--input_format") } } @@ -160,15 +159,6 @@ var WhiteboardUpdateOld = common.Shortcut{ Execute: wbUpdateExecute, } -type createResponse struct { - Code int `json:"code"` - Msg string `json:"msg"` - Data struct { - NodeIDs []string `json:"ids"` - IdempotentToken string `json:"client_token"` - } `json:"data"` -} - type plantumlCreateReq struct { PlantUmlCode string `json:"plant_uml_code"` SyntaxType int `json:"syntax_type"` @@ -182,28 +172,20 @@ type rawNodesCreateReq struct { Overwrite bool `json:"overwrite,omitempty"` } -type plantumlCreateResp struct { - Code int `json:"code"` - Msg string `json:"msg"` - Data struct { - NodeID string `json:"node_id"` - } `json:"data"` -} - func parseWBcliNodes(rawjson []byte) (wbNodes []interface{}, err error, isRaw bool) { var wbOutput WbCliOutput if err := json.Unmarshal(rawjson, &wbOutput); err != nil { - return nil, output.Errorf(output.ExitValidation, "parsing", fmt.Sprintf("unmarshal input json failed: %v", err)), false + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "unmarshal input json failed: %v", err).WithParam("--source"), false } if (wbOutput.Code != 0 || wbOutput.Data.To != "openapi") && wbOutput.RawNodes == nil { - return nil, output.Errorf(output.ExitValidation, "whiteboard-cli", "whiteboard-cli failed. please check previous log."), false + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "whiteboard-cli failed. please check previous log.").WithParam("--source"), false } if wbOutput.RawNodes != nil { wbNodes = wbOutput.RawNodes isRaw = true } else { if wbOutput.Data.Result.Nodes == nil { - return nil, output.Errorf(output.ExitValidation, "whiteboard-cli", "whiteboard-cli failed. please check previous log."), false + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "whiteboard-cli failed. please check previous log.").WithParam("--source"), false } wbNodes = wbOutput.Data.Result.Nodes } @@ -221,35 +203,18 @@ func updateWhiteboardByCode(ctx context.Context, runtime *common.RuntimeContext, Overwrite: overwrite, } - req := &larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes/plantuml", url.PathEscape(wbToken)), - Body: reqBody, - QueryParams: map[string][]string{}, - } + params := map[string]interface{}{} if idempotentToken != "" { - req.QueryParams["client_token"] = []string{idempotentToken} + params["client_token"] = idempotentToken } - resp, err := runtime.DoAPI(req) + data, err := runtime.CallAPITyped(http.MethodPost, fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes/plantuml", url.PathEscape(wbToken)), params, reqBody) if err != nil { - return output.ErrNetwork(fmt.Sprintf("update whiteboard by code failed: %v", err)) - } - if resp.StatusCode != http.StatusOK { - return output.ErrAPI(resp.StatusCode, string(resp.RawBody), nil) - } - - var createResp plantumlCreateResp - err = json.Unmarshal(resp.RawBody, &createResp) - if err != nil { - return output.Errorf(output.ExitInternal, "parsing", fmt.Sprintf("parse whiteboard create response failed: %v", err)) - } - if createResp.Code != 0 { - return output.ErrAPI(createResp.Code, "update whiteboard by code failed", fmt.Sprintf("update whiteboard by code failed: %s", createResp.Msg)) + return err } outData := make(map[string]string) - outData["created_node_id"] = createResp.Data.NodeID + outData["created_node_id"] = common.GetString(data, "node_id") runtime.OutFormat(outData, nil, func(w io.Writer) { if outData["created_node_id"] != "" { fmt.Fprintf(w, "New node created.\n") @@ -266,56 +231,59 @@ func updateWhiteboardByRawNodes(ctx context.Context, runtime *common.RuntimeCont if err != nil { return err } - outData := make(map[string]string) reqBody := rawNodesCreateReq{ Nodes: nodes, Overwrite: overwrite, } - req := &larkcore.ApiReq{ - HttpMethod: http.MethodPost, - ApiPath: fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes", url.PathEscape(wbToken)), - Body: reqBody, - QueryParams: map[string][]string{}, - } + params := map[string]interface{}{} if idempotentToken != "" { - req.QueryParams["client_token"] = []string{idempotentToken} + params["client_token"] = idempotentToken } - resp, err := runtime.DoAPI(req) + data, err := runtime.CallAPITyped(http.MethodPost, fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/nodes", url.PathEscape(wbToken)), params, reqBody) if err != nil { - return output.ErrNetwork(fmt.Sprintf("update whiteboard failed: %v", err)) - } - if resp.StatusCode != http.StatusOK { - var detail string - if isRaw { - detail = fmt.Sprintf("It is not advised to edit openapi format json directly. Please follow instruction in lark-whiteboard skill, " + - "using whiteboard-cli to transcript Whiteboard DSL pattern instead.") - } - return output.ErrAPI(resp.StatusCode, string(resp.RawBody), detail) - } - - var createResp createResponse - err = json.Unmarshal(resp.RawBody, &createResp) - if err != nil { - return output.Errorf(output.ExitInternal, "parsing", fmt.Sprintf("parse whiteboard create response failed: %v", err)) - } - if createResp.Code != 0 { - detail := fmt.Sprintf("update whiteboard failed: %s", createResp.Msg) + // Raw open-api JSON is hand-edited far more often than the DSL path, so + // steer the user back to the recommended workflow on any API failure. if isRaw { - detail += fmt.Sprintf("\n It is not advised to edit openapi format json directly. Please follow instruction in lark-whiteboard skill, " + - "using whiteboard-cli to transcript Whiteboard DSL pattern instead.") + if p, ok := errs.ProblemOf(err); ok { + rawHint := "It is not advised to edit openapi format json directly. " + + "Please follow instruction in lark-whiteboard skill, using whiteboard-cli " + + "to transcript Whiteboard DSL pattern instead." + if strings.TrimSpace(p.Hint) != "" { + p.Hint = p.Hint + "\n" + rawHint + } else { + p.Hint = rawHint + } + } } - return output.ErrAPI(createResp.Code, "update whiteboard failed", detail) + return err } - outData["created_node_ids"] = strings.Join(createResp.Data.NodeIDs, ",") + nodeIDs := stringSlice(data["ids"]) + outData := map[string]string{"created_node_ids": strings.Join(nodeIDs, ",")} runtime.OutFormat(outData, nil, func(w io.Writer) { if outData["created_node_ids"] != "" { - fmt.Fprintf(w, "%d new nodes created.\n", len(createResp.Data.NodeIDs)) + fmt.Fprintf(w, "%d new nodes created.\n", len(nodeIDs)) } fmt.Fprintf(w, "Update whiteboard success") }) return nil } + +// stringSlice coerces a JSON array value (decoded as []interface{}) into a +// []string, dropping non-string elements. +func stringSlice(v interface{}) []string { + raw, ok := v.([]interface{}) + if !ok { + return nil + } + out := make([]string, 0, len(raw)) + for _, e := range raw { + if s, ok := e.(string); ok { + out = append(out, s) + } + } + return out +} diff --git a/shortcuts/whiteboard/whiteboard_update_test.go b/shortcuts/whiteboard/whiteboard_update_test.go index 2cf27bcd3..0cea2e3e6 100644 --- a/shortcuts/whiteboard/whiteboard_update_test.go +++ b/shortcuts/whiteboard/whiteboard_update_test.go @@ -6,9 +6,11 @@ package whiteboard import ( "bytes" "context" + "errors" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -102,6 +104,54 @@ func TestWhiteboardUpdate_Validate(t *testing.T) { } } +// TestWhiteboardUpdate_Validate_TypedErrors locks the typed-envelope contract +// for +update input validation: failures are *errs.ValidationError with +// SubtypeInvalidArgument and the offending --flag. parseWBcliNodes likewise +// reports malformed --source input as a typed validation error. +func TestWhiteboardUpdate_Validate_TypedErrors(t *testing.T) { + ctx := context.Background() + + t.Run("idempotent-token too short", func(t *testing.T) { + rt := newTestRuntime(map[string]string{ + "whiteboard-token": "t", + "idempotent-token": "short", + "source": "{}", + }, nil) + assertValidationParam(t, wbUpdateValidate(ctx, rt), "--idempotent-token") + }) + + t.Run("bad input_format", func(t *testing.T) { + rt := newTestRuntime(map[string]string{ + "whiteboard-token": "t", + "input_format": "svg", + "source": "{}", + }, nil) + assertValidationParam(t, wbUpdateValidate(ctx, rt), "--input_format") + }) + + t.Run("malformed source json", func(t *testing.T) { + _, err, _ := parseWBcliNodes([]byte("not-json")) + assertValidationParam(t, err, "--source") + }) +} + +func assertValidationParam(t *testing.T, err error, wantParam string) { + t.Helper() + if err == nil { + t.Fatalf("expected error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error is not *errs.ValidationError: %T", err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != wantParam { + t.Errorf("Param = %q, want %q", ve.Param, wantParam) + } +} + func TestGetFormat(t *testing.T) { t.Parallel() @@ -447,9 +497,16 @@ func TestWhiteboardUpdateExecute_RawAPIError(t *testing.T) { source := `{"code":0,"data":{"to":"openapi","result":{"nodes":[]}}}` args := []string{"+update", "--whiteboard-token", "test-token-raw-api-error", "--input_format", "raw", "--source", source} err := runUpdateShortcut(t, WhiteboardUpdate, args, factory, stdout) - // We expect an error here, but don't fail the test because it's testing error path if err == nil { - t.Logf("Expected API error, but got none") + t.Fatalf("expected API error, but got none") + } + // The update boundary now yields a typed envelope carrying the Lark code. + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error is not a typed errs.* envelope: %T (%v)", err, err) + } + if p.Code != 10001 { + t.Errorf("Problem.Code = %d, want 10001", p.Code) } } @@ -471,9 +528,15 @@ invalid @@enduml` args := []string{"+update", "--whiteboard-token", "test-token-plantuml-error", "--input_format", "plantuml", "--source", source} err := runUpdateShortcut(t, WhiteboardUpdate, args, factory, stdout) - // We expect an error here, but don't fail the test because it's testing error path if err == nil { - t.Logf("Expected API error, but got none") + t.Fatalf("expected API error, but got none") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error is not a typed errs.* envelope: %T (%v)", err, err) + } + if p.Code != 10001 { + t.Errorf("Problem.Code = %d, want 10001", p.Code) } }