mirror of
https://github.com/larksuite/cli.git
synced 2026-07-03 14:02:43 +08:00
Improve secure label error handling (#1707)
* Improve secure label error handling * Address secure label review feedback
This commit is contained in:
@@ -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") }
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 ""
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user