From fc3f19771ee547e73883e1ea39dbcd1bbd76842a Mon Sep 17 00:00:00 2001 From: "zhaoyukun.yk" Date: Tue, 9 Jun 2026 14:59:59 +0800 Subject: [PATCH] feat(sheets): emit typed error envelopes across the sheets domain Emit structured validation, API, network, file, and internal error envelopes for Sheets shortcuts so users and agents can recover from failed spreadsheet workflows using stable type, subtype, param, and code fields. Add Sheets domain errscontract and golangci guards to prevent legacy envelope and common helper regressions. --- .golangci.yml | 6 +- .../rule_no_legacy_common_helper_call.go | 1 + .../rule_no_legacy_envelope_literal.go | 1 + lint/errscontract/rules_test.go | 18 +++ shortcuts/sheets/backward/helpers.go | 14 +- .../sheets/backward/lark_sheets_cell_data.go | 21 +-- .../backward/lark_sheets_cell_images.go | 24 +-- .../lark_sheets_cell_style_and_merge.go | 43 +++--- .../sheets/backward/lark_sheets_dropdown.go | 27 ++-- .../backward/lark_sheets_filter_views.go | 30 ++-- .../backward/lark_sheets_float_images.go | 36 +++-- .../lark_sheets_row_column_management.go | 35 +++-- .../backward/lark_sheets_sheet_export_test.go | 17 ++ .../backward/lark_sheets_sheet_manage_test.go | 45 ++---- .../backward/lark_sheets_sheet_management.go | 137 ++++++---------- .../lark_sheets_spreadsheet_management.go | 48 +++--- shortcuts/sheets/backward/sheets_errors.go | 15 ++ shortcuts/sheets/batch_op_dispatch.go | 39 ++--- shortcuts/sheets/csv_put_guard_test.go | 12 ++ shortcuts/sheets/csv_put_range_alias_test.go | 16 +- shortcuts/sheets/execute_paths_test.go | 28 +++- shortcuts/sheets/flag_schema_validate.go | 4 +- shortcuts/sheets/helpers.go | 91 ++++++++--- shortcuts/sheets/helpers_test.go | 67 ++++++++ shortcuts/sheets/lark_sheet_batch_update.go | 20 +-- shortcuts/sheets/lark_sheet_object_crud.go | 62 +++++--- .../sheets/lark_sheet_range_operations.go | 45 +++--- .../lark_sheet_range_operations_test.go | 43 +++--- shortcuts/sheets/lark_sheet_read_data.go | 6 +- shortcuts/sheets/lark_sheet_search_replace.go | 6 +- .../sheets/lark_sheet_sheet_structure.go | 28 ++-- shortcuts/sheets/lark_sheet_workbook.go | 146 +++++++++++------- shortcuts/sheets/lark_sheet_workbook_test.go | 91 +++++++++++ shortcuts/sheets/lark_sheet_write_cells.go | 87 +++++++---- shortcuts/sheets/sheet_ai_api.go | 13 +- shortcuts/sheets/validation_params_test.go | 104 +++++++++++++ 36 files changed, 943 insertions(+), 483 deletions(-) create mode 100644 shortcuts/sheets/backward/sheets_errors.go create mode 100644 shortcuts/sheets/validation_params_test.go diff --git a/.golangci.yml b/.golangci.yml index 2e52c66698..9428d7a2bb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -73,20 +73,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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) + - 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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) 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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper enforced on domains whose shared validation/save # helpers have migrated to typed final errors. - - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/lint/errscontract/rule_no_legacy_common_helper_call.go b/lint/errscontract/rule_no_legacy_common_helper_call.go index a1e404ff65..742b506dd3 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -27,6 +27,7 @@ var migratedCommonHelperPaths = []string{ "shortcuts/mail/", "shortcuts/minutes/", "shortcuts/okr/", + "shortcuts/sheets/", "shortcuts/task/", "shortcuts/vc/", "shortcuts/whiteboard/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index e75bb3fca9..e7bba084cf 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -28,6 +28,7 @@ var migratedEnvelopePaths = []string{ "shortcuts/mail/", "shortcuts/minutes/", "shortcuts/okr/", + "shortcuts/sheets/", "shortcuts/task/", "shortcuts/vc/", "shortcuts/whiteboard/", diff --git a/lint/errscontract/rules_test.go b/lint/errscontract/rules_test.go index 410bb6f8e0..22a7b396cf 100644 --- a/lint/errscontract/rules_test.go +++ b/lint/errscontract/rules_test.go @@ -954,6 +954,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes "shortcuts/drive/drive_search.go", "shortcuts/mail/mail_send.go", "shortcuts/okr/okr_progress_create.go", + "shortcuts/sheets/helpers.go", "shortcuts/task/task_update.go", "shortcuts/whiteboard/whiteboard_query.go", } @@ -1021,6 +1022,23 @@ func boom() { } } +func TestCheckNoLegacyCommonHelperCall_CoversSheetsPathWithAliasAndFunctionValue(t *testing.T) { + src := `package migrated + +import c "github.com/larksuite/cli/shortcuts/common" + +func boom() { + f := c.FlagErrorf + _ = f + c.WrapInputStatError(nil) +} +` + v := CheckNoLegacyCommonHelperCall("shortcuts/sheets/helpers.go", src) + if len(v) != 2 { + t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on sheets path, got %d: %+v", len(v), v) + } +} + func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) { src := `package contact diff --git a/shortcuts/sheets/backward/helpers.go b/shortcuts/sheets/backward/helpers.go index 9c8f3284c3..46fdc14fe9 100644 --- a/shortcuts/sheets/backward/helpers.go +++ b/shortcuts/sheets/backward/helpers.go @@ -9,7 +9,7 @@ import ( "strconv" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -27,7 +27,7 @@ var sheetRangeSeparatorReplacer = strings.NewReplacer(`\！`, "!", `\!`, "!", " // getFirstSheetID queries the spreadsheet and returns the first sheet's ID. func getFirstSheetID(runtime *common.RuntimeContext, spreadsheetToken string) (string, error) { - data, err := runtime.CallAPI("GET", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/query", validate.EncodePathSegment(spreadsheetToken)), nil, nil) + data, err := runtime.CallAPITyped("GET", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/query", validate.EncodePathSegment(spreadsheetToken)), nil, nil) if err != nil { return "", err } @@ -38,7 +38,7 @@ func getFirstSheetID(runtime *common.RuntimeContext, spreadsheetToken string) (s return id, nil } } - return "", output.Errorf(output.ExitAPI, "not_found", "no sheets found in this spreadsheet") + return "", errs.NewValidationError(errs.SubtypeFailedPrecondition, "no sheets found in this spreadsheet") } // extractSpreadsheetToken extracts spreadsheet token from URL. @@ -104,7 +104,7 @@ func validateSheetRangeInput(sheetID, input string) error { return nil } if looksLikeRelativeRange(input) { - return common.FlagErrorf("--range %q requires --sheet-id or a ! prefix", input) + return common.ValidationErrorf("--range %q requires --sheet-id or a ! prefix", input).WithParam("--range") } return nil } @@ -127,7 +127,7 @@ func validateSingleCellRange(input string) error { if strings.EqualFold(parts[0], parts[1]) { return nil } - return common.FlagErrorf("--range %q must be a single cell (e.g. A1 or A1:A1), got a multi-cell span", input) + return common.ValidationErrorf("--range %q must be a single cell (e.g. A1 or A1:A1), got a multi-cell span", input).WithParam("--range") } return nil } @@ -197,11 +197,11 @@ func matrixDimensions(values interface{}) (rows, cols int) { func offsetCell(cell string, rowOffset, colOffset int) (string, error) { matches := cellRefPattern.FindStringSubmatch(strings.TrimSpace(cell)) if len(matches) != 3 { - return "", fmt.Errorf("invalid cell reference: %s", cell) + return "", fmt.Errorf("invalid cell reference: %s", cell) //nolint:forbidigo // intermediate sentinel; sole caller buildRectRange discards it and falls back } colIndex := columnNameToIndex(matches[1]) if colIndex < 1 { - return "", fmt.Errorf("invalid column: %s", matches[1]) + return "", fmt.Errorf("invalid column: %s", matches[1]) //nolint:forbidigo // intermediate sentinel; sole caller buildRectRange discards it and falls back } rowIndex, err := strconv.Atoi(matches[2]) if err != nil { diff --git a/shortcuts/sheets/backward/lark_sheets_cell_data.go b/shortcuts/sheets/backward/lark_sheets_cell_data.go index 59d6fa74e9..77a1571be2 100644 --- a/shortcuts/sheets/backward/lark_sheets_cell_data.go +++ b/shortcuts/sheets/backward/lark_sheets_cell_data.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -15,10 +16,10 @@ import ( func parseValues2DJSON(raw string) ([][]interface{}, error) { var rows [][]interface{} if err := json.Unmarshal([]byte(raw), &rows); err != nil { - return nil, common.FlagErrorf("--values invalid JSON, must be a 2D array") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--values invalid JSON, must be a 2D array").WithParam("--values") } if rows == nil { - return nil, common.FlagErrorf("--values invalid JSON, must be a 2D array") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--values invalid JSON, must be a 2D array").WithParam("--values") } return rows, nil } @@ -46,7 +47,7 @@ var SheetRead = common.Shortcut{ } if r := runtime.Str("range"); r != "" { if rangeSheetID, _, ok := splitSheetRange(r); ok && runtime.Str("sheet-id") != "" && rangeSheetID != runtime.Str("sheet-id") { - return common.FlagErrorf("--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")).WithParam("--range") } } return nil @@ -90,7 +91,7 @@ var SheetRead = common.Shortcut{ params["valueRenderOption"] = renderOption } - data, err := runtime.CallAPI("GET", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values/%s", validate.EncodePathSegment(token), validate.EncodePathSegment(readRange)), params, nil) + data, err := runtime.CallAPITyped("GET", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values/%s", validate.EncodePathSegment(token), validate.EncodePathSegment(readRange)), params, nil) if err != nil { return err } @@ -167,7 +168,7 @@ var SheetWrite = common.Shortcut{ writeRange = normalizeWriteRange(runtime.Str("sheet-id"), writeRange, values) } - data, err := runtime.CallAPI("PUT", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values", validate.EncodePathSegment(token)), nil, map[string]interface{}{ + data, err := runtime.CallAPITyped("PUT", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values", validate.EncodePathSegment(token)), nil, map[string]interface{}{ "valueRange": map[string]interface{}{ "range": writeRange, "values": values, @@ -247,7 +248,7 @@ var SheetAppend = common.Shortcut{ appendRange = normalizePointRange(runtime.Str("sheet-id"), appendRange) } - data, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_append", validate.EncodePathSegment(token)), nil, map[string]interface{}{ + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_append", validate.EncodePathSegment(token)), nil, map[string]interface{}{ "valueRange": map[string]interface{}{ "range": appendRange, "values": values, @@ -288,7 +289,7 @@ var SheetFind = common.Shortcut{ } if r := runtime.Str("range"); r != "" { if rangeSheetID, _, ok := splitSheetRange(r); ok && runtime.Str("sheet-id") != "" && rangeSheetID != runtime.Str("sheet-id") { - return common.FlagErrorf("--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")).WithParam("--range") } } return nil @@ -336,7 +337,7 @@ var SheetFind = common.Shortcut{ "find": findText, } - data, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/find", validate.EncodePathSegment(token), validate.EncodePathSegment(sheetID)), nil, reqData) + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/find", validate.EncodePathSegment(token), validate.EncodePathSegment(sheetID)), nil, reqData) if err != nil { return err } @@ -373,7 +374,7 @@ var SheetReplace = common.Shortcut{ } if r := runtime.Str("range"); r != "" { if rangeSheetID, _, ok := splitSheetRange(r); ok && runtime.Str("sheet-id") != "" && rangeSheetID != runtime.Str("sheet-id") { - return common.FlagErrorf("--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range sheet ID %q does not match --sheet-id %q", rangeSheetID, runtime.Str("sheet-id")).WithParam("--range") } } return nil @@ -415,7 +416,7 @@ var SheetReplace = common.Shortcut{ findCondition["range"] = normalizeSheetRange(sheetID, runtime.Str("range")) } - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/replace", validate.EncodePathSegment(token), validate.EncodePathSegment(sheetID), diff --git a/shortcuts/sheets/backward/lark_sheets_cell_images.go b/shortcuts/sheets/backward/lark_sheets_cell_images.go index d2c0af692f..6f1a3cb566 100644 --- a/shortcuts/sheets/backward/lark_sheets_cell_images.go +++ b/shortcuts/sheets/backward/lark_sheets_cell_images.go @@ -11,8 +11,8 @@ import ( "os" "path/filepath" + "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" ) @@ -38,7 +38,7 @@ var SheetWriteImage = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } if err := validateSheetRangeInput(runtime.Str("sheet-id"), runtime.Str("range")); err != nil { return err @@ -91,7 +91,7 @@ var SheetWriteImage = common.Shortcut{ imageBytes, err := io.ReadAll(imageFile) if err != nil { - return output.ErrValidation("cannot read image file: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read image file: %s", err).WithParam("--image").WithCause(err) } imageName := runtime.Str("name") @@ -101,7 +101,7 @@ var SheetWriteImage = common.Shortcut{ fmt.Fprintf(runtime.IO().ErrOut, "Writing image: %s (%d bytes) → %s\n", imageName, stat.Size(), pointRange) - data, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_image", validate.EncodePathSegment(token)), nil, map[string]interface{}{ + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_image", validate.EncodePathSegment(token)), nil, map[string]interface{}{ "range": pointRange, "image": imageBytes, "name": imageName, @@ -116,35 +116,35 @@ var SheetWriteImage = common.Shortcut{ func validateSheetWriteImageFile(fio fileio.FileIO, imagePath string) (fileio.FileInfo, error) { if fio == nil { - return nil, output.ErrValidation("no file I/O provider registered") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "no file I/O provider registered") } stat, err := fio.Stat(imagePath) if err != nil { return nil, wrapSheetWriteImageStatError(err, imagePath) } if stat.IsDir() || !stat.Mode().IsRegular() { - return nil, output.ErrValidation("image must be a regular file: %s", imagePath) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "image must be a regular file: %s", imagePath).WithParam("--image") } const maxImageSize int64 = 20 * 1024 * 1024 if stat.Size() > maxImageSize { - return nil, output.ErrValidation("image %.1fMB exceeds 20MB limit", float64(stat.Size())/1024/1024) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "image %.1fMB exceeds 20MB limit", float64(stat.Size())/1024/1024).WithParam("--image") } return stat, nil } func wrapSheetWriteImageStatError(err error, imagePath string) error { if errors.Is(err, fileio.ErrPathValidation) { - return output.ErrValidation("unsafe image path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe image path: %s", err).WithParam("--image").WithCause(err) } if os.IsNotExist(err) { - return output.ErrValidation("image file not found: %s", imagePath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "image file not found: %s", imagePath).WithParam("--image").WithCause(err) } - return output.ErrValidation("cannot stat image file: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot stat image file: %s", err).WithParam("--image").WithCause(err) } func wrapSheetWriteImageOpenError(err error) error { if errors.Is(err, fileio.ErrPathValidation) { - return output.ErrValidation("unsafe image path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe image path: %s", err).WithParam("--image").WithCause(err) } - return output.ErrValidation("cannot read image file: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read image file: %s", err).WithParam("--image").WithCause(err) } diff --git a/shortcuts/sheets/backward/lark_sheets_cell_style_and_merge.go b/shortcuts/sheets/backward/lark_sheets_cell_style_and_merge.go index 520f91e367..30622c052c 100644 --- a/shortcuts/sheets/backward/lark_sheets_cell_style_and_merge.go +++ b/shortcuts/sheets/backward/lark_sheets_cell_style_and_merge.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -15,40 +16,40 @@ import ( func validateBatchStyleData(raw string) error { var data interface{} if err := json.Unmarshal([]byte(raw), &data); err != nil { - return common.FlagErrorf("--data must be valid JSON: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be valid JSON: %v", err).WithParam("--data") } arr, ok := data.([]interface{}) if !ok || len(arr) == 0 { - return common.FlagErrorf("--data must be a non-empty JSON array") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be a non-empty JSON array").WithParam("--data") } for i, item := range arr { entry, ok := item.(map[string]interface{}) if !ok { - return common.FlagErrorf("--data[%d] must be an object with ranges and style", i) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d] must be an object with ranges and style", i).WithParam("--data") } rangesRaw, ok := entry["ranges"] if !ok { - return common.FlagErrorf("--data[%d].ranges is required", i) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].ranges is required", i).WithParam("--data") } ranges, ok := rangesRaw.([]interface{}) if !ok || len(ranges) == 0 { - return common.FlagErrorf("--data[%d].ranges must be a non-empty array of strings", i) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].ranges must be a non-empty array of strings", i).WithParam("--data") } for j, r := range ranges { s, ok := r.(string) if !ok || s == "" { - return common.FlagErrorf("--data[%d].ranges[%d] must be a non-empty string", i, j) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].ranges[%d] must be a non-empty string", i, j).WithParam("--data") } if _, _, ok := splitSheetRange(s); !ok { - return common.FlagErrorf("--data[%d].ranges[%d] %q must include a sheetId! prefix", i, j, s) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].ranges[%d] %q must include a sheetId! prefix", i, j, s).WithParam("--data") } } styleRaw, ok := entry["style"] if !ok { - return common.FlagErrorf("--data[%d].style is required", i) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].style is required", i).WithParam("--data") } if _, ok := styleRaw.(map[string]interface{}); !ok { - return common.FlagErrorf("--data[%d].style must be a JSON object", i) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data[%d].style must be a JSON object", i).WithParam("--data") } } return nil @@ -74,14 +75,14 @@ var SheetSetStyle = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } var style interface{} if err := json.Unmarshal([]byte(runtime.Str("style")), &style); err != nil { - return common.FlagErrorf("--style must be valid JSON: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--style must be valid JSON: %v", err).WithParam("--style") } if _, ok := style.(map[string]interface{}); !ok { - return common.FlagErrorf("--style must be a JSON object, got %T", style) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--style must be a JSON object, got %T", style).WithParam("--style") } if err := validateSheetRangeInput(runtime.Str("sheet-id"), runtime.Str("range")); err != nil { return err @@ -115,10 +116,10 @@ var SheetSetStyle = common.Shortcut{ r := normalizePointRange(runtime.Str("sheet-id"), runtime.Str("range")) var style interface{} if err := json.Unmarshal([]byte(runtime.Str("style")), &style); err != nil { - return common.FlagErrorf("--style must be valid JSON: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--style must be valid JSON: %v", err).WithParam("--style") } - data, err := runtime.CallAPI("PUT", + data, err := runtime.CallAPITyped("PUT", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/style", validate.EncodePathSegment(token)), nil, map[string]interface{}{ @@ -154,7 +155,7 @@ var SheetBatchSetStyle = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } return validateBatchStyleData(runtime.Str("data")) }, @@ -181,11 +182,11 @@ var SheetBatchSetStyle = common.Shortcut{ var data interface{} if err := json.Unmarshal([]byte(runtime.Str("data")), &data); err != nil { - return common.FlagErrorf("--data must be valid JSON: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data must be valid JSON: %v", err).WithParam("--data") } normalizeBatchStyleRanges(data) - result, err := runtime.CallAPI("PUT", + result, err := runtime.CallAPITyped("PUT", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/styles_batch_update", validate.EncodePathSegment(token)), nil, map[string]interface{}{ @@ -242,7 +243,7 @@ var SheetMergeCells = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } if err := validateSheetRangeInput(runtime.Str("sheet-id"), runtime.Str("range")); err != nil { return err @@ -271,7 +272,7 @@ var SheetMergeCells = common.Shortcut{ r := normalizeSheetRange(runtime.Str("sheet-id"), runtime.Str("range")) - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/merge_cells", validate.EncodePathSegment(token)), nil, map[string]interface{}{ @@ -306,7 +307,7 @@ var SheetUnmergeCells = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } if err := validateSheetRangeInput(runtime.Str("sheet-id"), runtime.Str("range")); err != nil { return err @@ -334,7 +335,7 @@ var SheetUnmergeCells = common.Shortcut{ r := normalizeSheetRange(runtime.Str("sheet-id"), runtime.Str("range")) - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/unmerge_cells", validate.EncodePathSegment(token)), nil, map[string]interface{}{ diff --git a/shortcuts/sheets/backward/lark_sheets_dropdown.go b/shortcuts/sheets/backward/lark_sheets_dropdown.go index e5645af65e..6e07dad3ee 100644 --- a/shortcuts/sheets/backward/lark_sheets_dropdown.go +++ b/shortcuts/sheets/backward/lark_sheets_dropdown.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -27,7 +28,7 @@ func validateDropdownToken(runtime *common.RuntimeContext) (string, error) { token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return "", common.FlagErrorf("specify --url or --spreadsheet-token") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } return token, nil } @@ -35,10 +36,10 @@ func validateDropdownToken(runtime *common.RuntimeContext) (string, error) { func parseJSONStringArray(flagName, value string) ([]interface{}, error) { var typed []string if err := json.Unmarshal([]byte(value), &typed); err != nil { - return nil, common.FlagErrorf("--%s must be a JSON array of strings: %v", flagName, err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must be a JSON array of strings: %v", flagName, err).WithParam("--" + flagName) } if typed == nil { - return nil, common.FlagErrorf("--%s must be a JSON array, got null", flagName) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must be a JSON array, got null", flagName).WithParam("--" + flagName) } arr := make([]interface{}, len(typed)) for i, s := range typed { @@ -53,12 +54,12 @@ func validateRangesFlag(runtime *common.RuntimeContext) ([]interface{}, error) { return nil, err } if len(ranges) == 0 { - return nil, common.FlagErrorf("--ranges must not be empty") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--ranges must not be empty").WithParam("--ranges") } for i, r := range ranges { s, _ := r.(string) if _, _, ok := splitSheetRange(s); !ok { - return nil, common.FlagErrorf("--ranges[%d] %q must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)", i, s) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--ranges[%d] %q must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)", i, s).WithParam("--ranges") } } return ranges, nil @@ -70,7 +71,7 @@ func buildDropdownBody(runtime *common.RuntimeContext) (map[string]interface{}, return nil, err } if len(condValues) == 0 { - return nil, common.FlagErrorf("--condition-values must not be empty") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--condition-values must not be empty").WithParam("--condition-values") } dv := map[string]interface{}{ @@ -90,7 +91,7 @@ func buildDropdownBody(runtime *common.RuntimeContext) (map[string]interface{}, return nil, err } if len(colors) != len(condValues) { - return nil, common.FlagErrorf("--colors length (%d) must match --condition-values length (%d)", len(colors), len(condValues)) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--colors length (%d) must match --condition-values length (%d)", len(colors), len(condValues)).WithParam("--colors") } opts["colors"] = colors } @@ -123,7 +124,7 @@ var SheetSetDropdown = common.Shortcut{ return err } if _, _, ok := splitSheetRange(runtime.Str("range")); !ok { - return common.FlagErrorf("--range must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)").WithParam("--range") } _, err := buildDropdownBody(runtime) return err @@ -147,7 +148,7 @@ var SheetSetDropdown = common.Shortcut{ return err } - data, err := runtime.CallAPI("POST", dataValidationBasePath(token), nil, + data, err := runtime.CallAPITyped("POST", dataValidationBasePath(token), nil, map[string]interface{}{ "range": runtime.Str("range"), "dataValidationType": "list", @@ -214,7 +215,7 @@ var SheetUpdateDropdown = common.Shortcut{ return err } - data, err := runtime.CallAPI("PUT", dataValidationSheetPath(token, runtime.Str("sheet-id")), nil, + data, err := runtime.CallAPITyped("PUT", dataValidationSheetPath(token, runtime.Str("sheet-id")), nil, map[string]interface{}{ "ranges": ranges, "dataValidationType": "list", @@ -247,7 +248,7 @@ var SheetGetDropdown = common.Shortcut{ return err } if _, _, ok := splitSheetRange(runtime.Str("range")); !ok { - return common.FlagErrorf("--range must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range must be a fully qualified range with sheet ID prefix (e.g. !A2:A100)").WithParam("--range") } return nil }, @@ -259,7 +260,7 @@ var SheetGetDropdown = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateDropdownToken(runtime) - data, err := runtime.CallAPI("GET", dataValidationBasePath(token), + data, err := runtime.CallAPITyped("GET", dataValidationBasePath(token), map[string]interface{}{ "range": runtime.Str("range"), "dataValidationType": "list", @@ -319,7 +320,7 @@ var SheetDeleteDropdown = common.Shortcut{ dvRanges = append(dvRanges, map[string]interface{}{"range": r}) } - data, err := runtime.CallAPI("DELETE", dataValidationBasePath(token), nil, + data, err := runtime.CallAPITyped("DELETE", dataValidationBasePath(token), nil, map[string]interface{}{ "dataValidationRanges": dvRanges, }, diff --git a/shortcuts/sheets/backward/lark_sheets_filter_views.go b/shortcuts/sheets/backward/lark_sheets_filter_views.go index b76a473f3e..fc8105e185 100644 --- a/shortcuts/sheets/backward/lark_sheets_filter_views.go +++ b/shortcuts/sheets/backward/lark_sheets_filter_views.go @@ -9,7 +9,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -59,7 +59,7 @@ var SheetCreateFilterView = common.Shortcut{ return err } if strings.TrimSpace(runtime.Str("range")) == "" { - return common.FlagErrorf("--range must not be empty") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range must not be empty").WithParam("--range") } return nil }, @@ -85,7 +85,7 @@ var SheetCreateFilterView = common.Shortcut{ if s := runtime.Str("filter-view-id"); s != "" { body["filter_view_id"] = s } - data, err := runtime.CallAPI("POST", filterViewBasePath(token, runtime.Str("sheet-id")), nil, body) + data, err := runtime.CallAPITyped("POST", filterViewBasePath(token, runtime.Str("sheet-id")), nil, body) if err != nil { return err } @@ -115,7 +115,7 @@ var SheetUpdateFilterView = common.Shortcut{ } if !hasNonEmptyStringFlag(runtime, "range") && !hasNonEmptyStringFlag(runtime, "filter-view-name") { - return common.FlagErrorf("specify at least one of --range or --filter-view-name") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --range or --filter-view-name").WithParams(errs.InvalidParam{Name: "--range", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--filter-view-name", Reason: "required; specify at least one"}) } return nil }, @@ -141,7 +141,7 @@ var SheetUpdateFilterView = common.Shortcut{ if s := runtime.Str("filter-view-name"); s != "" { body["filter_view_name"] = s } - data, err := runtime.CallAPI("PATCH", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, body) + data, err := runtime.CallAPITyped("PATCH", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, body) if err != nil { return err } @@ -174,7 +174,7 @@ var SheetListFilterViews = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("GET", filterViewBasePath(token, runtime.Str("sheet-id"))+"/query", nil, nil) + data, err := runtime.CallAPITyped("GET", filterViewBasePath(token, runtime.Str("sheet-id"))+"/query", nil, nil) if err != nil { return err } @@ -208,7 +208,7 @@ var SheetGetFilterView = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("GET", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, nil) + data, err := runtime.CallAPITyped("GET", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, nil) if err != nil { return err } @@ -242,7 +242,7 @@ var SheetDeleteFilterView = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("DELETE", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, nil) + data, err := runtime.CallAPITyped("DELETE", filterViewItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, nil) if err != nil { return err } @@ -284,7 +284,7 @@ var SheetCreateFilterViewCondition = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) body := buildConditionBody(runtime, true) - data, err := runtime.CallAPI("POST", filterViewConditionBasePath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, body) + data, err := runtime.CallAPITyped("POST", filterViewConditionBasePath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id")), nil, body) if err != nil { return err } @@ -317,7 +317,7 @@ var SheetUpdateFilterViewCondition = common.Shortcut{ if !hasNonEmptyStringFlag(runtime, "filter-type") && !hasNonEmptyStringFlag(runtime, "compare-type") && !hasNonEmptyStringFlag(runtime, "expected") { - return common.FlagErrorf("specify at least one of --filter-type, --compare-type, or --expected") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --filter-type, --compare-type, or --expected").WithParams(errs.InvalidParam{Name: "--filter-type", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--compare-type", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--expected", Reason: "required; specify at least one"}) } if s := runtime.Str("expected"); s != "" { return validateExpectedFlag(s) @@ -335,7 +335,7 @@ var SheetUpdateFilterViewCondition = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) body := buildConditionBody(runtime, false) - data, err := runtime.CallAPI("PUT", + data, err := runtime.CallAPITyped("PUT", filterViewConditionItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id"), runtime.Str("condition-id")), nil, body) if err != nil { @@ -371,7 +371,7 @@ var SheetListFilterViewConditions = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("GET", + data, err := runtime.CallAPITyped("GET", filterViewConditionBasePath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id"))+"/query", nil, nil) if err != nil { @@ -409,7 +409,7 @@ var SheetGetFilterViewCondition = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("GET", + data, err := runtime.CallAPITyped("GET", filterViewConditionItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id"), runtime.Str("condition-id")), nil, nil) if err != nil { @@ -447,7 +447,7 @@ var SheetDeleteFilterViewCondition = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFilterViewToken(runtime) - data, err := runtime.CallAPI("DELETE", + data, err := runtime.CallAPITyped("DELETE", filterViewConditionItemPath(token, runtime.Str("sheet-id"), runtime.Str("filter-view-id"), runtime.Str("condition-id")), nil, nil) if err != nil { @@ -464,7 +464,7 @@ func validateExpectedFlag(s string) error { } var arr []interface{} if err := json.Unmarshal([]byte(s), &arr); err != nil { - return output.ErrValidation("--expected must be a JSON array (e.g. [\"6\"]), got: %s", s) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--expected must be a JSON array (e.g. [\"6\"]), got: %s", s).WithParam("--expected") } return nil } diff --git a/shortcuts/sheets/backward/lark_sheets_float_images.go b/shortcuts/sheets/backward/lark_sheets_float_images.go index a0b7bf4904..a06cb7f204 100644 --- a/shortcuts/sheets/backward/lark_sheets_float_images.go +++ b/shortcuts/sheets/backward/lark_sheets_float_images.go @@ -8,8 +8,8 @@ import ( "fmt" "path/filepath" + "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" ) @@ -115,10 +115,14 @@ var SheetMediaUpload = common.Shortcut{ func validateSheetMediaUploadFile(runtime *common.RuntimeContext, filePath string) (string, fileio.FileInfo, error) { stat, err := runtime.FileIO().Stat(filePath) if err != nil { - return "", nil, common.WrapInputStatError(err, "file not found") + wrapped := common.WrapInputStatErrorTyped(err, "file not found") + if v, ok := wrapped.(*errs.ValidationError); ok { + return "", nil, v.WithParam("--file") + } + return "", nil, wrapped } if !stat.Mode().IsRegular() { - return "", nil, output.ErrValidation("file must be a regular file: %s", filePath) + return "", nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", filePath).WithParam("--file") } return filePath, stat, nil } @@ -131,7 +135,7 @@ func resolveSheetMediaUploadParent(runtime *common.RuntimeContext) (string, erro } } if token == "" { - return "", common.FlagErrorf("specify --url or --spreadsheet-token") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } return token, nil } @@ -181,7 +185,7 @@ func validateFloatImageToken(runtime *common.RuntimeContext) (string, error) { } } if token == "" { - return "", common.FlagErrorf("specify --url or --spreadsheet-token") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } return token, nil } @@ -194,7 +198,7 @@ func validateFloatImageRange(sheetID, rangeVal string) error { return err } if prefix, _, ok := splitSheetRange(rangeVal); ok && sheetID != "" && prefix != sheetID { - return common.FlagErrorf("--range prefix %q does not match --sheet-id %q", prefix, sheetID) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--range prefix %q does not match --sheet-id %q", prefix, sheetID).WithParam("--range") } return nil } @@ -206,7 +210,7 @@ func validateFloatImageUpdatePayload(runtime *common.RuntimeContext) error { runtime.Cmd.Flags().Changed("offset-x") || runtime.Cmd.Flags().Changed("offset-y") if !hasField { - return common.FlagErrorf("specify at least one of --range, --width, --height, --offset-x, --offset-y to update") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --range, --width, --height, --offset-x, --offset-y to update").WithParams(errs.InvalidParam{Name: "--range", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--width", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--height", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--offset-x", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--offset-y", Reason: "required; specify at least one"}) } return nil } @@ -214,22 +218,22 @@ func validateFloatImageUpdatePayload(runtime *common.RuntimeContext) error { func validateFloatImageDims(runtime *common.RuntimeContext) error { if runtime.Cmd.Flags().Changed("width") { if v := runtime.Int("width"); v < 20 { - return common.FlagErrorf("--width must be >= 20 pixels, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--width must be >= 20 pixels, got %d", v).WithParam("--width") } } if runtime.Cmd.Flags().Changed("height") { if v := runtime.Int("height"); v < 20 { - return common.FlagErrorf("--height must be >= 20 pixels, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--height must be >= 20 pixels, got %d", v).WithParam("--height") } } if runtime.Cmd.Flags().Changed("offset-x") { if v := runtime.Int("offset-x"); v < 0 { - return common.FlagErrorf("--offset-x must be >= 0, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--offset-x must be >= 0, got %d", v).WithParam("--offset-x") } } if runtime.Cmd.Flags().Changed("offset-y") { if v := runtime.Int("offset-y"); v < 0 { - return common.FlagErrorf("--offset-y must be >= 0, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--offset-y must be >= 0, got %d", v).WithParam("--offset-y") } } return nil @@ -304,7 +308,7 @@ var SheetCreateFloatImage = common.Shortcut{ if s := runtime.Str("float-image-id"); s != "" { body["float_image_id"] = s } - data, err := runtime.CallAPI("POST", floatImageBasePath(token, runtime.Str("sheet-id")), nil, body) + data, err := runtime.CallAPITyped("POST", floatImageBasePath(token, runtime.Str("sheet-id")), nil, body) if err != nil { return err } @@ -353,7 +357,7 @@ var SheetUpdateFloatImage = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFloatImageToken(runtime) body := buildFloatImageBody(runtime, false) - data, err := runtime.CallAPI("PATCH", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, body) + data, err := runtime.CallAPITyped("PATCH", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, body) if err != nil { return err } @@ -387,7 +391,7 @@ var SheetGetFloatImage = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFloatImageToken(runtime) - data, err := runtime.CallAPI("GET", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, nil) + data, err := runtime.CallAPITyped("GET", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, nil) if err != nil { return err } @@ -420,7 +424,7 @@ var SheetListFloatImages = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFloatImageToken(runtime) - data, err := runtime.CallAPI("GET", floatImageBasePath(token, runtime.Str("sheet-id"))+"/query", nil, nil) + data, err := runtime.CallAPITyped("GET", floatImageBasePath(token, runtime.Str("sheet-id"))+"/query", nil, nil) if err != nil { return err } @@ -454,7 +458,7 @@ var SheetDeleteFloatImage = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateFloatImageToken(runtime) - data, err := runtime.CallAPI("DELETE", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, nil) + data, err := runtime.CallAPITyped("DELETE", floatImageItemPath(token, runtime.Str("sheet-id"), runtime.Str("float-image-id")), nil, nil) if err != nil { return err } diff --git a/shortcuts/sheets/backward/lark_sheets_row_column_management.go b/shortcuts/sheets/backward/lark_sheets_row_column_management.go index 581c8eb0ed..ea1e913a85 100644 --- a/shortcuts/sheets/backward/lark_sheets_row_column_management.go +++ b/shortcuts/sheets/backward/lark_sheets_row_column_management.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -31,7 +32,7 @@ var SheetAddDimension = common.Shortcut{ } length := runtime.Int("length") if length < 1 || length > 5000 { - return common.FlagErrorf("--length must be between 1 and 5000, got %d", length) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--length must be between 1 and 5000, got %d", length).WithParam("--length") } return nil }, @@ -51,7 +52,7 @@ var SheetAddDimension = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)), nil, map[string]interface{}{ @@ -91,10 +92,10 @@ var SheetInsertDimension = common.Shortcut{ return err } if runtime.Int("start-index") < 0 { - return common.FlagErrorf("--start-index must be >= 0") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--start-index must be >= 0").WithParam("--start-index") } if runtime.Int("end-index") <= runtime.Int("start-index") { - return common.FlagErrorf("--end-index must be greater than --start-index") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--end-index must be greater than --start-index").WithParam("--end-index") } return nil }, @@ -131,7 +132,7 @@ var SheetInsertDimension = common.Shortcut{ body["inheritStyle"] = s } - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/insert_dimension_range", validate.EncodePathSegment(token)), nil, body, ) @@ -165,16 +166,16 @@ var SheetUpdateDimension = common.Shortcut{ return err } if runtime.Int("start-index") < 1 { - return common.FlagErrorf("--start-index must be >= 1") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--start-index must be >= 1").WithParam("--start-index") } if runtime.Int("end-index") < runtime.Int("start-index") { - return common.FlagErrorf("--end-index must be >= --start-index") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--end-index must be >= --start-index").WithParam("--end-index") } if !runtime.Cmd.Flags().Changed("visible") && !runtime.Cmd.Flags().Changed("fixed-size") { - return common.FlagErrorf("specify at least one of --visible or --fixed-size") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --visible or --fixed-size").WithParams(errs.InvalidParam{Name: "--visible", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--fixed-size", Reason: "required; specify at least one"}) } if runtime.Cmd.Flags().Changed("fixed-size") && runtime.Int("fixed-size") < 1 { - return common.FlagErrorf("--fixed-size must be >= 1") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--fixed-size must be >= 1").WithParam("--fixed-size") } return nil }, @@ -211,7 +212,7 @@ var SheetUpdateDimension = common.Shortcut{ props["fixedSize"] = runtime.Int("fixed-size") } - data, err := runtime.CallAPI("PUT", + data, err := runtime.CallAPITyped("PUT", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)), nil, map[string]interface{}{ @@ -253,13 +254,13 @@ var SheetMoveDimension = common.Shortcut{ return err } if runtime.Int("start-index") < 0 { - return common.FlagErrorf("--start-index must be >= 0") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--start-index must be >= 0").WithParam("--start-index") } if runtime.Int("end-index") < runtime.Int("start-index") { - return common.FlagErrorf("--end-index must be >= --start-index") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--end-index must be >= --start-index").WithParam("--end-index") } if runtime.Int("destination-index") < 0 { - return common.FlagErrorf("--destination-index must be >= 0") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--destination-index must be >= 0").WithParam("--destination-index") } return nil }, @@ -281,7 +282,7 @@ var SheetMoveDimension = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s/sheets/%s/move_dimension", validate.EncodePathSegment(token), validate.EncodePathSegment(runtime.Str("sheet-id")), @@ -324,10 +325,10 @@ var SheetDeleteDimension = common.Shortcut{ return err } if runtime.Int("start-index") < 1 { - return common.FlagErrorf("--start-index must be >= 1") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--start-index must be >= 1").WithParam("--start-index") } if runtime.Int("end-index") < runtime.Int("start-index") { - return common.FlagErrorf("--end-index must be >= --start-index") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--end-index must be >= --start-index").WithParam("--end-index") } return nil }, @@ -348,7 +349,7 @@ var SheetDeleteDimension = common.Shortcut{ Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("DELETE", + data, err := runtime.CallAPITyped("DELETE", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)), nil, map[string]interface{}{ diff --git a/shortcuts/sheets/backward/lark_sheets_sheet_export_test.go b/shortcuts/sheets/backward/lark_sheets_sheet_export_test.go index afe456b5d7..60d5435c9c 100644 --- a/shortcuts/sheets/backward/lark_sheets_sheet_export_test.go +++ b/shortcuts/sheets/backward/lark_sheets_sheet_export_test.go @@ -76,6 +76,23 @@ func TestSheetExportDryRunIncludesSubIDForCSV(t *testing.T) { } } +func TestSheetExportDryRunRejectsUnsafeOutputPath(t *testing.T) { + t.Parallel() + + f, _, _, _ := cmdutil.TestFactory(t, sheetsTestConfig()) + err := mountAndRunSheets(t, SheetExport, []string{ + "+export", + "--spreadsheet-token", "shtTOKEN", + "--file-extension", "xlsx", + "--output-path", "../escape.xlsx", + "--dry-run", + "--as", "user", + }, f, nil) + if err == nil || !strings.Contains(err.Error(), "unsafe output path") { + t.Fatalf("expected unsafe output-path validation error, got: %v", err) + } +} + func TestSheetExportCommandRejectsInvalidFileExtension(t *testing.T) { t.Parallel() diff --git a/shortcuts/sheets/backward/lark_sheets_sheet_manage_test.go b/shortcuts/sheets/backward/lark_sheets_sheet_manage_test.go index 1a8115b7a6..ea52fe159b 100644 --- a/shortcuts/sheets/backward/lark_sheets_sheet_manage_test.go +++ b/shortcuts/sheets/backward/lark_sheets_sheet_manage_test.go @@ -6,14 +6,13 @@ package backward import ( "context" "encoding/json" - "errors" "reflect" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" "github.com/tidwall/gjson" ) @@ -402,38 +401,26 @@ func TestSheetCopySheetExecuteMoveFailureIncludesCopiedSheetRecovery(t *testing. t.Fatal("expected move failure, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected *output.ExitError with detail, got %T: %v", err, err) - } - if exitErr.Detail.Code != 1310211 { - t.Fatalf("error code = %d, want 1310211", exitErr.Detail.Code) - } - if !strings.Contains(exitErr.Detail.Message, `sheet copied successfully as "sheet_copy"`) { - t.Fatalf("message missing copied sheet id: %q", exitErr.Detail.Message) - } - if !strings.Contains(exitErr.Detail.Hint, "do not retry +copy-sheet") { - t.Fatalf("hint missing retry guard: %q", exitErr.Detail.Hint) - } - if !strings.Contains(exitErr.Detail.Hint, "+update-sheet --spreadsheet-token shtTOKEN --sheet-id sheet_copy --index 2") { - t.Fatalf("hint missing recovery command: %q", exitErr.Detail.Hint) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T: %v", err, err) } - - detail, _ := exitErr.Detail.Detail.(map[string]interface{}) - if detail["partial_success"] != true { - t.Fatalf("partial_success = %#v, want true", detail["partial_success"]) + if p.Code != 1310211 { + t.Fatalf("error code = %d, want 1310211", p.Code) } - if detail["sheet_id"] != "sheet_copy" { - t.Fatalf("sheet_id = %#v, want %q", detail["sheet_id"], "sheet_copy") + if !strings.Contains(p.Message, `sheet copied successfully as "sheet_copy"`) { + t.Fatalf("message missing copied sheet id: %q", p.Message) } - if detail["requested_index"] != 2 { - t.Fatalf("requested_index = %#v, want 2", detail["requested_index"]) + if !strings.Contains(p.Hint, "do not retry +copy-sheet") { + t.Fatalf("hint missing retry guard: %q", p.Hint) } - if detail["retry_command"] != "lark-cli sheets +update-sheet --spreadsheet-token shtTOKEN --sheet-id sheet_copy --index 2" { - t.Fatalf("retry_command = %#v", detail["retry_command"]) + // The recovery command in the hint is the AI-actionable signal: retry only + // the move (not the whole +copy-sheet, which would duplicate the sheet). + if !strings.Contains(p.Hint, "+update-sheet --spreadsheet-token shtTOKEN --sheet-id sheet_copy --index 2") { + t.Fatalf("hint missing recovery command: %q", p.Hint) } - if detail["log_id"] != "log-move-failed" { - t.Fatalf("log_id = %#v, want %q", detail["log_id"], "log-move-failed") + if p.LogID != "log-move-failed" { + t.Fatalf("log_id = %q, want %q", p.LogID, "log-move-failed") } } diff --git a/shortcuts/sheets/backward/lark_sheets_sheet_management.go b/shortcuts/sheets/backward/lark_sheets_sheet_management.go index 0484a6cd46..4988fdd4c0 100644 --- a/shortcuts/sheets/backward/lark_sheets_sheet_management.go +++ b/shortcuts/sheets/backward/lark_sheets_sheet_management.go @@ -5,11 +5,10 @@ package backward import ( "context" - "errors" "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -21,63 +20,63 @@ func sheetBatchUpdatePath(token string) string { } func validateSheetManageToken(runtime *common.RuntimeContext) (string, error) { - if err := common.ExactlyOne(runtime, "url", "spreadsheet-token"); err != nil { + if err := common.ExactlyOneTyped(runtime, "url", "spreadsheet-token"); err != nil { return "", err } if token := strings.TrimSpace(runtime.Str("spreadsheet-token")); token != "" { if err := validate.RejectControlChars(token, "spreadsheet-token"); err != nil { - return "", common.FlagErrorf("%v", err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--spreadsheet-token").WithCause(err) } return token, nil } url := strings.TrimSpace(runtime.Str("url")) if url == "" { - return "", common.FlagErrorf("specify --url or --spreadsheet-token") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } token := extractSpreadsheetToken(url) if token == "" || token == url { - return "", common.FlagErrorf("--url must be a spreadsheet URL like https://.../sheets/") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--url must be a spreadsheet URL like https://.../sheets/").WithParam("--url") } if err := validate.RejectControlChars(token, "url"); err != nil { - return "", common.FlagErrorf("%v", err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--url").WithCause(err) } return token, nil } func validateSheetID(flagName, sheetID string) error { if strings.TrimSpace(sheetID) == "" { - return common.FlagErrorf("specify --%s", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --%s", flagName).WithParam("--" + flagName) } if err := validate.RejectControlChars(sheetID, flagName); err != nil { - return common.FlagErrorf("%v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--" + flagName).WithCause(err) } return nil } func validateSheetTitle(flagName, title string) error { if title == "" { - return common.FlagErrorf("--%s must not be empty", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must not be empty", flagName).WithParam("--" + flagName) } if strings.ContainsAny(title, "\t\r\n") { - return common.FlagErrorf("--%s must not contain tabs or line breaks", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must not contain tabs or line breaks", flagName).WithParam("--" + flagName) } if err := validate.RejectControlChars(title, flagName); err != nil { - return common.FlagErrorf("%v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--" + flagName).WithCause(err) } if len([]rune(title)) > 100 { - return common.FlagErrorf("--%s must be <= 100 characters", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must be <= 100 characters", flagName).WithParam("--" + flagName) } if strings.ContainsAny(title, `/\?*[]:`) || strings.Contains(title, `\`) { - return common.FlagErrorf("--%s must not contain any of / \\ ? * [ ] :", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must not contain any of / \\ ? * [ ] :", flagName).WithParam("--" + flagName) } return nil } func validateNonNegativeInt(flagName string, value int) error { if value < 0 { - return common.FlagErrorf("--%s must be >= 0, got %d", flagName, value) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--%s must be >= 0, got %d", flagName, value).WithParam("--" + flagName) } return nil } @@ -287,36 +286,18 @@ func mergeSheetOutputs(base, overlay map[string]interface{}) map[string]interfac return out } -func mergeSheetErrorDetail(detail interface{}, overlay map[string]interface{}) interface{} { - if len(overlay) == 0 { - return detail - } - if detail == nil { - return overlay - } - if existing, ok := detail.(map[string]interface{}); ok { - merged := map[string]interface{}{} - for k, v := range existing { - merged[k] = v - } - for k, v := range overlay { - merged[k] = v - } - return merged - } - - merged := map[string]interface{}{} - for k, v := range overlay { - merged[k] = v - } - merged["cause_detail"] = detail - return merged -} - func copySheetMoveRetryCommand(token, sheetID string, index int) string { return fmt.Sprintf("lark-cli sheets +update-sheet --spreadsheet-token %s --sheet-id %s --index %d", token, sheetID, index) } +// wrapCopySheetMoveError reports a +copy-sheet that created the new sheet but +// then failed to move it to the requested index. The copy already succeeded, so +// the recovery is to retry only the move (not the whole +copy-sheet, which would +// duplicate the sheet) — that guard and the exact retry command go into the +// hint. The underlying move error is already a typed errs.* error from +// CallAPITyped; its category/subtype/code/log_id are preserved in place +// (mirroring drive's enrichDriveSearchError) so the failure stays accurately +// classified, with only the partial-success context folded into message and hint. func wrapCopySheetMoveError(err error, token, sheetID string, index int) error { if strings.TrimSpace(sheetID) == "" { return err @@ -329,46 +310,22 @@ func wrapCopySheetMoveError(err error, token, sheetID string, index int) error { sheetID, retryCommand, ) - detail := map[string]interface{}{ - "partial_success": true, - "failed_step": "move_copied_sheet", - "spreadsheet_token": token, - "sheet_id": sheetID, - "requested_index": index, - "retry_command": retryCommand, - } - - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - if upstreamHint := strings.TrimSpace(exitErr.Detail.Hint); upstreamHint != "" { - hint = upstreamHint + "\n" + hint - } - return &output.ExitError{ - Code: exitErr.Code, - Detail: &output.ErrDetail{ - Type: exitErr.Detail.Type, - Code: exitErr.Detail.Code, - Message: fmt.Sprintf("%s: %s", msg, exitErr.Detail.Message), - Hint: hint, - ConsoleURL: exitErr.Detail.ConsoleURL, - Risk: exitErr.Detail.Risk, - Detail: mergeSheetErrorDetail(exitErr.Detail.Detail, detail), - }, - Err: err, - Raw: exitErr.Raw, + + if p, ok := errs.ProblemOf(err); ok { + if upstream := strings.TrimSpace(p.Message); upstream != "" { + p.Message = fmt.Sprintf("%s: %s", msg, upstream) + } else { + p.Message = msg } + if upstreamHint := strings.TrimSpace(p.Hint); upstreamHint != "" { + p.Hint = upstreamHint + "\n" + hint + } else { + p.Hint = hint + } + return err } - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "api_error", - Message: fmt.Sprintf("%s: %v", msg, err), - Hint: hint, - Detail: detail, - }, - Err: err, - } + return errs.NewInternalError(errs.SubtypeSDKError, "%s: %v", msg, err).WithHint(hint).WithCause(err) } func validateUpdateSheetFlags(runtime *common.RuntimeContext) error { @@ -397,7 +354,7 @@ func validateUpdateSheetFlags(runtime *common.RuntimeContext) error { } if runtime.Changed("lock-info") { if err := validate.RejectControlChars(runtime.Str("lock-info"), "lock-info"); err != nil { - return common.FlagErrorf("%v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--lock-info").WithCause(err) } } @@ -405,24 +362,24 @@ func validateUpdateSheetFlags(runtime *common.RuntimeContext) error { if hasProtectConfig { lock := runtime.Str("lock") if !runtime.Changed("lock") { - return common.FlagErrorf("specify --lock when updating protection settings") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --lock when updating protection settings").WithParam("--lock") } if runtime.Changed("lock-info") && lock != "LOCK" { - return common.FlagErrorf("--lock-info requires --lock LOCK") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--lock-info requires --lock LOCK").WithParam("--lock-info") } if runtime.Changed("user-ids") { if lock != "LOCK" { - return common.FlagErrorf("--user-ids requires --lock LOCK") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-ids requires --lock LOCK").WithParam("--user-ids") } if runtime.Str("user-id-type") == "" { - return common.FlagErrorf("--user-ids requires --user-id-type") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-ids requires --user-id-type").WithParam("--user-id-type") } userIDs, err := parseJSONStringArray("user-ids", runtime.Str("user-ids")) if err != nil { return err } if len(userIDs) == 0 { - return common.FlagErrorf("--user-ids must not be empty") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-ids must not be empty").WithParam("--user-ids") } } } @@ -434,7 +391,7 @@ func validateUpdateSheetFlags(runtime *common.RuntimeContext) error { runtime.Changed("frozen-col-count") || hasProtectConfig if !hasUpdate { - return common.FlagErrorf("specify at least one of --title, --index, --hidden, --frozen-row-count, --frozen-col-count, --lock, --lock-info, or --user-ids") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify at least one of --title, --index, --hidden, --frozen-row-count, --frozen-col-count, --lock, --lock-info, or --user-ids").WithParams(errs.InvalidParam{Name: "--title", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--index", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--hidden", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--frozen-row-count", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--frozen-col-count", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--lock", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--lock-info", Reason: "required; specify at least one"}, errs.InvalidParam{Name: "--user-ids", Reason: "required; specify at least one"}) } return nil @@ -530,7 +487,7 @@ var SheetCreateSheet = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("POST", sheetBatchUpdatePath(token), nil, buildCreateSheetBody(runtime)) + data, err := runtime.CallAPITyped("POST", sheetBatchUpdatePath(token), nil, buildCreateSheetBody(runtime)) if err != nil { return err } @@ -593,7 +550,7 @@ var SheetCopySheet = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("POST", sheetBatchUpdatePath(token), nil, buildCopySheetBody(runtime)) + data, err := runtime.CallAPITyped("POST", sheetBatchUpdatePath(token), nil, buildCopySheetBody(runtime)) if err != nil { return err } @@ -604,7 +561,7 @@ var SheetCopySheet = common.Shortcut{ } if runtime.Changed("index") { copiedSheetID, _ := out["sheet_id"].(string) - moveResp, err := runtime.CallAPI("POST", sheetBatchUpdatePath(token), nil, buildMoveCopiedSheetBody(copiedSheetID, runtime.Int("index"))) + moveResp, err := runtime.CallAPITyped("POST", sheetBatchUpdatePath(token), nil, buildMoveCopiedSheetBody(copiedSheetID, runtime.Int("index"))) if err != nil { return wrapCopySheetMoveError(err, token, copiedSheetID, runtime.Int("index")) } @@ -644,7 +601,7 @@ var SheetDeleteSheet = common.Shortcut{ }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { token, _ := validateSheetManageToken(runtime) - data, err := runtime.CallAPI("POST", sheetBatchUpdatePath(token), nil, buildDeleteSheetBody(runtime.Str("sheet-id"))) + data, err := runtime.CallAPITyped("POST", sheetBatchUpdatePath(token), nil, buildDeleteSheetBody(runtime.Str("sheet-id"))) if err != nil { return err } @@ -707,7 +664,7 @@ var SheetUpdateSheet = common.Shortcut{ params = map[string]interface{}{"user_id_type": userIDType} } - data, err := runtime.CallAPI("POST", sheetBatchUpdatePath(token), params, body) + data, err := runtime.CallAPITyped("POST", sheetBatchUpdatePath(token), params, body) if err != nil { return err } diff --git a/shortcuts/sheets/backward/lark_sheets_spreadsheet_management.go b/shortcuts/sheets/backward/lark_sheets_spreadsheet_management.go index 5aa1fdaec1..2f9314f92f 100644 --- a/shortcuts/sheets/backward/lark_sheets_spreadsheet_management.go +++ b/shortcuts/sheets/backward/lark_sheets_spreadsheet_management.go @@ -13,8 +13,8 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "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" ) @@ -36,7 +36,7 @@ var SheetInfo = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } if token == "" { - return common.FlagErrorf("specify --url or --spreadsheet-token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "specify --url or --spreadsheet-token").WithParams(errs.InvalidParam{Name: "--url", Reason: "required; specify one"}, errs.InvalidParam{Name: "--spreadsheet-token", Reason: "required; specify one"}) } return nil }, @@ -55,7 +55,7 @@ var SheetInfo = common.Shortcut{ token = extractSpreadsheetToken(runtime.Str("url")) } - spreadsheetData, err := runtime.CallAPI("GET", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s", validate.EncodePathSegment(token)), nil, nil) + spreadsheetData, err := runtime.CallAPITyped("GET", fmt.Sprintf("/open-apis/sheets/v3/spreadsheets/%s", validate.EncodePathSegment(token)), nil, nil) if err != nil { return err } @@ -95,13 +95,13 @@ var SheetCreate = common.Shortcut{ if headersStr := runtime.Str("headers"); headersStr != "" { var headers []interface{} if err := json.Unmarshal([]byte(headersStr), &headers); err != nil { - return common.FlagErrorf("--headers invalid JSON, must be a 1D array") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--headers invalid JSON, must be a 1D array").WithParam("--headers") } } if dataStr := runtime.Str("data"); dataStr != "" { var rows [][]interface{} if err := json.Unmarshal([]byte(dataStr), &rows); err != nil { - return common.FlagErrorf("--data invalid JSON, must be a 2D array") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data invalid JSON, must be a 2D array").WithParam("--data") } } return nil @@ -129,7 +129,7 @@ var SheetCreate = common.Shortcut{ if headersStr != "" { var headers []interface{} if err := json.Unmarshal([]byte(headersStr), &headers); err != nil { - return common.FlagErrorf("--headers invalid JSON, must be a 1D array") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--headers invalid JSON, must be a 1D array").WithParam("--headers") } if len(headers) > 0 { allRows = append(allRows, any(headers)) @@ -139,7 +139,7 @@ var SheetCreate = common.Shortcut{ if dataStr != "" { var rows []interface{} if err := json.Unmarshal([]byte(dataStr), &rows); err != nil { - return common.FlagErrorf("--data invalid JSON, must be a 2D array") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--data invalid JSON, must be a 2D array").WithParam("--data") } if len(rows) > 0 { allRows = append(allRows, rows...) @@ -151,7 +151,7 @@ var SheetCreate = common.Shortcut{ createData["folder_token"] = folderToken } - data, err := runtime.CallAPI("POST", "/open-apis/sheets/v3/spreadsheets", nil, createData) + data, err := runtime.CallAPITyped("POST", "/open-apis/sheets/v3/spreadsheets", nil, createData) if err != nil { return err } @@ -164,7 +164,7 @@ var SheetCreate = common.Shortcut{ if err != nil { return err } - if _, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_append", validate.EncodePathSegment(token)), nil, map[string]interface{}{ + if _, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/values_append", validate.EncodePathSegment(token)), nil, map[string]interface{}{ "valueRange": map[string]interface{}{ "range": appendRange, "values": allRows, @@ -211,8 +211,11 @@ var SheetExport = common.Shortcut{ if _, err := validateSheetManageToken(runtime); err != nil { return err } + if err := validateSheetExportOutputPath(runtime); err != nil { + return err + } if runtime.Str("file-extension") == "csv" && strings.TrimSpace(runtime.Str("sheet-id")) == "" { - return common.FlagErrorf("--sheet-id is required when --file-extension is csv") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--sheet-id is required when --file-extension is csv").WithParam("--sheet-id") } return nil }, @@ -238,10 +241,8 @@ var SheetExport = common.Shortcut{ outputPath := runtime.Str("output-path") sheetID := runtime.Str("sheet-id") - if outputPath != "" { - if _, err := runtime.ResolveSavePath(outputPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) - } + if err := validateSheetExportOutputPath(runtime); err != nil { + return err } exportData := map[string]interface{}{ @@ -253,7 +254,7 @@ var SheetExport = common.Shortcut{ exportData["sub_id"] = sheetID } - data, err := runtime.CallAPI("POST", "/open-apis/drive/v1/export_tasks", nil, exportData) + data, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/export_tasks", nil, exportData) if err != nil { return err } @@ -280,7 +281,7 @@ var SheetExport = common.Shortcut{ } if fileToken == "" { - return output.Errorf(output.ExitAPI, "api_error", "export task timed out") + return errs.NewNetworkError(errs.SubtypeNetworkTimeout, "export task timed out").WithRetryable() } fmt.Fprintf(runtime.IO().ErrOut, "Export complete: file_token=%s\n", fileToken) @@ -298,7 +299,7 @@ var SheetExport = common.Shortcut{ ApiPath: fmt.Sprintf("/open-apis/drive/v1/export_tasks/file/%s/download", validate.EncodePathSegment(fileToken)), }) if err != nil { - return output.ErrNetwork("download failed: %s", err) + return wrapSheetsNetworkErr(err, "download failed: %s", err) } defer resp.Body.Close() @@ -307,7 +308,7 @@ var SheetExport = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return common.WrapSaveErrorTyped(err) } savedPath, _ := runtime.ResolveSavePath(outputPath) @@ -321,3 +322,14 @@ var SheetExport = common.Shortcut{ return nil }, } + +func validateSheetExportOutputPath(runtime *common.RuntimeContext) error { + outputPath := strings.TrimSpace(runtime.Str("output-path")) + if outputPath == "" { + return nil + } + if _, err := runtime.ResolveSavePath(outputPath); err != nil { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output-path").WithCause(err) + } + return nil +} diff --git a/shortcuts/sheets/backward/sheets_errors.go b/shortcuts/sheets/backward/sheets_errors.go new file mode 100644 index 0000000000..65e3bc7a61 --- /dev/null +++ b/shortcuts/sheets/backward/sheets_errors.go @@ -0,0 +1,15 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package backward + +import "github.com/larksuite/cli/errs" + +// wrapSheetsNetworkErr preserves typed boundary errors and only classifies raw +// transport failures that still surface from stream/download paths. +func wrapSheetsNetworkErr(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/sheets/batch_op_dispatch.go b/shortcuts/sheets/batch_op_dispatch.go index 271d4eeca1..5f7d492fa4 100644 --- a/shortcuts/sheets/batch_op_dispatch.go +++ b/shortcuts/sheets/batch_op_dispatch.go @@ -5,8 +5,6 @@ package sheets import ( "strings" - - "github.com/larksuite/cli/shortcuts/common" ) // ─── +batch-update sub-op dispatch ───────────────────────────────────── @@ -198,7 +196,7 @@ var batchOpDispatch = map[string]batchOpMapping{ // turned into a file_token. Callers must pass --image-token / --image-uri. func rejectLocalImageInBatch(fv flagView) error { if strings.TrimSpace(fv.Str("image")) != "" { - return common.FlagErrorf("--image (local upload) is not supported inside +batch-update; pass --image-token or --image-uri instead") + return sheetsValidationForFlag("image", "--image (local upload) is not supported inside +batch-update; pass --image-token or --image-uri instead") } return nil } @@ -208,23 +206,23 @@ func rejectLocalImageInBatch(fv flagView) error { // auto-derives sheet_id / source_index, so both must be supplied explicitly. func sheetMoveBatchInput(fv flagView, token, sheetID, sheetName string) (map[string]interface{}, error) { if sheetID == "" { - return nil, common.FlagErrorf("+sheet-move in +batch-update requires sheet_id (sheet_name needs a network lookup unavailable mid-batch)") + return nil, sheetsValidationForFlag("sheet-id", "+sheet-move in +batch-update requires sheet_id (sheet_name needs a network lookup unavailable mid-batch)") } if !fv.Changed("source-index") { - return nil, common.FlagErrorf("+sheet-move in +batch-update requires source_index (auto-derive needs a network lookup unavailable mid-batch)") + return nil, sheetsValidationForFlag("source-index", "+sheet-move in +batch-update requires source_index (auto-derive needs a network lookup unavailable mid-batch)") } if fv.Int("source-index") < 0 { - return nil, common.FlagErrorf("--source-index must be >= 0") + return nil, sheetsValidationForFlag("source-index", "--source-index must be >= 0") } // Standalone +sheet-move requires --index (see SheetMove.Validate). A batch // sub-op skips that path, and mapFlagView falls back to the flag default (0), // which would silently move the sheet to the front. Require it explicitly so // the batch contract matches the standalone one. if !fv.Changed("index") { - return nil, common.FlagErrorf("+sheet-move in +batch-update requires index") + return nil, sheetsValidationForFlag("index", "+sheet-move in +batch-update requires index") } if fv.Int("index") < 0 { - return nil, common.FlagErrorf("--index must be >= 0") + return nil, sheetsValidationForFlag("index", "--index must be >= 0") } return map[string]interface{}{ "excel_id": token, @@ -254,19 +252,20 @@ var reservedSubOpKeys = []string{"excel_id", "spreadsheet_token", "url"} func translateBatchOp(raw interface{}, token string, index int) (map[string]interface{}, error) { op, ok := raw.(map[string]interface{}) if !ok { - return nil, common.FlagErrorf("operations[%d] must be a JSON object", index) + return nil, sheetsValidationForFlag("operations", "operations[%d] must be a JSON object", index) } scRaw, present := op["shortcut"] if !present { - return nil, common.FlagErrorf("operations[%d]: 'shortcut' field is required", index) + return nil, sheetsValidationForFlag("operations", "operations[%d]: 'shortcut' field is required", index) } sc, ok := scRaw.(string) if !ok || sc == "" { - return nil, common.FlagErrorf("operations[%d]: 'shortcut' must be a non-empty string (got %T)", index, scRaw) + return nil, sheetsValidationForFlag("operations", "operations[%d]: 'shortcut' must be a non-empty string (got %T)", index, scRaw) } mapping, ok := batchOpDispatch[sc] if !ok { - return nil, common.FlagErrorf( + return nil, sheetsValidationForFlag( + "operations", "operations[%d]: shortcut %q not allowed in +batch-update "+ "(read ops / fan-out wrappers like +batch-update / +cells-batch-set-style / +cells-batch-clear / +dropdown-{update,delete} are excluded; "+ "run `lark-cli sheets +batch-update --print-schema --flag-name operations` to see the full enum)", @@ -280,12 +279,13 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte } else { input, ok = inputRaw.(map[string]interface{}) if !ok { - return nil, common.FlagErrorf("operations[%d] (%s): 'input' must be a JSON object (got %T)", index, sc, inputRaw) + return nil, sheetsValidationForFlag("operations", "operations[%d] (%s): 'input' must be a JSON object (got %T)", index, sc, inputRaw) } } // 禁手填 operation —— 由 shortcut 名表达，手填易与 shortcut 不一致。 if _, has := input["operation"]; has { - return nil, common.FlagErrorf( + return nil, sheetsValidationForFlag( + "operations", "operations[%d] (%s): do not pass input.operation manually — it is implied by the shortcut name", index, sc, ) @@ -293,7 +293,8 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte // 禁在 sub-op 重复填 spreadsheet 定位 —— 由 +batch-update 顶层 --url/--token 统一提供。 for _, k := range reservedSubOpKeys { if _, has := input[k]; has { - return nil, common.FlagErrorf( + return nil, sheetsValidationForFlag( + "operations", "operations[%d] (%s): do not pass input.%s — it is already set from +batch-update top-level --url / --token", index, sc, k, ) @@ -302,7 +303,7 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte // 拒绝任何额外的 sub-op 顶层 key（防御未来 schema drift / 用户笔误）。 for k := range op { if k != "shortcut" && k != "input" { - return nil, common.FlagErrorf("operations[%d] (%s): unknown top-level key %q (expected only 'shortcut' and 'input')", index, sc, k) + return nil, sheetsValidationForFlag("operations", "operations[%d] (%s): unknown top-level key %q (expected only 'shortcut' and 'input')", index, sc, k) } } fv := newMapFlagViewForCommand(sc, input) @@ -310,14 +311,14 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte // sub-op's scalar fields here before the translator reads them via // Int/Bool/Float64 (which would otherwise coerce a wrong type to zero). if err := fv.validateRawTypes(); err != nil { - return nil, common.FlagErrorf("operations[%d] (%s): %v", index, sc, err) + return nil, sheetsValidationForFlag("operations", "operations[%d] (%s): %v", index, sc, err) } sheetIDFlag, sheetNameFlag := sheetSelectorFlagsForSubOp(sc) sheetID := strings.TrimSpace(fv.Str(sheetIDFlag)) sheetName := strings.TrimSpace(fv.Str(sheetNameFlag)) body, err := mapping.translate(fv, token, sheetID, sheetName) if err != nil { - return nil, common.FlagErrorf("operations[%d] (%s): %v", index, sc, err) + return nil, sheetsValidationForFlag("operations", "operations[%d] (%s): %v", index, sc, err) } return map[string]interface{}{ "tool_name": mapping.mcpToolName, @@ -328,7 +329,7 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte // translateBatchOperations 翻译整个 ops 数组；fail-fast，遇错立即返回。 func translateBatchOperations(rawOps []interface{}, token string) ([]interface{}, error) { if len(rawOps) == 0 { - return nil, common.FlagErrorf("--operations must be a non-empty JSON array") + return nil, sheetsValidationForFlag("operations", "--operations must be a non-empty JSON array") } out := make([]interface{}, 0, len(rawOps)) for i, raw := range rawOps { diff --git a/shortcuts/sheets/csv_put_guard_test.go b/shortcuts/sheets/csv_put_guard_test.go index 5191e45ee2..29a47815c7 100644 --- a/shortcuts/sheets/csv_put_guard_test.go +++ b/shortcuts/sheets/csv_put_guard_test.go @@ -4,10 +4,12 @@ package sheets import ( + "errors" "os" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" _ "github.com/larksuite/cli/internal/vfs/localfileio" "github.com/larksuite/cli/shortcuts/common" @@ -41,6 +43,16 @@ func TestGuardCSVValueIsNotFilePath(t *testing.T) { if !strings.Contains(err.Error(), "existing file") || !strings.Contains(err.Error(), "@data.csv") { t.Errorf("error should flag the file and suggest @data.csv, got: %v", err) } + if p, ok := errs.ProblemOf(err); !ok || p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("problem = %+v, want validation/invalid_argument", p) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("guard error = %T, want *errs.ValidationError", err) + } + if ve.Param != "--csv" { + t.Errorf("param = %q, want --csv", ve.Param) + } // Content that is not a real file must pass through unchanged. for _, v := range []string{ diff --git a/shortcuts/sheets/csv_put_range_alias_test.go b/shortcuts/sheets/csv_put_range_alias_test.go index 4a631d6f6d..e4157c22bb 100644 --- a/shortcuts/sheets/csv_put_range_alias_test.go +++ b/shortcuts/sheets/csv_put_range_alias_test.go @@ -21,7 +21,6 @@ func TestCsvPutInput_RangeAliasForStartCell(t *testing.T) { {"start-cell direct (unchanged)", map[string]interface{}{"csv": "a,b", "start-cell": "B2"}, "B2"}, {"range alias, single cell", map[string]interface{}{"csv": "a,b", "range": "B2"}, "B2"}, {"range alias collapses to top-left", map[string]interface{}{"csv": "a,b", "range": "A1:H17"}, "A1"}, - {"start-cell wins when both set", map[string]interface{}{"csv": "a,b", "start-cell": "C3", "range": "A1:H17"}, "C3"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -38,6 +37,21 @@ func TestCsvPutInput_RangeAliasForStartCell(t *testing.T) { } } +func TestCsvPutInput_RejectsStartCellAndRangeTogether(t *testing.T) { + fv := newMapFlagViewForCommand("+csv-put", map[string]interface{}{ + "csv": "a,b", + "start-cell": "C3", + "range": "A1:H17", + }) + _, err := csvPutInput(fv, "tok", "sid", "") + if err == nil { + t.Fatal("csvPutInput accepted both start-cell and range; want mutual-exclusion error") + } + if !strings.Contains(err.Error(), "--start-cell and --range are mutually exclusive") { + t.Errorf("error = %q, want it to mention start-cell/range mutual exclusion", err.Error()) + } +} + // With neither --start-cell nor --range explicitly set, csvPutInput rejects the // call instead of silently anchoring at the "A1" flag default. Standalone never // reaches this path — cobra's MarkFlagsOneRequired(start-cell, range) catches it diff --git a/shortcuts/sheets/execute_paths_test.go b/shortcuts/sheets/execute_paths_test.go index 8cd24bb256..c04a8caabf 100644 --- a/shortcuts/sheets/execute_paths_test.go +++ b/shortcuts/sheets/execute_paths_test.go @@ -5,11 +5,12 @@ package sheets import ( "encoding/json" + "errors" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" ) // TestExecute_WorkbookInfo_Happy stubs the invoke_read endpoint and @@ -453,16 +454,27 @@ func TestExecute_WorkbookCreate_FillFailureKeepsToken(t *testing.T) { if err == nil { t.Fatalf("expected a partial-success error; got nil\nout=%s", out) } - exitErr, ok := err.(*output.ExitError) + p, ok := errs.ProblemOf(err) if !ok { - t.Fatalf("error type = %T, want *output.ExitError (structured)", err) + t.Fatalf("error type = %T, want typed problem", err) } - if exitErr.Detail == nil { - t.Fatal("ExitError.Detail is nil; want structured detail carrying the token") + if p.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("subtype = %q, want failed_precondition (the spreadsheet exists; caller must change state, not retry)", p.Subtype) } - detail, _ := exitErr.Detail.Detail.(map[string]interface{}) - if detail["spreadsheet_token"] != "shtNEW" { - t.Errorf("detail.spreadsheet_token = %v, want shtNEW (must survive the fill failure)", detail["spreadsheet_token"]) + if !strings.Contains(p.Message, "shtNEW") { + t.Errorf("message = %q, want spreadsheet token for recovery", p.Message) + } + if !strings.Contains(p.Hint, "spreadsheet_token") { + t.Errorf("hint = %q, want recovery guidance naming spreadsheet_token", p.Hint) + } + // The underlying fill failure is preserved as the cause so its subtype and + // log_id stay diagnosable rather than being flattened into the message. + inner := errors.Unwrap(err) + if inner == nil { + t.Fatalf("expected the underlying fill failure preserved as the cause") + } + if ip, ok := errs.ProblemOf(inner); !ok || ip.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("cause = %v, want the underlying invalid_response failure preserved for diagnosis", inner) } } diff --git a/shortcuts/sheets/flag_schema_validate.go b/shortcuts/sheets/flag_schema_validate.go index fb3299f2cd..701a859a1a 100644 --- a/shortcuts/sheets/flag_schema_validate.go +++ b/shortcuts/sheets/flag_schema_validate.go @@ -8,8 +8,6 @@ import ( "fmt" "sort" "strings" - - "github.com/larksuite/cli/shortcuts/common" ) // ─── schema-driven flag validation ──────────────────────────────────── @@ -95,7 +93,7 @@ func validateValueAgainstSchema(fv flagView, name string, value interface{}) err var schema schemaProperty json.Unmarshal(raw, &schema) if vErr := validateAgainstSchema(value, &schema, ""); vErr != nil { - return common.FlagErrorf("--%s: %s", name, vErr.Error()) + return sheetsValidationForFlag(name, "--%s: %s", name, vErr.Error()) } return nil } diff --git a/shortcuts/sheets/helpers.go b/shortcuts/sheets/helpers.go index 3465484cb0..621f214eb4 100644 --- a/shortcuts/sheets/helpers.go +++ b/shortcuts/sheets/helpers.go @@ -12,20 +12,52 @@ import ( "encoding/json" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) +func sheetsFlagParam(name string) string { + if strings.HasPrefix(name, "--") { + return name + } + return "--" + name +} + +func sheetsInvalidParam(name, reason string) errs.InvalidParam { + return errs.InvalidParam{Name: sheetsFlagParam(name), Reason: reason} +} + +func sheetsValidationForFlag(name, format string, args ...any) *errs.ValidationError { + return common.ValidationErrorf(format, args...).WithParam(sheetsFlagParam(name)) +} + +func sheetsValidationCauseForFlag(name string, cause error) *errs.ValidationError { + return common.ValidationErrorf("%v", cause).WithParam(sheetsFlagParam(name)).WithCause(cause) +} + +// sheetsInputStatError wraps a local input-file stat/open failure as a typed +// validation error tagged with the flag the path came from, so callers learn +// which flag to fix. It reuses the shared common.WrapInputStatErrorTyped +// classification and only adds the domain's flag param. +func sheetsInputStatError(flag string, err error) error { + wrapped := common.WrapInputStatErrorTyped(err) + if v, ok := wrapped.(*errs.ValidationError); ok { + return v.WithParam(sheetsFlagParam(flag)) + } + return wrapped +} + // resolveSpreadsheetToken applies the public --url / --spreadsheet-token XOR // pair shared by every sheets canonical shortcut and returns the resolved // token. Network-free, safe to call from Validate and DryRun. func resolveSpreadsheetToken(runtime *common.RuntimeContext) (string, error) { - if err := common.ExactlyOne(runtime, "url", "spreadsheet-token"); err != nil { + if err := common.ExactlyOneTyped(runtime, "url", "spreadsheet-token"); err != nil { return "", err } if token := strings.TrimSpace(runtime.Str("spreadsheet-token")); token != "" { if err := validate.RejectControlChars(token, "spreadsheet-token"); err != nil { - return "", common.FlagErrorf("%v", err) + return "", sheetsValidationCauseForFlag("spreadsheet-token", err) } return token, nil } @@ -33,10 +65,10 @@ func resolveSpreadsheetToken(runtime *common.RuntimeContext) (string, error) { url := strings.TrimSpace(runtime.Str("url")) token := extractSpreadsheetToken(url) if token == "" || token == url { - return "", common.FlagErrorf("--url must be a spreadsheet URL like https://.../sheets/") + return "", sheetsValidationForFlag("url", "--url must be a spreadsheet URL like https://.../sheets/") } if err := validate.RejectControlChars(token, "url"); err != nil { - return "", common.FlagErrorf("%v", err) + return "", sheetsValidationCauseForFlag("url", err) } return token, nil } @@ -64,18 +96,18 @@ func extractSpreadsheetToken(input string) string { // Returned tuple: (sheetID, sheetName). Exactly one is non-empty — callers // pass both through to the tool input; the server picks whichever fits. func resolveSheetSelector(runtime *common.RuntimeContext) (sheetID, sheetName string, err error) { - if err := common.ExactlyOne(runtime, "sheet-id", "sheet-name"); err != nil { + if err := common.ExactlyOneTyped(runtime, "sheet-id", "sheet-name"); err != nil { return "", "", err } if id := strings.TrimSpace(runtime.Str("sheet-id")); id != "" { if err := validate.RejectControlChars(id, "sheet-id"); err != nil { - return "", "", common.FlagErrorf("%v", err) + return "", "", sheetsValidationCauseForFlag("sheet-id", err) } return id, "", nil } name := strings.TrimSpace(runtime.Str("sheet-name")) if err := validate.RejectControlChars(name, "sheet-name"); err != nil { - return "", "", common.FlagErrorf("%v", err) + return "", "", sheetsValidationCauseForFlag("sheet-name", err) } return "", name, nil } @@ -116,18 +148,26 @@ func requireSheetSelector(sheetID, sheetName string) error { sheetID = strings.TrimSpace(sheetID) sheetName = strings.TrimSpace(sheetName) if sheetID == "" && sheetName == "" { - return common.FlagErrorf("specify at least one of --sheet-id or --sheet-name") + return common.ValidationErrorf("specify at least one of --sheet-id or --sheet-name"). + WithParams( + sheetsInvalidParam("sheet-id", "required; specify at least one"), + sheetsInvalidParam("sheet-name", "required; specify at least one"), + ) } if sheetID != "" && sheetName != "" { - return common.FlagErrorf("--sheet-id and --sheet-name are mutually exclusive") + return common.ValidationErrorf("--sheet-id and --sheet-name are mutually exclusive"). + WithParams( + sheetsInvalidParam("sheet-id", "mutually exclusive"), + sheetsInvalidParam("sheet-name", "mutually exclusive"), + ) } if sheetID != "" { if err := validate.RejectControlChars(sheetID, "sheet-id"); err != nil { - return common.FlagErrorf("%v", err) + return sheetsValidationCauseForFlag("sheet-id", err) } } else { if err := validate.RejectControlChars(sheetName, "sheet-name"); err != nil { - return common.FlagErrorf("%v", err) + return sheetsValidationCauseForFlag("sheet-name", err) } } return nil @@ -152,15 +192,19 @@ func optionalSheetSelector(sheetID, sheetName, idFlagName, nameFlagName string) sheetID = strings.TrimSpace(sheetID) sheetName = strings.TrimSpace(sheetName) if sheetID != "" && sheetName != "" { - return common.FlagErrorf("--%s and --%s are mutually exclusive", idFlagName, nameFlagName) + return common.ValidationErrorf("--%s and --%s are mutually exclusive", idFlagName, nameFlagName). + WithParams( + sheetsInvalidParam(idFlagName, "mutually exclusive"), + sheetsInvalidParam(nameFlagName, "mutually exclusive"), + ) } if sheetID != "" { if err := validate.RejectControlChars(sheetID, idFlagName); err != nil { - return common.FlagErrorf("%v", err) + return sheetsValidationCauseForFlag(idFlagName, err) } } else if sheetName != "" { if err := validate.RejectControlChars(sheetName, nameFlagName); err != nil { - return common.FlagErrorf("%v", err) + return sheetsValidationCauseForFlag(nameFlagName, err) } } return nil @@ -197,7 +241,7 @@ func parseJSONFlag(runtime flagView, name string) (interface{}, error) { } var out interface{} if err := json.Unmarshal([]byte(raw), &out); err != nil { - return nil, common.FlagErrorf("--%s: invalid JSON: %v", name, err) + return nil, sheetsValidationForFlag(name, "--%s: invalid JSON: %v", name, err).WithCause(err) } // Schema-driven flag validation at the user-input boundary. Skips // --properties (validated at the input-builder tail after enhance @@ -216,11 +260,11 @@ func requireJSONObject(runtime flagView, name string) (map[string]interface{}, e return nil, err } if v == nil { - return nil, common.FlagErrorf("--%s is required", name) + return nil, sheetsValidationForFlag(name, "--%s is required", name) } m, ok := v.(map[string]interface{}) if !ok { - return nil, common.FlagErrorf("--%s must be a JSON object", name) + return nil, sheetsValidationForFlag(name, "--%s must be a JSON object", name) } return m, nil } @@ -232,11 +276,11 @@ func requireJSONArray(runtime flagView, name string) ([]interface{}, error) { return nil, err } if v == nil { - return nil, common.FlagErrorf("--%s is required", name) + return nil, sheetsValidationForFlag(name, "--%s is required", name) } a, ok := v.([]interface{}) if !ok { - return nil, common.FlagErrorf("--%s must be a JSON array", name) + return nil, sheetsValidationForFlag(name, "--%s must be a JSON array", name) } return a, nil } @@ -293,7 +337,7 @@ func borderStylesFromFlag(runtime flagView) (map[string]interface{}, error) { } m, ok := v.(map[string]interface{}) if !ok { - return nil, common.FlagErrorf("--border-styles must be a JSON object") + return nil, sheetsValidationForFlag("border-styles", "--border-styles must be a JSON object") } return m, nil } @@ -307,5 +351,10 @@ func requireAnyStyleFlag(runtime flagView) error { if runtime.Str("border-styles") != "" { return nil } - return common.FlagErrorf("at least one style flag is required (e.g. --background-color, --font-weight, --border-styles)") + return common.ValidationErrorf("at least one style flag is required (e.g. --background-color, --font-weight, --border-styles)"). + WithParams( + sheetsInvalidParam("background-color", "required; specify at least one style flag"), + sheetsInvalidParam("font-weight", "required; specify at least one style flag"), + sheetsInvalidParam("border-styles", "required; specify at least one style flag"), + ) } diff --git a/shortcuts/sheets/helpers_test.go b/shortcuts/sheets/helpers_test.go index 15eb06166d..686676955c 100644 --- a/shortcuts/sheets/helpers_test.go +++ b/shortcuts/sheets/helpers_test.go @@ -6,11 +6,13 @@ package sheets 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" @@ -79,6 +81,71 @@ func runShortcutWithStubs(t *testing.T, sc common.Shortcut, args []string, stubs return stdout.String(), err } +func TestSheetHelpersValidationMetadata(t *testing.T) { + t.Parallel() + + t.Run("missing sheet selector reports both params", func(t *testing.T) { + t.Parallel() + err := requireSheetSelector("", "") + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want *errs.ValidationError", err, err) + } + if len(validationErr.Params) != 2 { + t.Fatalf("params = %#v, want two structured params", validationErr.Params) + } + if validationErr.Params[0].Name != "--sheet-id" || validationErr.Params[1].Name != "--sheet-name" { + t.Fatalf("params = %#v, want --sheet-id/--sheet-name", validationErr.Params) + } + }) + + t.Run("spreadsheet url shape reports url param", func(t *testing.T) { + t.Parallel() + cmd := &cobra.Command{Use: "sheets"} + cmd.Flags().String("url", "not-a-sheet-url", "") + cmd.Flags().String("spreadsheet-token", "", "") + _, err := resolveSpreadsheetToken(common.TestNewRuntimeContext(cmd, testConfig(t))) + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want *errs.ValidationError", err, err) + } + if validationErr.Param != "--url" { + t.Fatalf("param = %q, want --url", validationErr.Param) + } + }) + + t.Run("sheet selector control char keeps param and cause", func(t *testing.T) { + t.Parallel() + err := requireSheetSelector("bad\x00id", "") + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want *errs.ValidationError", err, err) + } + if validationErr.Param != "--sheet-id" { + t.Fatalf("param = %q, want --sheet-id", validationErr.Param) + } + if validationErr.Unwrap() == nil { + t.Fatalf("expected control-char validation cause to be preserved") + } + }) + + t.Run("invalid json flag keeps param and cause", func(t *testing.T) { + t.Parallel() + fv := newMapFlagViewForCommand("+cells-set", map[string]interface{}{"cells": "{"}) + _, err := parseJSONFlag(fv, "cells") + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("error = %T %v, want *errs.ValidationError", err, err) + } + if validationErr.Param != "--cells" { + t.Fatalf("param = %q, want --cells", validationErr.Param) + } + if validationErr.Unwrap() == nil { + t.Fatalf("expected JSON parse cause to be preserved") + } + }) +} + // parseDryRunBody runs the shortcut in --dry-run and returns the first // api call's body. The dry-run output format is: // diff --git a/shortcuts/sheets/lark_sheet_batch_update.go b/shortcuts/sheets/lark_sheet_batch_update.go index 696c40f239..ad58db0f33 100644 --- a/shortcuts/sheets/lark_sheet_batch_update.go +++ b/shortcuts/sheets/lark_sheet_batch_update.go @@ -132,7 +132,7 @@ func parseBatchOperationsFlag(runtime *common.RuntimeContext) ([]interface{}, er return nil, err } if v == nil { - return nil, common.FlagErrorf("--operations is required") + return nil, sheetsValidationForFlag("operations", "--operations is required") } if arr, ok := v.([]interface{}); ok { return arr, nil @@ -142,7 +142,7 @@ func parseBatchOperationsFlag(runtime *common.RuntimeContext) ([]interface{}, er return ops, nil } } - return nil, common.FlagErrorf("--operations must be a JSON array (or { operations: [...] } envelope)") + return nil, sheetsValidationForFlag("operations", "--operations must be a JSON array (or { operations: [...] } envelope)") } // CellsBatchSetStyle stamps one style block across many sheet-prefixed @@ -222,7 +222,7 @@ func cellsBatchSetStyleInput(runtime *common.RuntimeContext, token string) (map[ } rows, cols, err := rangeDimensions(sub) if err != nil { - return nil, common.FlagErrorf("range %q: %v", rng, err) + return nil, sheetsValidationForFlag("range", "range %q: %v", rng, err) } cells := fillCellsMatrix(rows, cols, prototype) ops = append(ops, map[string]interface{}{ @@ -386,7 +386,7 @@ var DropdownDelete = common.Shortcut{ return err } if len(ranges) > 100 { - return common.FlagErrorf("--ranges accepts at most 100 entries; got %d", len(ranges)) + return sheetsValidationForFlag("ranges", "--ranges accepts at most 100 entries; got %d", len(ranges)) } return nil }, @@ -439,7 +439,7 @@ func dropdownBatchInput(runtime *common.RuntimeContext, token string, clear bool } rows, cols, err := rangeDimensions(sub) if err != nil { - return nil, common.FlagErrorf("range %q: %v", rng, err) + return nil, sheetsValidationForFlag("range", "range %q: %v", rng, err) } cells := fillCellsMatrix(rows, cols, prototype) ops = append(ops, map[string]interface{}{ @@ -471,21 +471,21 @@ func validateDropdownRanges(runtime *common.RuntimeContext) ([]string, error) { for i, v := range raw { s, ok := v.(string) if !ok { - return nil, common.FlagErrorf("--ranges[%d] must be a string", i) + return nil, sheetsValidationForFlag("ranges", "--ranges[%d] must be a string", i) } s = strings.TrimSpace(s) if !strings.Contains(s, "!") { - return nil, common.FlagErrorf("--ranges[%d] (%q) must include a sheet prefix", i, s) + return nil, sheetsValidationForFlag("ranges", "--ranges[%d] (%q) must include a sheet prefix", i, s) } // Validate the sheet!range shape up front so malformed entries like // "!A1" (no sheet), "Sheet1!" (no range) or "Sheet1!bad" (bad ref) fail // here at Validate instead of slipping through to DryRun/Execute. _, sub, err := splitSheetPrefixedRange(s) if err != nil { - return nil, common.FlagErrorf("--ranges[%d]: %v", i, err) + return nil, sheetsValidationForFlag("ranges", "--ranges[%d]: %v", i, err) } if _, _, err := rangeDimensions(sub); err != nil { - return nil, common.FlagErrorf("--ranges[%d] (%q): %v", i, s, err) + return nil, sheetsValidationForFlag("ranges", "--ranges[%d] (%q): %v", i, s, err) } out = append(out, s) } @@ -496,7 +496,7 @@ func validateDropdownRanges(runtime *common.RuntimeContext) ([]string, error) { func splitSheetPrefixedRange(rng string) (sheet, sub string, err error) { idx := strings.Index(rng, "!") if idx <= 0 || idx == len(rng)-1 { - return "", "", common.FlagErrorf("range %q must use sheet!range form", rng) + return "", "", sheetsValidationForFlag("range", "range %q must use sheet!range form", rng) } return strings.TrimSpace(rng[:idx]), strings.TrimSpace(rng[idx+1:]), nil } diff --git a/shortcuts/sheets/lark_sheet_object_crud.go b/shortcuts/sheets/lark_sheet_object_crud.go index 221b9d03d9..410be3d870 100644 --- a/shortcuts/sheets/lark_sheet_object_crud.go +++ b/shortcuts/sheets/lark_sheet_object_crud.go @@ -9,7 +9,7 @@ import ( "path/filepath" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -251,7 +251,7 @@ func objectUpdateInput(runtime flagView, token, sheetID, sheetName string, spec return nil, err } if spec.idFlag != "" && strings.TrimSpace(runtime.Str(spec.idFlag)) == "" { - return nil, common.FlagErrorf("--%s is required", spec.idFlag) + return nil, sheetsValidationForFlag(spec.idFlag, "--%s is required", spec.idFlag) } props, err := requireJSONObject(runtime, "properties") if err != nil { @@ -335,7 +335,7 @@ func objectDeleteInput(runtime flagView, token, sheetID, sheetName string, spec return nil, err } if spec.idFlag != "" && strings.TrimSpace(runtime.Str(spec.idFlag)) == "" { - return nil, common.FlagErrorf("--%s is required", spec.idFlag) + return nil, sheetsValidationForFlag(spec.idFlag, "--%s is required", spec.idFlag) } input := map[string]interface{}{ "excel_id": token, @@ -517,16 +517,16 @@ func validateSparklineUpdateItems(input map[string]interface{}) error { } arr, ok := raw.([]interface{}) if !ok { - return common.FlagErrorf("+sparkline-update properties.sparklines must be an array") + return sheetsValidationForFlag("properties", "+sparkline-update properties.sparklines must be an array") } for i, item := range arr { m, _ := item.(map[string]interface{}) if m == nil { - return common.FlagErrorf("+sparkline-update properties.sparklines[%d] must be an object", i) + return sheetsValidationForFlag("properties", "+sparkline-update properties.sparklines[%d] must be an object", i) } id, _ := m["sparkline_id"].(string) if strings.TrimSpace(id) == "" { - return common.FlagErrorf("+sparkline-update properties.sparklines[%d] missing sparkline_id (run `+sparkline-list --group-id ` first to read sparkline_id for each item, then echo each id back on the corresponding update entry)", i) + return sheetsValidationForFlag("properties", "+sparkline-update properties.sparklines[%d] missing sparkline_id (run `+sparkline-list --group-id ` first to read sparkline_id for each item, then echo each id back on the corresponding update entry)", i) } } return nil @@ -595,20 +595,44 @@ func floatImageProperties(runtime flagView, uploadedImageToken string, requireIm } } if set == 0 && requireImageSource { - return nil, common.FlagErrorf("one of --image, --image-token, or --image-uri is required") + return nil, common.ValidationErrorf("one of --image, --image-token, or --image-uri is required").WithParams(sheetsInvalidParam("image", "required; specify one"), sheetsInvalidParam("image-token", "required; specify one"), sheetsInvalidParam("image-uri", "required; specify one")) } if set > 1 { - return nil, common.FlagErrorf("--image, --image-token, and --image-uri are mutually exclusive") + params := make([]errs.InvalidParam, 0, 3) + if img != "" { + params = append(params, sheetsInvalidParam("image", "mutually exclusive")) + } + if token != "" { + params = append(params, sheetsInvalidParam("image-token", "mutually exclusive")) + } + if uri != "" { + params = append(params, sheetsInvalidParam("image-uri", "mutually exclusive")) + } + return nil, common.ValidationErrorf("--image, --image-token, and --image-uri are mutually exclusive").WithParams(params...) } name := floatImageName(runtime) if name == "" { - return nil, common.FlagErrorf("--image-name is required") + return nil, sheetsValidationForFlag("image-name", "--image-name is required") } if !runtime.Changed("position-row") || !runtime.Changed("position-col") { - return nil, common.FlagErrorf("--position-row and --position-col are required") + params := make([]errs.InvalidParam, 0, 2) + if !runtime.Changed("position-row") { + params = append(params, sheetsInvalidParam("position-row", "required")) + } + if !runtime.Changed("position-col") { + params = append(params, sheetsInvalidParam("position-col", "required")) + } + return nil, common.ValidationErrorf("--position-row and --position-col are required").WithParams(params...) } if !runtime.Changed("size-width") || !runtime.Changed("size-height") { - return nil, common.FlagErrorf("--size-width and --size-height are required") + params := make([]errs.InvalidParam, 0, 2) + if !runtime.Changed("size-width") { + params = append(params, sheetsInvalidParam("size-width", "required")) + } + if !runtime.Changed("size-height") { + params = append(params, sheetsInvalidParam("size-height", "required")) + } + return nil, common.ValidationErrorf("--size-width and --size-height are required").WithParams(params...) } props := map[string]interface{}{ "image_name": name, @@ -626,7 +650,9 @@ func floatImageProperties(runtime flagView, uploadedImageToken string, requireIm // Local file: validate path safety here so --dry-run also rejects // unsafe paths; Execute uploads it and passes the real token in. if _, err := validate.SafeLocalFlagPath("--image", img); err != nil { - return nil, output.ErrValidation("%s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err). + WithParam("--image"). + WithCause(err) } if uploadedImageToken != "" { props["image_token"] = uploadedImageToken @@ -746,7 +772,7 @@ func uploadFloatImageIfLocal(runtime *common.RuntimeContext, spreadsheetToken st } info, err := runtime.FileIO().Stat(img) if err != nil { - return "", common.WrapInputStatError(err) + return "", sheetsInputStatError("image", err) } return common.UploadDriveMediaAll(runtime, common.DriveMediaUploadAllConfig{ FilePath: img, @@ -762,7 +788,7 @@ func floatImageWriteInput(runtime flagView, token, sheetID, sheetName, op string return nil, err } if withIDFlag && strings.TrimSpace(runtime.Str("float-image-id")) == "" { - return nil, common.FlagErrorf("--float-image-id is required") + return nil, sheetsValidationForFlag("float-image-id", "--float-image-id is required") } props, err := floatImageProperties(runtime, uploadedImageToken, op == "create") if err != nil { @@ -882,7 +908,7 @@ func filterCreateInput(runtime flagView, token, sheetID, sheetName string) (map[ return nil, err } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } props := map[string]interface{}{ "range": strings.TrimSpace(runtime.Str("range")), @@ -957,10 +983,10 @@ func filterUpdateInput(runtime flagView, token, sheetID, sheetName string) (map[ return nil, err } if sheetID == "" { - return nil, common.FlagErrorf("+filter-update requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)") + return nil, sheetsValidationForFlag("sheet-id", "+filter-update requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)") } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } props, err := requireJSONObject(runtime, "properties") if err != nil { @@ -1031,7 +1057,7 @@ func filterDeleteInput(runtime flagView, token, sheetID, sheetName string) (map[ return nil, err } if sheetID == "" { - return nil, common.FlagErrorf("+filter-delete requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)") + return nil, sheetsValidationForFlag("sheet-id", "+filter-delete requires --sheet-id (filter_id must equal sheet_id; --sheet-name needs a network lookup unavailable here — call +workbook-info first or pass --sheet-id directly)") } input := map[string]interface{}{ "excel_id": token, diff --git a/shortcuts/sheets/lark_sheet_range_operations.go b/shortcuts/sheets/lark_sheet_range_operations.go index 669e5eaf52..188fdfe73f 100644 --- a/shortcuts/sheets/lark_sheet_range_operations.go +++ b/shortcuts/sheets/lark_sheet_range_operations.go @@ -5,10 +5,9 @@ package sheets import ( "context" - "errors" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -76,7 +75,7 @@ func cellsClearInput(runtime flagView, token, sheetID, sheetName string) (map[st return nil, err } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } input := map[string]interface{}{ "excel_id": token, @@ -108,22 +107,22 @@ func normalizeClearType(scope string) string { // pivot-occupied A1 with cells-clear; point the agent at the object's own // delete command instead. Non-matching errors pass through untouched. func annotateEmbeddedBlockClearErr(err error) error { - var ee *output.ExitError - if !errors.As(err, &ee) || ee.Detail == nil { + p, ok := errs.ProblemOf(err) + if !ok { return err } - if !strings.Contains(strings.ToLower(ee.Detail.Message), "embedded block") { + if !strings.Contains(strings.ToLower(p.Message), "embedded block") { return err } const hint = "the range overlaps an embedded object (pivot table / chart); " + "cells-clear only clears cell values/formats and cannot delete it — " + "delete the object with its own command (+pivot-delete / +chart-delete; find the id via +pivot-list / +chart-list)" - if ee.Detail.Hint == "" { - ee.Detail.Hint = hint + if p.Hint == "" { + p.Hint = hint } else { - ee.Detail.Hint += "; " + hint + p.Hint += "; " + hint } - return ee + return err } // CellsMerge / CellsUnmerge share the merge_cells tool, dispatched by the @@ -191,7 +190,7 @@ func mergeInput(runtime flagView, token, sheetID, sheetName, op string, withMerg return nil, err } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } input := map[string]interface{}{ "excel_id": token, @@ -345,36 +344,36 @@ func resizeInput(runtime flagView, token, sheetID, sheetName, dimension string) return nil, err } if !runtime.Changed("range") { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } rangeStr := strings.TrimSpace(runtime.Str("range")) parsedDim, _, _, err := parseA1Range(rangeStr) if err != nil { - return nil, common.FlagErrorf("invalid --range %q: %v", rangeStr, err) + return nil, sheetsValidationForFlag("range", "invalid --range %q: %v", rangeStr, err) } if parsedDim != dimension { want := "row numbers (e.g. \"2:10\")" if dimension == "column" { want = "column letters (e.g. \"A:E\")" } - return nil, common.FlagErrorf("--range %q is a %s range; %s expects %s", rangeStr, parsedDim, commandForDimension(dimension), want) + return nil, sheetsValidationForFlag("range", "--range %q is a %s range; %s expects %s", rangeStr, parsedDim, commandForDimension(dimension), want) } if !strings.Contains(rangeStr, ":") { rangeStr = rangeStr + ":" + rangeStr } typ := strings.TrimSpace(runtime.Str("type")) if typ == "" { - return nil, common.FlagErrorf("--type is required (pixel / standard%s)", autoSuffix(dimension)) + return nil, sheetsValidationForFlag("type", "--type is required (pixel / standard%s)", autoSuffix(dimension)) } if dimension == "column" && typ == "auto" { - return nil, common.FlagErrorf("--type auto is rows-only (column widths do not support auto-fit); use +rows-resize") + return nil, sheetsValidationForFlag("type", "--type auto is rows-only (column widths do not support auto-fit); use +rows-resize") } hasSize := runtime.Changed("size") && runtime.Int("size") > 0 if typ == "pixel" && !hasSize { - return nil, common.FlagErrorf("--type pixel requires --size ") + return nil, common.ValidationErrorf("--type pixel requires --size ").WithParams(sheetsInvalidParam("type", "required"), sheetsInvalidParam("size", "required")) } if typ != "pixel" && hasSize { - return nil, common.FlagErrorf("--size is only valid with --type pixel") + return nil, common.ValidationErrorf("--size is only valid with --type pixel").WithParams(sheetsInvalidParam("size", "mutually exclusive"), sheetsInvalidParam("type", "mutually exclusive")) } input := map[string]interface{}{ "excel_id": token, @@ -567,10 +566,10 @@ func transformMoveCopyInput(runtime flagView, token, sheetID, sheetName, op stri return nil, err } if strings.TrimSpace(runtime.Str("source-range")) == "" { - return nil, common.FlagErrorf("--source-range is required") + return nil, sheetsValidationForFlag("source-range", "--source-range is required") } if strings.TrimSpace(runtime.Str("target-range")) == "" { - return nil, common.FlagErrorf("--target-range is required") + return nil, sheetsValidationForFlag("target-range", "--target-range is required") } input := map[string]interface{}{ "excel_id": token, @@ -609,10 +608,10 @@ func rangeFillInput(runtime flagView, token, sheetID, sheetName string) (map[str return nil, err } if strings.TrimSpace(runtime.Str("source-range")) == "" { - return nil, common.FlagErrorf("--source-range is required") + return nil, sheetsValidationForFlag("source-range", "--source-range is required") } if strings.TrimSpace(runtime.Str("target-range")) == "" { - return nil, common.FlagErrorf("--target-range is required") + return nil, sheetsValidationForFlag("target-range", "--target-range is required") } input := map[string]interface{}{ "excel_id": token, @@ -641,7 +640,7 @@ func rangeSortInput(runtime flagView, token, sheetID, sheetName string) (map[str return nil, err } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } // requireJSONArray runs the embedded JSON Schema for --sort-keys // via parseJSONFlag → validateParsedJSONFlag, so each item is diff --git a/shortcuts/sheets/lark_sheet_range_operations_test.go b/shortcuts/sheets/lark_sheet_range_operations_test.go index e0f4647090..0c85bbb667 100644 --- a/shortcuts/sheets/lark_sheet_range_operations_test.go +++ b/shortcuts/sheets/lark_sheet_range_operations_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -16,34 +16,35 @@ func TestAnnotateEmbeddedBlockClearErr(t *testing.T) { t.Parallel() t.Run("adds pivot-delete hint on embedded-block error", func(t *testing.T) { - in := &output.ExitError{Code: output.ExitAPI, Detail: &output.ErrDetail{ - Type: "api", - Message: `tool "clear_cell_range" failed: [500] can not find embedded block`, - }} - var ee *output.ExitError - if !errors.As(annotateEmbeddedBlockClearErr(in), &ee) || ee.Detail == nil { - t.Fatal("expected ExitError with detail") + in := errs.NewAPIError(errs.SubtypeServerError, `tool "clear_cell_range" failed: [500] can not find embedded block`) + p, ok := errs.ProblemOf(annotateEmbeddedBlockClearErr(in)) + if !ok { + t.Fatal("expected typed problem") } - if !strings.Contains(ee.Detail.Hint, "+pivot-delete") { - t.Errorf("hint should point at +pivot-delete, got %q", ee.Detail.Hint) + if !strings.Contains(p.Hint, "+pivot-delete") { + t.Errorf("hint should point at +pivot-delete, got %q", p.Hint) } }) t.Run("appends to existing hint", func(t *testing.T) { - in := &output.ExitError{Code: output.ExitAPI, Detail: &output.ErrDetail{ - Message: "embedded block missing", Hint: "preexisting", - }} - out := annotateEmbeddedBlockClearErr(in).(*output.ExitError) - if !strings.HasPrefix(out.Detail.Hint, "preexisting; ") { - t.Errorf("existing hint should be preserved and appended, got %q", out.Detail.Hint) + in := errs.NewAPIError(errs.SubtypeServerError, "embedded block missing").WithHint("preexisting") + p, ok := errs.ProblemOf(annotateEmbeddedBlockClearErr(in)) + if !ok { + t.Fatal("expected typed problem") + } + if !strings.HasPrefix(p.Hint, "preexisting; ") { + t.Errorf("existing hint should be preserved and appended, got %q", p.Hint) } }) - t.Run("passes through unrelated ExitError untouched", func(t *testing.T) { - in := &output.ExitError{Code: output.ExitAPI, Detail: &output.ErrDetail{Message: "some other failure"}} - out := annotateEmbeddedBlockClearErr(in).(*output.ExitError) - if out.Detail.Hint != "" { - t.Errorf("unrelated error should not gain a hint, got %q", out.Detail.Hint) + t.Run("passes through unrelated typed error untouched", func(t *testing.T) { + in := errs.NewAPIError(errs.SubtypeServerError, "some other failure") + p, ok := errs.ProblemOf(annotateEmbeddedBlockClearErr(in)) + if !ok { + t.Fatal("expected typed problem") + } + if p.Hint != "" { + t.Errorf("unrelated error should not gain a hint, got %q", p.Hint) } }) diff --git a/shortcuts/sheets/lark_sheet_read_data.go b/shortcuts/sheets/lark_sheet_read_data.go index 40044c94ac..034e541bf0 100644 --- a/shortcuts/sheets/lark_sheet_read_data.go +++ b/shortcuts/sheets/lark_sheet_read_data.go @@ -49,7 +49,7 @@ var CellsGet = common.Shortcut{ return err } if strings.TrimSpace(runtime.Str("range")) == "" { - return common.FlagErrorf("--range is required") + return sheetsValidationForFlag("range", "--range is required") } return nil }, @@ -142,7 +142,7 @@ var CsvGet = common.Shortcut{ return err } if strings.TrimSpace(runtime.Str("range")) == "" { - return common.FlagErrorf("--range is required") + return sheetsValidationForFlag("range", "--range is required") } return nil }, @@ -484,7 +484,7 @@ var DropdownGet = common.Shortcut{ return err } if strings.TrimSpace(runtime.Str("range")) == "" { - return common.FlagErrorf("--range is required") + return sheetsValidationForFlag("range", "--range is required") } return nil }, diff --git a/shortcuts/sheets/lark_sheet_search_replace.go b/shortcuts/sheets/lark_sheet_search_replace.go index 6e0b8ecb36..4777d24547 100644 --- a/shortcuts/sheets/lark_sheet_search_replace.go +++ b/shortcuts/sheets/lark_sheet_search_replace.go @@ -36,7 +36,7 @@ var CellsSearch = common.Shortcut{ return err } if strings.TrimSpace(runtime.Str("find")) == "" { - return common.FlagErrorf("--find is required") + return sheetsValidationForFlag("find", "--find is required") } return nil }, @@ -151,10 +151,10 @@ func replaceInput(runtime flagView, token, sheetID, sheetName string) (map[strin return nil, err } if strings.TrimSpace(runtime.Str("find")) == "" { - return nil, common.FlagErrorf("--find is required") + return nil, sheetsValidationForFlag("find", "--find is required") } if !runtime.Changed("replacement") { - return nil, common.FlagErrorf("--replacement is required (pass an empty string to delete matches)") + return nil, sheetsValidationForFlag("replacement", "--replacement is required (pass an empty string to delete matches)") } input := map[string]interface{}{ "excel_id": token, diff --git a/shortcuts/sheets/lark_sheet_sheet_structure.go b/shortcuts/sheets/lark_sheet_sheet_structure.go index fcdd9667a0..41228578ca 100644 --- a/shortcuts/sheets/lark_sheet_sheet_structure.go +++ b/shortcuts/sheets/lark_sheet_sheet_structure.go @@ -164,18 +164,18 @@ func dimInsertInput(runtime flagView, token, sheetID, sheetName string) (map[str return nil, err } if !runtime.Changed("position") { - return nil, common.FlagErrorf("--position is required") + return nil, sheetsValidationForFlag("position", "--position is required") } if !runtime.Changed("count") { - return nil, common.FlagErrorf("--count is required") + return nil, sheetsValidationForFlag("count", "--count is required") } position := strings.TrimSpace(runtime.Str("position")) if _, _, err := parseA1Position(position); err != nil { - return nil, common.FlagErrorf("invalid --position %q: %v", position, err) + return nil, sheetsValidationForFlag("position", "invalid --position %q: %v", position, err) } count := runtime.Int("count") if count <= 0 { - return nil, common.FlagErrorf("--count must be > 0 (got %d)", count) + return nil, sheetsValidationForFlag("count", "--count must be > 0 (got %d)", count) } input := map[string]interface{}{ "excel_id": token, @@ -326,13 +326,13 @@ func dimFreezeInput(runtime flagView, token, sheetID, sheetName string) (map[str return nil, err } if !runtime.Changed("dimension") { - return nil, common.FlagErrorf("--dimension is required") + return nil, sheetsValidationForFlag("dimension", "--dimension is required") } if !runtime.Changed("count") { - return nil, common.FlagErrorf("--count is required (0 unfreezes)") + return nil, sheetsValidationForFlag("count", "--count is required (0 unfreezes)") } if runtime.Int("count") < 0 { - return nil, common.FlagErrorf("--count must be >= 0") + return nil, sheetsValidationForFlag("count", "--count must be >= 0") } dim := runtime.Str("dimension") count := runtime.Int("count") @@ -361,11 +361,11 @@ func dimRangeOpInput(runtime flagView, token, sheetID, sheetName, op string) (ma return nil, err } if !runtime.Changed("range") { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } rangeStr := strings.TrimSpace(runtime.Str("range")) if _, _, _, err := parseA1Range(rangeStr); err != nil { - return nil, common.FlagErrorf("invalid --range %q: %v", rangeStr, err) + return nil, sheetsValidationForFlag("range", "invalid --range %q: %v", rangeStr, err) } input := map[string]interface{}{ "excel_id": token, @@ -611,7 +611,7 @@ var DimMove = common.Shortcut{ } sheetID = lookedID } - data, err := runtime.CallAPI("POST", dimMovePath(token, sheetID), nil, dimMoveBody(runtime)) + data, err := runtime.CallAPITyped("POST", dimMovePath(token, sheetID), nil, dimMoveBody(runtime)) if err != nil { return err } @@ -632,20 +632,20 @@ type dimMovePlan struct { // target dimension matches the source. Used by both Validate and Execute. func buildDimMovePlan(runtime flagView) (*dimMovePlan, error) { if !runtime.Changed("source-range") || !runtime.Changed("target") { - return nil, common.FlagErrorf("--source-range and --target are required") + return nil, common.ValidationErrorf("--source-range and --target are required").WithParams(sheetsInvalidParam("source-range", "required"), sheetsInvalidParam("target", "required")) } src := strings.TrimSpace(runtime.Str("source-range")) dim, startIdx, endIdx, err := parseA1Range(src) if err != nil { - return nil, common.FlagErrorf("invalid --source-range %q: %v", src, err) + return nil, sheetsValidationForFlag("source-range", "invalid --source-range %q: %v", src, err) } tgt := strings.TrimSpace(runtime.Str("target")) tgtDim, tgtIdx, err := parseA1Position(tgt) if err != nil { - return nil, common.FlagErrorf("invalid --target %q: %v", tgt, err) + return nil, sheetsValidationForFlag("target", "invalid --target %q: %v", tgt, err) } if tgtDim != dim { - return nil, common.FlagErrorf("--target %q dimension (%s) must match --source-range %q dimension (%s)", tgt, tgtDim, src, dim) + return nil, common.ValidationErrorf("--target %q dimension (%s) must match --source-range %q dimension (%s)", tgt, tgtDim, src, dim).WithParams(sheetsInvalidParam("target", "dimension mismatch"), sheetsInvalidParam("source-range", "dimension mismatch")) } return &dimMovePlan{dimension: dim, startIdx: startIdx, endIdx: endIdx, targetIdx: tgtIdx}, nil } diff --git a/shortcuts/sheets/lark_sheet_workbook.go b/shortcuts/sheets/lark_sheet_workbook.go index 6ea725b5aa..aed687f361 100644 --- a/shortcuts/sheets/lark_sheet_workbook.go +++ b/shortcuts/sheets/lark_sheet_workbook.go @@ -13,9 +13,9 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/client" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -122,13 +122,13 @@ var SheetCreate = common.Shortcut{ func sheetCreateInput(runtime flagView, token string) (map[string]interface{}, error) { if strings.TrimSpace(runtime.Str("title")) == "" { - return nil, common.FlagErrorf("--title is required") + return nil, sheetsValidationForFlag("title", "--title is required") } if n := runtime.Int("row-count"); n < 0 || n > 50000 { - return nil, common.FlagErrorf("--row-count must be between 0 and 50000") + return nil, sheetsValidationForFlag("row-count", "--row-count must be between 0 and 50000") } if n := runtime.Int("col-count"); n < 0 || n > 200 { - return nil, common.FlagErrorf("--col-count must be between 0 and 200") + return nil, sheetsValidationForFlag("col-count", "--col-count must be between 0 and 200") } input := map[string]interface{}{ "excel_id": token, @@ -167,7 +167,7 @@ func sheetRenameInput(runtime flagView, token, sheetID, sheetName string) (map[s return nil, err } if strings.TrimSpace(runtime.Str("title")) == "" { - return nil, common.FlagErrorf("--title is required") + return nil, sheetsValidationForFlag("title", "--title is required") } input := map[string]interface{}{ "excel_id": token, @@ -192,7 +192,7 @@ func sheetSetTabColorInput(runtime flagView, token, sheetID, sheetName string) ( return nil, err } if !runtime.Changed("color") { - return nil, common.FlagErrorf("--color is required (empty string clears)") + return nil, sheetsValidationForFlag("color", "--color is required (empty string clears)") } input := map[string]interface{}{ "excel_id": token, @@ -311,13 +311,13 @@ var SheetMove = common.Shortcut{ return err } if !runtime.Changed("index") { - return common.FlagErrorf("--index is required") + return sheetsValidationForFlag("index", "--index is required") } if runtime.Int("index") < 0 { - return common.FlagErrorf("--index must be >= 0") + return sheetsValidationForFlag("index", "--index must be >= 0") } if runtime.Changed("source-index") && runtime.Int("source-index") < 0 { - return common.FlagErrorf("--source-index must be >= 0") + return sheetsValidationForFlag("source-index", "--source-index must be >= 0") } return nil }, @@ -561,7 +561,7 @@ var WorkbookCreate = common.Shortcut{ Flags: flagsFor("+workbook-create"), Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("title")) == "" { - return common.FlagErrorf("--title is required") + return sheetsValidationForFlag("title", "--title is required") } if runtime.Str("headers") != "" { v, err := parseJSONFlag(runtime, "headers") @@ -569,7 +569,7 @@ var WorkbookCreate = common.Shortcut{ return err } if _, ok := v.([]interface{}); !ok { - return common.FlagErrorf("--headers must be a JSON array") + return sheetsValidationForFlag("headers", "--headers must be a JSON array") } } if runtime.Str("values") != "" { @@ -579,11 +579,11 @@ var WorkbookCreate = common.Shortcut{ } rows, ok := v.([]interface{}) if !ok { - return common.FlagErrorf("--values must be a JSON 2D array") + return sheetsValidationForFlag("values", "--values must be a JSON 2D array") } for i, r := range rows { if _, ok := r.([]interface{}); !ok { - return common.FlagErrorf("--values[%d] must be an array", i) + return sheetsValidationForFlag("values", "--values[%d] must be an array", i) } } } @@ -613,7 +613,7 @@ var WorkbookCreate = common.Shortcut{ if v := strings.TrimSpace(runtime.Str("folder-token")); v != "" { body["folder_token"] = v } - data, err := runtime.CallAPI("POST", "/open-apis/sheets/v3/spreadsheets", nil, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/sheets/v3/spreadsheets", nil, body) if err != nil { return err } @@ -623,7 +623,7 @@ var WorkbookCreate = common.Shortcut{ token = common.GetString(ss, "token") } if token == "" { - return output.Errorf(output.ExitAPI, "api_error", "spreadsheet created but token missing in response") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "spreadsheet created but token missing in response") } result := map[string]interface{}{"spreadsheet": ss} @@ -641,14 +641,12 @@ var WorkbookCreate = common.Shortcut{ // response doesn't echo the default sheet's id, so read it back. firstSheetID, err := lookupFirstSheetID(ctx, runtime, token) if err != nil { - return workbookCreatedButFillFailed(token, ss, - fmt.Sprintf("resolving its first sheet for initial fill failed: %v", err)) + return workbookCreatedButFillFailed(token, "resolving its first sheet for initial fill failed", err) } fill["sheet_id"] = firstSheetID fillOut, err := callTool(ctx, runtime, token, ToolKindWrite, "set_cell_range", fill) if err != nil { - return workbookCreatedButFillFailed(token, ss, - fmt.Sprintf("initial fill failed: %v", err)) + return workbookCreatedButFillFailed(token, "initial fill failed", err) } result["initial_fill"] = fillOut } @@ -660,24 +658,17 @@ var WorkbookCreate = common.Shortcut{ }, } -// workbookCreatedButFillFailed builds a structured partial-success error for the -// window where the spreadsheet POST succeeded but the follow-up initial fill did -// not. The new spreadsheet_token is surfaced in the error detail so callers can -// retry the fill (+cells-set / +csv-put) or delete the orphan, instead of only -// finding the token interpolated into a bare error string. -func workbookCreatedButFillFailed(token string, spreadsheet interface{}, reason string) error { - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "partial_success", - Message: fmt.Sprintf("spreadsheet %s created but %s", token, reason), - Hint: "the spreadsheet exists; retry the fill with the returned spreadsheet_token, or delete it", - Detail: map[string]interface{}{ - "spreadsheet_token": token, - "spreadsheet": spreadsheet, - }, - }, - } +// workbookCreatedButFillFailed reports the window where the spreadsheet POST +// succeeded but the follow-up initial fill did not. The spreadsheet now exists, +// so the caller must change state (retry the fill against the returned token, or +// delete the orphan) rather than re-run create — a failed_precondition. The +// underlying fill failure is kept as the cause so its subtype and log_id stay +// diagnosable, and the created spreadsheet_token is named in the message and +// recovery hint so the caller can act on it. +func workbookCreatedButFillFailed(token, reason string, cause error) error { + return errs.NewValidationError(errs.SubtypeFailedPrecondition, "spreadsheet %s created but %s", token, reason). + WithCause(cause). + WithHint("the spreadsheet exists; retry the fill with the returned spreadsheet_token (+cells-set / +csv-put), or delete it") } // buildInitialFillInput zips --headers + --values into a single set_cell_range @@ -765,7 +756,7 @@ var WorkbookExport = common.Shortcut{ ext = "xlsx" } if ext == "csv" && strings.TrimSpace(runtime.Str("sheet-id")) == "" { - return common.FlagErrorf("--sheet-id is required when --file-extension=csv") + return sheetsValidationForFlag("sheet-id", "--sheet-id is required when --file-extension=csv") } return nil }, @@ -813,13 +804,13 @@ var WorkbookExport = common.Shortcut{ if sid := strings.TrimSpace(runtime.Str("sheet-id")); sid != "" { body["sub_id"] = sid } - taskData, err := runtime.CallAPI("POST", "/open-apis/drive/v1/export_tasks", nil, body) + taskData, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/export_tasks", nil, body) if err != nil { return err } ticket := common.GetString(taskData, "ticket") if ticket == "" { - return output.Errorf(output.ExitAPI, "api_error", "export task created but ticket missing") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "export task created but ticket missing") } result := map[string]interface{}{ @@ -847,9 +838,9 @@ var WorkbookExport = common.Shortcut{ continue default: // any non-zero status outside the in-progress window is a failure if status.JobErrorMsg != "" { - return output.Errorf(output.ExitAPI, "api_error", "export task %s failed: %s", ticket, status.JobErrorMsg) + return errs.NewAPIError(errs.SubtypeServerError, "export task %s failed: %s", ticket, status.JobErrorMsg) } - return output.Errorf(output.ExitAPI, "api_error", "export task %s failed with job_status=%d", ticket, status.JobStatus) + return errs.NewAPIError(errs.SubtypeServerError, "export task %s failed with job_status=%d", ticket, status.JobStatus) } } if fileToken == "" { @@ -887,7 +878,7 @@ type exportTaskStatus struct { } func pollExportTask(runtime *common.RuntimeContext, token, ticket string) (exportTaskStatus, error) { - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/drive/v1/export_tasks/%s", validate.EncodePathSegment(ticket)), map[string]interface{}{"token": token}, @@ -898,7 +889,7 @@ func pollExportTask(runtime *common.RuntimeContext, token, ticket string) (expor } result := common.GetMap(data, "result") if result == nil { - return exportTaskStatus{}, output.Errorf(output.ExitAPI, "api_error", "export task %s: empty result", ticket) + return exportTaskStatus{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "export task %s: empty result", ticket) } js, _ := util.ToFloat64(result["job_status"]) fs, _ := util.ToFloat64(result["file_size"]) @@ -918,10 +909,10 @@ func downloadExportFile(ctx context.Context, runtime *common.RuntimeContext, fil ApiPath: fmt.Sprintf("/open-apis/drive/v1/export_tasks/file/%s/download", validate.EncodePathSegment(fileToken)), }, larkcore.WithFileDownload()) if err != nil { - return "", output.ErrNetwork("download failed: %s", err) + return "", sheetsDownloadRequestError(err) } if apiResp.StatusCode >= 400 { - return "", output.ErrNetwork("download failed: HTTP %d: %s", apiResp.StatusCode, string(apiResp.RawBody)) + return "", sheetsDownloadHTTPStatusError(apiResp) } target := outPath if info, statErr := runtime.FileIO().Stat(outPath); statErr == nil && info.IsDir() { @@ -935,7 +926,7 @@ func downloadExportFile(ctx context.Context, runtime *common.RuntimeContext, fil ContentType: apiResp.Header.Get("Content-Type"), ContentLength: int64(len(apiResp.RawBody)), }, strings.NewReader(string(apiResp.RawBody))); err != nil { - return "", common.WrapSaveErrorByCategory(err, "io") + return "", common.WrapSaveErrorTyped(err) } resolved, _ := runtime.FileIO().ResolvePath(target) if resolved == "" { @@ -944,6 +935,57 @@ func downloadExportFile(ctx context.Context, runtime *common.RuntimeContext, fil return resolved, nil } +func sheetsDownloadRequestError(err error) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, "download failed: %s", err).WithCause(err) +} + +func sheetsDownloadHTTPStatusError(resp *larkcore.ApiResp) error { + status := resp.StatusCode + body := strings.TrimSpace(string(resp.RawBody)) + if body == "" { + body = http.StatusText(status) + } + logID := sheetsDownloadResponseLogID(resp) + if status >= http.StatusInternalServerError { + err := errs.NewNetworkError(errs.SubtypeNetworkServer, "download failed: HTTP %d: %s", status, body). + WithCode(status). + WithRetryable() + if logID != "" { + err = err.WithLogID(logID) + } + return err + } + if status == http.StatusTooManyRequests { + err := errs.NewAPIError(errs.SubtypeRateLimit, "download failed: HTTP %d: %s", status, body). + WithCode(status). + WithRetryable() + if logID != "" { + err = err.WithLogID(logID) + } + return err + } + subtype := errs.SubtypeUnknown + if status == http.StatusNotFound { + subtype = errs.SubtypeNotFound + } + err := errs.NewAPIError(subtype, "download failed: HTTP %d: %s", status, body).WithCode(status) + if logID != "" { + err = err.WithLogID(logID) + } + return err +} + +func sheetsDownloadResponseLogID(resp *larkcore.ApiResp) string { + logID := strings.TrimSpace(resp.Header.Get(larkcore.HttpHeaderKeyLogId)) + if logID == "" { + logID = strings.TrimSpace(resp.Header.Get(larkcore.HttpHeaderKeyRequestId)) + } + return logID +} + // lookupSheetIndex finds a sub-sheet by id or name and returns its canonical // id + current 0-based index. Caller is responsible for ensuring at least one // of sheetID/sheetName is non-empty. @@ -956,7 +998,7 @@ func lookupSheetIndex(ctx context.Context, runtime *common.RuntimeContext, token } m, ok := out.(map[string]interface{}) if !ok { - return "", 0, output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned non-object output") + return "", 0, errs.NewInternalError(errs.SubtypeInvalidResponse, "get_workbook_structure returned non-object output") } sheets, _ := m["sheets"].([]interface{}) for _, raw := range sheets { @@ -975,7 +1017,7 @@ func lookupSheetIndex(ctx context.Context, runtime *common.RuntimeContext, token if (sheetID != "" && id == sheetID) || (sheetName != "" && name == sheetName) { idx, ok := util.ToFloat64(sm["index"]) if !ok { - return "", 0, output.Errorf(output.ExitAPI, "tool_output", "sheet entry missing index field") + return "", 0, errs.NewInternalError(errs.SubtypeInvalidResponse, "sheet entry missing index field") } return id, int(idx), nil } @@ -984,7 +1026,7 @@ func lookupSheetIndex(ctx context.Context, runtime *common.RuntimeContext, token if target == "" { target = sheetName } - return "", 0, output.Errorf(output.ExitAPI, "not_found", fmt.Sprintf("sheet %q not found in workbook", target)) + return "", 0, errs.NewValidationError(errs.SubtypeFailedPrecondition, "sheet %q not found in workbook", target) } // lookupFirstSheetID returns the sheet_id of the sub-sheet at index 0 (the @@ -1001,7 +1043,7 @@ func lookupFirstSheetID(ctx context.Context, runtime *common.RuntimeContext, tok } m, ok := out.(map[string]interface{}) if !ok { - return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned non-object output") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "get_workbook_structure returned non-object output") } sheets, _ := m["sheets"].([]interface{}) bestID := "" @@ -1029,7 +1071,7 @@ func lookupFirstSheetID(ctx context.Context, runtime *common.RuntimeContext, tok } } if bestID == "" { - return "", output.Errorf(output.ExitAPI, "tool_output", "get_workbook_structure returned no sheets") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "get_workbook_structure returned no sheets") } return bestID, nil } diff --git a/shortcuts/sheets/lark_sheet_workbook_test.go b/shortcuts/sheets/lark_sheet_workbook_test.go index c7cad75f92..48baa5baf8 100644 --- a/shortcuts/sheets/lark_sheet_workbook_test.go +++ b/shortcuts/sheets/lark_sheet_workbook_test.go @@ -4,9 +4,14 @@ package sheets import ( + "errors" + "net/http" "strings" "testing" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -391,6 +396,92 @@ func TestWorkbookExport_DryRun(t *testing.T) { }) } +func TestWorkbookExportDownloadErrorClassification(t *testing.T) { + t.Parallel() + + t.Run("preserves typed request errors", func(t *testing.T) { + t.Parallel() + in := errs.NewAPIError(errs.SubtypeServerError, "typed upstream").WithCode(123) + got := sheetsDownloadRequestError(in) + if got != in { + t.Fatalf("typed error was not preserved: got %T %v", got, got) + } + }) + + t.Run("wraps raw request errors as network transport", func(t *testing.T) { + t.Parallel() + got := sheetsDownloadRequestError(errors.New("dial refused")) + p, ok := errs.ProblemOf(got) + if !ok { + t.Fatalf("expected typed problem, got %T %v", got, got) + } + if p.Category != errs.CategoryNetwork || p.Subtype != errs.SubtypeNetworkTransport { + t.Fatalf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, errs.CategoryNetwork, errs.SubtypeNetworkTransport) + } + }) + + tests := []struct { + name string + status int + wantCategory errs.Category + wantSubtype errs.Subtype + wantRetryable bool + }{ + { + name: "5xx is retryable network server error", + status: http.StatusBadGateway, + wantCategory: errs.CategoryNetwork, + wantSubtype: errs.SubtypeNetworkServer, + wantRetryable: true, + }, + { + name: "404 is API not found", + status: http.StatusNotFound, + wantCategory: errs.CategoryAPI, + wantSubtype: errs.SubtypeNotFound, + }, + { + name: "429 is retryable API rate limit", + status: http.StatusTooManyRequests, + wantCategory: errs.CategoryAPI, + wantSubtype: errs.SubtypeRateLimit, + wantRetryable: true, + }, + { + name: "other 4xx is API unknown", + status: http.StatusForbidden, + wantCategory: errs.CategoryAPI, + wantSubtype: errs.SubtypeUnknown, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := sheetsDownloadHTTPStatusError(&larkcore.ApiResp{ + StatusCode: tt.status, + RawBody: []byte("body"), + Header: http.Header{larkcore.HttpHeaderKeyLogId: []string{"log123"}}, + }) + p, ok := errs.ProblemOf(got) + if !ok { + t.Fatalf("expected typed problem, got %T %v", got, got) + } + if p.Category != tt.wantCategory || p.Subtype != tt.wantSubtype { + t.Fatalf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, tt.wantCategory, tt.wantSubtype) + } + if p.Code != tt.status { + t.Fatalf("code = %d, want %d", p.Code, tt.status) + } + if p.LogID != "log123" { + t.Fatalf("log_id = %q, want log123", p.LogID) + } + if p.Retryable != tt.wantRetryable { + t.Fatalf("retryable = %v, want %v", p.Retryable, tt.wantRetryable) + } + }) + } +} + // assertInputEquals compares the decoded tool input map against the wanted // fields. Extra fields in `got` are allowed (defaults, optional fields); // every key in `want` must match exactly. diff --git a/shortcuts/sheets/lark_sheet_write_cells.go b/shortcuts/sheets/lark_sheet_write_cells.go index 9cb51fbae9..09030e5067 100644 --- a/shortcuts/sheets/lark_sheet_write_cells.go +++ b/shortcuts/sheets/lark_sheet_write_cells.go @@ -15,7 +15,7 @@ import ( "strconv" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" "github.com/spf13/cobra" @@ -82,7 +82,7 @@ func cellsSetInput(runtime flagView, token, sheetID, sheetName string) (map[stri return nil, err } if strings.TrimSpace(runtime.Str("range")) == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } cells, err := requireJSONArray(runtime, "cells") if err != nil { @@ -156,11 +156,11 @@ func cellsSetStyleInput(runtime flagView, token, sheetID, sheetName string) (map } rangeStr := strings.TrimSpace(runtime.Str("range")) if rangeStr == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } rows, cols, err := rangeDimensions(rangeStr) if err != nil { - return nil, common.FlagErrorf("--range %q: %v", rangeStr, err) + return nil, sheetsValidationForFlag("range", "--range %q: %v", rangeStr, err) } if err := requireAnyStyleFlag(runtime); err != nil { return nil, err @@ -218,6 +218,7 @@ var CsvPut = common.Shortcut{ delete(fl.Annotations, cobra.BashCompOneRequiredFlag) } cmd.MarkFlagsOneRequired("start-cell", "range") + cmd.MarkFlagsMutuallyExclusive("start-cell", "range") }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if err := guardCSVValueIsNotFilePath(runtime); err != nil { @@ -324,7 +325,7 @@ func guardCSVValueIsNotFilePath(runtime *common.RuntimeContext) error { if err != nil || info == nil || info.IsDir() { return nil //nolint:nilerr // fail-open: a missing/unreadable path is treated as inline content, not a forgotten @ } - return common.FlagErrorf( + return sheetsValidationForFlag("csv", "--csv value %q is an existing file, not inline CSV; to read it use --csv @%s, or pass the literal text via stdin (--csv -)", raw, raw, ) @@ -335,7 +336,10 @@ func csvPutInput(runtime flagView, token, sheetID, sheetName string) (map[string return nil, err } if strings.TrimSpace(runtime.Str("csv")) == "" { - return nil, common.FlagErrorf("--csv is required") + return nil, sheetsValidationForFlag("csv", "--csv is required") + } + if runtime.Changed("start-cell") && runtime.Changed("range") { + return nil, common.ValidationErrorf("--start-cell and --range are mutually exclusive").WithParams(sheetsInvalidParam("start-cell", "mutually exclusive"), sheetsInvalidParam("range", "mutually exclusive")) } anchor := strings.TrimSpace(runtime.Str("start-cell")) // --range is accepted as an alias for --start-cell. +csv-get and +cells-set @@ -346,23 +350,27 @@ func csvPutInput(runtime flagView, token, sheetID, sheetName string) (map[string // collapses to its top-left cell; +csv-put pastes from the anchor and // auto-expands, so the range's lower-right bound is irrelevant. // - // Standalone enforces "one of --start-cell / --range" via cobra's - // MarkFlagsOneRequired (see PostMount). A +batch-update sub-op never runs - // cobra, so without an explicit check the default "A1" silently wins and the - // paste lands at A1 instead of failing like the standalone command. Mirror - // the standalone contract: when --start-cell is absent, --range is mandatory. + // Standalone enforces exactly one of --start-cell / --range via cobra's + // flag groups (see PostMount). A +batch-update sub-op never runs cobra, so + // without explicit checks the default "A1" silently wins and the paste lands + // at A1 instead of failing like the standalone command. Mirror the + // standalone contract: double-set is invalid, and when --start-cell is + // absent, --range is mandatory. if !runtime.Changed("start-cell") { rng := strings.TrimSpace(runtime.Str("range")) if rng == "" { - return nil, common.FlagErrorf("--start-cell or --range is required") + return nil, common.ValidationErrorf("--start-cell or --range is required").WithParams(sheetsInvalidParam("start-cell", "required; specify exactly one"), sheetsInvalidParam("range", "required; specify exactly one")) } anchor = strings.TrimSpace(strings.SplitN(rng, ":", 2)[0]) + if idx := strings.Index(anchor, "!"); idx >= 0 { + anchor = anchor[idx+1:] + } } if anchor == "" { - return nil, common.FlagErrorf("--start-cell is required") + return nil, sheetsValidationForFlag("start-cell", "--start-cell is required") } if _, _, ok := splitCellRef(anchor); !ok { - return nil, common.FlagErrorf("--start-cell %q must be a single cell ref (e.g. A1)", anchor) + return nil, sheetsValidationForFlag("start-cell", "--start-cell %q must be a single cell ref (e.g. A1)", anchor) } input := map[string]interface{}{ "excel_id": token, @@ -433,11 +441,11 @@ func dropdownSetInput(runtime flagView, token, sheetID, sheetName string) (map[s } rangeStr := strings.TrimSpace(runtime.Str("range")) if rangeStr == "" { - return nil, common.FlagErrorf("--range is required") + return nil, sheetsValidationForFlag("range", "--range is required") } rows, cols, err := rangeDimensions(rangeStr) if err != nil { - return nil, common.FlagErrorf("--range %q: %v", rangeStr, err) + return nil, sheetsValidationForFlag("range", "--range %q: %v", rangeStr, err) } validation, err := buildDropdownValidation(runtime) if err != nil { @@ -496,7 +504,7 @@ func buildDropdownValidation(runtime flagView) (map[string]interface{}, error) { return nil, err } if len(colors) > sourceSize { - return nil, common.FlagErrorf("--colors length (%d) must not exceed dropdown source size (%d)", len(colors), sourceSize) + return nil, sheetsValidationForFlag("colors", "--colors length (%d) must not exceed dropdown source size (%d)", len(colors), sourceSize) } dv["highlight_colors"] = colors } @@ -518,9 +526,9 @@ func dropdownTypeAndItems(runtime flagView) (int, map[string]interface{}, error) sourceRange := strings.TrimSpace(runtime.Str("source-range")) switch { case optsRaw != "" && sourceRange != "": - return 0, nil, common.FlagErrorf("--options and --source-range are mutually exclusive; pass exactly one") + return 0, nil, common.ValidationErrorf("--options and --source-range are mutually exclusive; pass exactly one").WithParams(sheetsInvalidParam("options", "mutually exclusive"), sheetsInvalidParam("source-range", "mutually exclusive")) case optsRaw == "" && sourceRange == "": - return 0, nil, common.FlagErrorf("one of --options (inline list) or --source-range (listFromRange) is required") + return 0, nil, common.ValidationErrorf("one of --options (inline list) or --source-range (listFromRange) is required").WithParams(sheetsInvalidParam("options", "required; specify exactly one"), sheetsInvalidParam("source-range", "required; specify exactly one")) case optsRaw != "": options, err := requireJSONArray(runtime, "options") if err != nil { @@ -533,7 +541,7 @@ func dropdownTypeAndItems(runtime flagView) (int, map[string]interface{}, error) default: // sourceRange != "" rows, cols, err := rangeDimensions(sourceRange) if err != nil { - return 0, nil, common.FlagErrorf("--source-range %q: %v", sourceRange, err) + return 0, nil, sheetsValidationForFlag("source-range", "--source-range %q: %v", sourceRange, err) } return rows * cols, map[string]interface{}{ "type": "listFromRange", @@ -558,7 +566,7 @@ func validateDropdownSourceOrOptions(runtime flagView) (int, error) { return 0, err } if len(colors) > sourceSize { - return 0, common.FlagErrorf("--colors length (%d) must not exceed dropdown source size (%d)", len(colors), sourceSize) + return 0, sheetsValidationForFlag("colors", "--colors length (%d) must not exceed dropdown source size (%d)", len(colors), sourceSize) } } return sourceSize, nil @@ -731,18 +739,18 @@ var CellsSetImage = common.Shortcut{ } r := strings.TrimSpace(runtime.Str("range")) if r == "" { - return common.FlagErrorf("--range is required") + return sheetsValidationForFlag("range", "--range is required") } rows, cols, err := rangeDimensions(r) if err != nil { - return common.FlagErrorf("--range %q: %v", r, err) + return sheetsValidationForFlag("range", "--range %q: %v", r, err) } if rows != 1 || cols != 1 { - return common.FlagErrorf("--range %q must be exactly one cell (got %d×%d)", r, rows, cols) + return sheetsValidationForFlag("range", "--range %q must be exactly one cell (got %d×%d)", r, rows, cols) } imgPath := strings.TrimSpace(runtime.Str("image")) if imgPath == "" { - return common.FlagErrorf("--image is required") + return sheetsValidationForFlag("image", "--image is required") } // Validate path safety here (not just at Execute) so --dry-run also // rejects unsafe paths instead of giving a false-positive preview. @@ -750,7 +758,9 @@ var CellsSetImage = common.Shortcut{ // not existence, so legitimate relative paths still dry-run cleanly; // the Execute-time Stat below still reports a missing/unreadable file. if _, err := validate.SafeLocalFlagPath("--image", imgPath); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err). + WithParam("--image"). + WithCause(err) } return nil }, @@ -806,16 +816,18 @@ var CellsSetImage = common.Shortcut{ } info, err := runtime.FileIO().Stat(imgPath) if err != nil { - return common.WrapInputStatError(err) + return sheetsInputStatError("image", err) } imgFile, err := runtime.FileIO().Open(imgPath) if err != nil { - return common.WrapInputStatError(err) + return sheetsInputStatError("image", err) } imgCfg, _, err := image.DecodeConfig(imgFile) imgFile.Close() if err != nil { - return fmt.Errorf("decode image dimensions: %w", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "decode image dimensions: %s", err). + WithParam("--image"). + WithCause(err) } fileToken, err := common.UploadDriveMediaAll(runtime, common.DriveMediaUploadAllConfig{ FilePath: imgPath, @@ -844,7 +856,7 @@ var CellsSetImage = common.Shortcut{ sheetSelectorForToolInput(setCellInput, sheetID, sheetName) setCellOut, err := callTool(ctx, runtime, token, ToolKindWrite, "set_cell_range", setCellInput) if err != nil { - return fmt.Errorf("image uploaded (file_token=%s) but cell write failed: %w", fileToken, err) + return wrapCellsSetImageWriteError(err, fileToken) } runtime.Out(map[string]interface{}{ "file_token": fileToken, @@ -857,3 +869,18 @@ var CellsSetImage = common.Shortcut{ "--range must be a single cell. The uploaded image becomes a cell-internal embed; use +float-image-create for floating images.", }, } + +func wrapCellsSetImageWriteError(err error, fileToken string) error { + hint := fmt.Sprintf("image was uploaded as file_token=%s; retry only the cell write with that token or remove the uploaded media", fileToken) + if p, ok := errs.ProblemOf(err); ok { + if strings.TrimSpace(p.Hint) != "" { + p.Hint += "\n" + hint + } else { + p.Hint = hint + } + return err + } + return errs.NewInternalError(errs.SubtypeSDKError, "image uploaded (file_token=%s) but cell write failed: %s", fileToken, err). + WithHint(hint). + WithCause(err) +} diff --git a/shortcuts/sheets/sheet_ai_api.go b/shortcuts/sheets/sheet_ai_api.go index eb43684763..78560d88bc 100644 --- a/shortcuts/sheets/sheet_ai_api.go +++ b/shortcuts/sheets/sheet_ai_api.go @@ -8,7 +8,7 @@ import ( "encoding/json" "fmt" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -42,7 +42,7 @@ func toolInvokePath(token string, kind ToolKind) string { func buildToolBody(toolName string, input map[string]interface{}) (map[string]interface{}, error) { inputJSON, err := json.Marshal(input) if err != nil { - return nil, fmt.Errorf("encode tool input: %w", err) + return nil, errs.NewInternalError(errs.SubtypeSDKError, "encode tool input: %v", err).WithCause(err) } return map[string]interface{}{ "tool_name": toolName, @@ -77,13 +77,14 @@ func callTool( envelope, ok := raw.(map[string]interface{}) if !ok { - return nil, output.Errorf(output.ExitAPI, "tool_response", + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "tool %q: unexpected non-JSON-object response: %v", toolName, raw) } code, _ := util.ToFloat64(envelope["code"]) if code != 0 { msg, _ := envelope["msg"].(string) - return nil, output.ErrAPI(int(code), fmt.Sprintf("tool %q failed: [%d] %s", toolName, int(code), msg), envelope["error"]) + return nil, errs.NewAPIError(errs.SubtypeServerError, "tool %q failed: [%d] %s", toolName, int(code), msg). + WithCode(int(code)) } data, _ := envelope["data"].(map[string]interface{}) rawOutput, _ := data["output"].(string) @@ -93,8 +94,8 @@ func callTool( var out interface{} if err := json.Unmarshal([]byte(rawOutput), &out); err != nil { - return nil, output.Errorf(output.ExitAPI, "tool_output", - "tool %q returned invalid JSON output: %v", toolName, err) + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "tool %q returned invalid JSON output: %v", toolName, err).WithCause(err) } return out, nil } diff --git a/shortcuts/sheets/validation_params_test.go b/shortcuts/sheets/validation_params_test.go new file mode 100644 index 0000000000..c12177737c --- /dev/null +++ b/shortcuts/sheets/validation_params_test.go @@ -0,0 +1,104 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package sheets + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" +) + +// TestValidationParamMetadata locks the structured-param contract across the +// sheets domain: a validation failure must carry typed metadata +// (category/subtype via errs.ProblemOf) and tag the offending flag(s) via +// Param/Params, so consumers (agents) know which flag to fix without parsing +// the human message. Covers the four representative shapes — required, +// at-least-one, mutually-exclusive, and local-input-file errors. +func TestValidationParamMetadata(t *testing.T) { + t.Parallel() + + // assertValidationProblem checks the typed metadata (category + subtype) via + // errs.ProblemOf and returns the ValidationError for param-level assertions. + assertValidationProblem := func(t *testing.T, err error) *errs.ValidationError { + t.Helper() + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("error = %T %v, want typed problem", err, err) + } + if p.Category != errs.CategoryValidation { + t.Errorf("category = %q, want %q", p.Category, errs.CategoryValidation) + } + if p.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", p.Subtype, errs.SubtypeInvalidArgument) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error = %T, want *errs.ValidationError", err) + } + return ve + } + + assertParam := func(t *testing.T, err error, want string) { + t.Helper() + if ve := assertValidationProblem(t, err); ve.Param != want { + t.Fatalf("param = %q, want %q", ve.Param, want) + } + } + + assertParams := func(t *testing.T, err error, want ...string) { + t.Helper() + ve := assertValidationProblem(t, err) + got := map[string]bool{} + for _, p := range ve.Params { + got[p.Name] = true + } + if len(ve.Params) != len(want) { + t.Fatalf("params = %#v, want %v", ve.Params, want) + } + for _, w := range want { + if !got[w] { + t.Fatalf("params = %#v, missing %q", ve.Params, w) + } + } + } + + t.Run("required flag tags single param", func(t *testing.T) { + t.Parallel() + // --image-token satisfies the image source, so the missing --image-name + // trips the single-flag required check routed through sheetsValidationForFlag. + fv := newMapFlagViewForCommand("+float-image-create", map[string]interface{}{"image-token": "tok"}) + _, err := floatImageProperties(fv, "", true) + assertParam(t, err, "--image-name") + }) + + t.Run("at-least-one tags every candidate flag", func(t *testing.T) { + t.Parallel() + fv := newMapFlagViewForCommand("+float-image-create", map[string]interface{}{}) + _, err := floatImageProperties(fv, "", true) + assertParams(t, err, "--image", "--image-token", "--image-uri") + }) + + t.Run("mutually exclusive tags only the conflicting flags", func(t *testing.T) { + t.Parallel() + // Only --image and --image-token are set; the param list must not blame + // the untouched --image-uri. + fv := newMapFlagViewForCommand("+float-image-create", map[string]interface{}{ + "image": "a.png", + "image-token": "tok", + }) + _, err := floatImageProperties(fv, "", true) + assertParams(t, err, "--image", "--image-token") + }) + + t.Run("local input file error tags flag and preserves cause", func(t *testing.T) { + t.Parallel() + cause := errors.New("stat failed") + err := sheetsInputStatError("image", cause) + assertParam(t, err, "--image") + if !errors.Is(err, cause) { + t.Errorf("expected the original stat error preserved as the cause") + } + }) +}