diff --git a/internal/errclass/codemeta_drive.go b/internal/errclass/codemeta_drive.go index 9722d7e4..83ac13ec 100644 --- a/internal/errclass/codemeta_drive.go +++ b/internal/errclass/codemeta_drive.go @@ -10,15 +10,20 @@ import "github.com/larksuite/cli/errs" // ambiguous codes fall back to CategoryAPI via BuildAPIError. // BuildAPIError consumes this map via mergeCodeMeta + LookupCodeMeta. var driveCodeMeta = map[int]CodeMeta{ - 1061001: {Category: errs.CategoryAPI, Subtype: errs.SubtypeServerError, Retryable: true}, // Drive "unknown error" - 1061002: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // params error - 1061004: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypePermissionDenied}, // forbidden - 1061007: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // file has been deleted - 1061043: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded}, // file size beyond limit - 1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload) - 1062009: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // actual size inconsistent with declared size - 1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters" - 2200: {Category: errs.CategoryAPI, Subtype: errs.SubtypeServerError, Retryable: true}, // Drive tenant/internal errors + 1061001: {Category: errs.CategoryAPI, Subtype: errs.SubtypeServerError, Retryable: true}, // Drive "unknown error" + 1061002: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // params error + 1061004: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypePermissionDenied}, // forbidden + 1061007: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // file has been deleted + 1061043: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded}, // file size beyond limit + 1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload) + 1062009: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // actual size inconsistent with declared size + 1063001: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // secure label invalid parameter + 1063002: {Category: errs.CategoryAuthorization, Subtype: errs.SubtypePermissionDenied}, // secure label permission denied + 1063013: {Category: errs.CategoryValidation, Subtype: errs.SubtypeFailedPrecondition}, // secure label downgrade requires approval + 1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters" + 99992402: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // platform field validation failed + 9499: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // invalid parameter type in JSON field + 2200: {Category: errs.CategoryAPI, Subtype: errs.SubtypeServerError, Retryable: true}, // Drive tenant/internal errors } func init() { mergeCodeMeta(driveCodeMeta, "drive") } diff --git a/internal/errclass/codemeta_drive_test.go b/internal/errclass/codemeta_drive_test.go index a65a94f2..39e8b4b8 100644 --- a/internal/errclass/codemeta_drive_test.go +++ b/internal/errclass/codemeta_drive_test.go @@ -27,6 +27,13 @@ func TestLookupCodeMeta_DriveCodes(t *testing.T) { // 1069302: comment endpoint's opaque "Invalid or missing parameters" // (shortcuts/drive/drive_add_comment.go) → API-side parameter rejection. {1069302, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + // Secure label endpoint codes observed from drive +secure-label-update + // failure telemetry. + {1063001, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {1063002, errs.CategoryAuthorization, errs.SubtypePermissionDenied, false}, + {1063013, errs.CategoryValidation, errs.SubtypeFailedPrecondition, false}, + {99992402, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + {9499, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, } for _, tc := range cases { t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) { diff --git a/shortcuts/drive/drive_secure_label.go b/shortcuts/drive/drive_secure_label.go index 8c99f370..67b8250f 100644 --- a/shortcuts/drive/drive_secure_label.go +++ b/shortcuts/drive/drive_secure_label.go @@ -6,6 +6,7 @@ package drive import ( "context" "fmt" + "strings" "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" @@ -17,6 +18,13 @@ const ( secureLabelUpdateScope = "docs:secure_label:write_only" ) +type secureLabelOperation string + +const ( + secureLabelOperationList secureLabelOperation = "list" + secureLabelOperationUpdate secureLabelOperation = "update" +) + var secureLabelTypes = permApplyTypes // DriveSecureLabelList lists secure labels available to the current user. @@ -28,6 +36,9 @@ var DriveSecureLabelList = common.Shortcut{ Scopes: []string{secureLabelReadScope}, AuthTypes: []string{"user"}, HasFormat: true, + Tips: []string{ + "Use the `id` field from this command as --label-id for +secure-label-update; do not use the display name.", + }, Flags: []common.Flag{ {Name: "page-size", Type: "int", Default: "10", Desc: "page size, 1-10"}, {Name: "page-token", Desc: "pagination token from previous response"}, @@ -53,7 +64,7 @@ var DriveSecureLabelList = common.Shortcut{ nil, ) if err != nil { - return err + return decorateSecureLabelError(err, secureLabelOperationList) } runtime.OutFormat(data, nil, nil) return nil @@ -68,13 +79,21 @@ var DriveSecureLabelUpdate = common.Shortcut{ Risk: "write", Scopes: []string{secureLabelUpdateScope}, AuthTypes: []string{"user"}, + Tips: []string{ + "Pass the numeric label id returned by +secure-label-list; display names like Public(D) are rejected.", + "Downgrading a secure label may require approval; retrying the same request will not bypass approval.", + "When updating many files, serialize requests and back off on rate_limit errors.", + }, Flags: []common.Flag{ {Name: "token", Desc: "target file token or document URL (docx/sheets/base/file/wiki/doc/mindnote/slides)", Required: true}, {Name: "type", Desc: "target type; auto-inferred from URL when omitted", Enum: secureLabelTypes}, {Name: "label-id", Desc: "secure label ID to set", Required: true}, }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { - _, _, err := resolveSecureLabelTarget(runtime.Str("token"), runtime.Str("type")) + if _, _, err := resolveSecureLabelTarget(runtime.Str("token"), runtime.Str("type")); err != nil { + return err + } + _, err := normalizeSecureLabelID(runtime.Str("label-id")) return err }, DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { @@ -82,11 +101,15 @@ var DriveSecureLabelUpdate = common.Shortcut{ if err != nil { return common.NewDryRunAPI().Set("error", err.Error()) } + labelID, err := normalizeSecureLabelID(runtime.Str("label-id")) + if err != nil { + return common.NewDryRunAPI().Set("error", err.Error()) + } return common.NewDryRunAPI(). Desc("Update Drive secure label"). PATCH("/open-apis/drive/v2/files/:file_token/secure_label"). Params(map[string]interface{}{"type": docType}). - Body(map[string]interface{}{"id": runtime.Str("label-id")}). + Body(map[string]interface{}{"id": labelID}). Set("file_token", token) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { @@ -94,14 +117,18 @@ var DriveSecureLabelUpdate = common.Shortcut{ if err != nil { return err } - body := map[string]interface{}{"id": runtime.Str("label-id")} + labelID, err := normalizeSecureLabelID(runtime.Str("label-id")) + if err != nil { + return err + } + body := map[string]interface{}{"id": labelID} data, err := runtime.CallAPITyped("PATCH", fmt.Sprintf("/open-apis/drive/v2/files/%s/secure_label", validate.EncodePathSegment(token)), map[string]interface{}{"type": docType}, body, ) if err != nil { - return err + return decorateSecureLabelError(err, secureLabelOperationUpdate) } runtime.Out(data, nil) return nil @@ -122,3 +149,70 @@ func buildSecureLabelListParams(runtime *common.RuntimeContext) map[string]inter func resolveSecureLabelTarget(raw, explicitType string) (token, docType string, err error) { return resolvePermApplyTarget(raw, explicitType) } + +// normalizeSecureLabelID trims a label id and rejects display names before the +// request reaches Drive, where they otherwise surface as opaque JSON errors. +func normalizeSecureLabelID(raw string) (string, error) { + labelID := strings.TrimSpace(raw) + if labelID == "" { + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--label-id is required"). + WithParam("--label-id") + } + for _, r := range labelID { + if r < '0' || r > '9' { + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--label-id must be a numeric secure label ID, not a display name: %q", raw). + WithParam("--label-id"). + WithHint("run `lark-cli drive +secure-label-list` and pass the numeric `id` value; do not pass label names like `Public(D)`") + } + } + return labelID, nil +} + +// decorateSecureLabelError appends command-aware recovery guidance while +// preserving upstream/classifier hints already attached to the typed error. +func decorateSecureLabelError(err error, operation secureLabelOperation) error { + if err == nil { + return nil + } + p, ok := errs.ProblemOf(err) + if !ok { + return err + } + guidance := secureLabelErrorGuidance(p.Code, operation) + if guidance == "" { + return err + } + if p.Hint == "" { + p.Hint = guidance + } else if !strings.Contains(p.Hint, guidance) { + p.Hint = p.Hint + "; " + guidance + } + return err +} + +// secureLabelErrorGuidance returns recovery guidance for secure-label API +// failures whose generic code-level classification needs command context. +func secureLabelErrorGuidance(code int, operation secureLabelOperation) string { + switch code { + case 99991400: + if operation == secureLabelOperationUpdate { + return "secure label updates are rate limited; retry later with exponential backoff and serialize bulk updates" + } + return "secure label listing is rate limited; retry later with exponential backoff" + case 1063013: + if operation == secureLabelOperationUpdate { + return "secure label downgrade requires approval; request approval or choose a non-downgrade label before retrying" + } + case 1063002: + if operation == secureLabelOperationUpdate { + return "the current user lacks permission to update this file's secure label; use a user with file and security-label permission" + } + return "the current user lacks permission to list secure labels; use a user with security-label read permission" + case 1063001, 99992402, 9499: + if operation == secureLabelOperationUpdate { + return "check --token/--type and pass a secure label ID from `lark-cli drive +secure-label-list`, not the display name" + } + return "check secure label list parameters such as --page-size, --page-token, and --lang" + } + return "" +} diff --git a/shortcuts/drive/drive_secure_label_test.go b/shortcuts/drive/drive_secure_label_test.go index 49edaa1b..c467930f 100644 --- a/shortcuts/drive/drive_secure_label_test.go +++ b/shortcuts/drive/drive_secure_label_test.go @@ -5,9 +5,11 @@ package drive import ( "encoding/json" + "errors" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" ) @@ -90,13 +92,54 @@ func TestDriveSecureLabelList_ExecuteSuccess(t *testing.T) { } } +func TestDriveSecureLabelList_RateLimitPreservesUpstreamHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: "GET", + URL: "/open-apis/drive/v2/my_secure_labels?page_size=10", + Status: 429, + Body: map[string]interface{}{ + "code": 99991400, + "msg": "rate limit exceeded", + "error": map[string]interface{}{ + "details": []interface{}{ + map[string]interface{}{"value": "server says slow down"}, + }, + }, + }, + }) + + err := mountAndRunDrive(t, DriveSecureLabelList, []string{ + "+secure-label-list", + "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected rate limit error") + } + var apiErr *errs.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected APIError, got %T: %v", err, err) + } + if apiErr.Subtype != errs.SubtypeRateLimit || apiErr.Code != 99991400 || !apiErr.Retryable { + t.Fatalf("problem = %+v, want code=99991400 subtype=rate_limit retryable=true", apiErr.Problem) + } + for _, want := range []string{"server says slow down", "secure label listing is rate limited"} { + if !strings.Contains(apiErr.Hint, want) { + t.Fatalf("hint missing %q: %q", want, apiErr.Hint) + } + } + if strings.Contains(apiErr.Hint, "updates are rate limited") { + t.Fatalf("list hint should not use update-specific wording: %q", apiErr.Hint) + } +} + func TestDriveSecureLabelUpdate_DryRunInfersTypeFromURL(t *testing.T) { t.Parallel() f, stdout, _, _ := cmdutil.TestFactory(t, driveTestConfig()) err := mountAndRunDrive(t, DriveSecureLabelUpdate, []string{ "+secure-label-update", "--token", "https://example.feishu.cn/docx/doxTok123?from=share", - "--label-id", "7217780879644737539", + "--label-id", " 7217780879644737539 ", "--dry-run", "--as", "user", }, f, stdout) if err != nil { @@ -132,7 +175,7 @@ func TestDriveSecureLabelUpdate_ExecuteSuccess(t *testing.T) { "+secure-label-update", "--token", "doxTok123", "--type", "docx", - "--label-id", "7217780879644737539", + "--label-id", " 7217780879644737539 ", "--as", "user", }, f, stdout) if err != nil { @@ -148,7 +191,32 @@ func TestDriveSecureLabelUpdate_ExecuteSuccess(t *testing.T) { } } -func TestDriveSecureLabelUpdate_DowngradeApprovalReturnsAPIError(t *testing.T) { +func TestDriveSecureLabelUpdate_RejectsDisplayNameAsLabelID(t *testing.T) { + t.Parallel() + f, stdout, _, _ := cmdutil.TestFactory(t, driveTestConfig()) + err := mountAndRunDrive(t, DriveSecureLabelUpdate, []string{ + "+secure-label-update", + "--token", "doxTok123", + "--type", "docx", + "--label-id", "Public(D)", + "--as", "user", + }, f, stdout) + if err == nil { + t.Fatal("expected label id validation error") + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected ValidationError, got %T: %v", err, err) + } + if validationErr.Param != "--label-id" { + t.Fatalf("Param = %q, want --label-id", validationErr.Param) + } + if !strings.Contains(validationErr.Hint, "+secure-label-list") { + t.Fatalf("hint missing list guidance: %q", validationErr.Hint) + } +} + +func TestDriveSecureLabelUpdate_DowngradeApprovalReturnsFailedPrecondition(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) reg.Register(&httpmock.Stub{ Method: "PATCH", @@ -169,7 +237,78 @@ func TestDriveSecureLabelUpdate_DowngradeApprovalReturnsAPIError(t *testing.T) { if err == nil { t.Fatal("expected 1063013 error") } - if !strings.Contains(err.Error(), "Security label downgrade requires approval") { - t.Fatalf("expected raw API error message, got: %v", err) + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected ValidationError, got %T: %v", err, err) + } + if validationErr.Subtype != errs.SubtypeFailedPrecondition || validationErr.Code != 1063013 { + t.Fatalf("problem = %+v, want code=1063013 subtype=failed_precondition", validationErr.Problem) + } + if !strings.Contains(validationErr.Hint, "approval") { + t.Fatalf("hint missing approval guidance: %q", validationErr.Hint) + } +} + +func TestDriveSecureLabelUpdate_InvalidJSONTypeGetsLabelHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: "PATCH", + URL: "/open-apis/drive/v2/files/doxTok123/secure_label", + Status: 400, + Body: map[string]interface{}{ + "code": 9499, "msg": "Invalid parameter type in json: id", + }, + }) + + err := mountAndRunDrive(t, DriveSecureLabelUpdate, []string{ + "+secure-label-update", + "--token", "https://example.feishu.cn/docx/doxTok123", + "--label-id", "7217780879644737539", + "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected 9499 error") + } + var apiErr *errs.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected APIError, got %T: %v", err, err) + } + if apiErr.Subtype != errs.SubtypeInvalidParameters || apiErr.Code != 9499 { + t.Fatalf("problem = %+v, want code=9499 subtype=invalid_parameters", apiErr.Problem) + } + if !strings.Contains(apiErr.Hint, "+secure-label-list") { + t.Fatalf("hint missing secure label list guidance: %q", apiErr.Hint) + } +} + +func TestDriveSecureLabelUpdate_RateLimitIsRetryableWithBackoffHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: "PATCH", + URL: "/open-apis/drive/v2/files/doxTok123/secure_label", + Status: 429, + Body: map[string]interface{}{ + "code": 99991400, "msg": "rate limit exceeded", + }, + }) + + err := mountAndRunDrive(t, DriveSecureLabelUpdate, []string{ + "+secure-label-update", + "--token", "https://example.feishu.cn/docx/doxTok123", + "--label-id", "7217780879644737539", + "--as", "user", + }, f, nil) + if err == nil { + t.Fatal("expected rate limit error") + } + var apiErr *errs.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected APIError, got %T: %v", err, err) + } + if apiErr.Subtype != errs.SubtypeRateLimit || apiErr.Code != 99991400 || !apiErr.Retryable { + t.Fatalf("problem = %+v, want code=99991400 subtype=rate_limit retryable=true", apiErr.Problem) + } + if !strings.Contains(apiErr.Hint, "backoff") { + t.Fatalf("hint missing backoff guidance: %q", apiErr.Hint) } }