Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion internal/errclass/classify.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,13 @@ func BuildAPIError(resp map[string]any, cc ClassifyContext) error {
}
case errs.CategoryAPI:
// A server-supplied detail (lifted into base.Hint above) wins over the
// context-free APIHint default; only fall back to APIHint when absent.
// context-free defaults; below that, a per-code CodeMeta.Hint (for codes
// whose recovery is more specific than their subtype's wording) wins
// over the per-subtype APIHint; only fall back to APIHint when both
// are absent.
if base.Hint == "" {
base.Hint = meta.Hint
}
if base.Hint == "" {
base.Hint = APIHint(base.Subtype) // "" for subtypes without a context-free default
}
Expand Down
48 changes: 48 additions & 0 deletions internal/errclass/classify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,54 @@ func TestBuildAPIError_TaskInvalidParamsRoutesToAPIError(t *testing.T) {
}
}

// TestBuildAPIError_PerCodeHint pins that the CategoryAPI arm surfaces the
// per-code CodeMeta.Hint on the typed envelope — the commercial-plan codes
// are produced by typed call sites (slides +create, drive import), so without
// this the plan-quota codes would inherit the misleading per-subtype default
// ("retry after the relevant quota resets") and 90003088 would carry no hint
// at all. Also pins precedence: a server-supplied detail still wins.
func TestBuildAPIError_PerCodeHint(t *testing.T) {
t.Run("quota code gets per-code hint over subtype default", func(t *testing.T) {
resp := map[string]any{"code": 90003087, "msg": "A2 create quota exceeded"}
err := errclass.BuildAPIError(resp, errclass.ClassifyContext{})
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatal("ProblemOf returned !ok")
}
if !strings.Contains(p.Hint, "retrying will not help") {
t.Errorf("Hint = %q, want the per-code plan-quota hint, not the generic quota-resets default", p.Hint)
}
})
t.Run("failed-precondition code gets per-code hint despite empty subtype default", func(t *testing.T) {
resp := map[string]any{"code": 90003088, "msg": "docs module unbundled"}
err := errclass.BuildAPIError(resp, errclass.ClassifyContext{})
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatal("ProblemOf returned !ok")
}
if !strings.Contains(p.Hint, "docs module") {
t.Errorf("Hint = %q, want the per-code docs-module hint", p.Hint)
}
})
t.Run("server detail still wins over per-code hint", func(t *testing.T) {
resp := map[string]any{
"code": 90003087,
"msg": "A2 create quota exceeded",
"error": map[string]any{
"details": []any{map[string]any{"value": "server-side specifics"}},
},
}
err := errclass.BuildAPIError(resp, errclass.ClassifyContext{})
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatal("ProblemOf returned !ok")
}
if !strings.Contains(p.Hint, "server-side specifics") {
t.Errorf("Hint = %q, want lifted server detail to take precedence", p.Hint)
}
})
}

// TestBuildAPIError_TroubleshooterLiftedOnAPIArm pins that BuildAPIError lifts
// resp.error.troubleshooter into Problem.Troubleshooter when the response
// routes to the catch-all CategoryAPI arm. troubleshooter is the only
Expand Down
11 changes: 10 additions & 1 deletion internal/errclass/codemeta.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,25 @@ import (
)

// CodeMeta is the classification metadata attached to a Lark numeric code.
// It does NOT carry Message or Hint — those are derived at the dispatcher
// It does NOT carry Message — that is derived at the dispatcher
// (see BuildAPIError).
//
// Hint is the optional per-code context-free recovery hint for codes whose
// recovery action is more specific than their subtype's APIHint default
// (e.g. a plan-quota code whose quota never resets, so the generic
// "retry after the quota resets" wording would mislead). The CategoryAPI
// dispatcher arm prefers a server-supplied detail, then this per-code hint,
// then APIHint(subtype). Leave empty when the subtype default (or no hint)
// is accurate.
//
// Risk + Action are populated only for codes that route to CategoryConfirmation;
// the dispatcher falls back to RiskUnknown + ctx.LarkCmd when either is empty
// so the envelope is never wire-invalid.
type CodeMeta struct {
Category errs.Category
Subtype errs.Subtype
Retryable bool
Hint string // CategoryAPI arm only; empty → fall back to APIHint(subtype)
Risk string // CategoryConfirmation arm only; empty otherwise
Action string // CategoryConfirmation arm only; empty otherwise
}
Expand Down
20 changes: 20 additions & 0 deletions internal/errclass/codemeta_drive.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,26 @@ import "github.com/larksuite/cli/errs"
var driveCodeMeta = map[int]CodeMeta{
1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload)
1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters"

// Commercial plan codes returned by the cloud-space explorer service
// and passed through verbatim by document backends (observed via
// slides_ai create — engine log "A2 create quota exceeded, code=90003087"
// — and drive/v1/import_tasks, both as HTTP 200 with body code≠0).
// Plan/billing limits: retrying can never succeed. 90003086/90003087 are
// plan creation-count quotas (upgrade the plan or free quota); 90003088 is
// not a quota at all — the tenant has not purchased/enabled the docs
// module, so it maps to FailedPrecondition (admin must enable the module)
// rather than QuotaExceeded, whose default hint suggests freeing quota.
// Hint is set per-code because the SubtypeQuotaExceeded default
// ("retry after the relevant quota resets") misleads here: plan quotas
// never reset on their own. Keep the wording in sync with the legacy
// path's legacyHints entries (internal/output/lark_errors.go).
90003086: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded,
Hint: "document creation quota of the current plan reached: upgrade the plan or delete documents you no longer need to free quota; retrying will not help"}, // premium plan creation count limit reached
90003087: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded,
Hint: "document creation quota of the current plan reached: upgrade the plan or delete documents you no longer need to free quota; retrying will not help"}, // A2 plan creation count limit reached
90003088: {Category: errs.CategoryAPI, Subtype: errs.SubtypeFailedPrecondition,
Hint: "the tenant has not purchased or enabled the docs module; ask the tenant admin to enable it before creating documents"}, // unbundle: tenant has not purchased / been granted the docs module
}

func init() { mergeCodeMeta(driveCodeMeta, "drive") }
8 changes: 8 additions & 0 deletions internal/errclass/codemeta_drive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ 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},
// 9000308x: cloud-space explorer commercial plan codes, passed
// through by document backends (slides_ai create, drive import_tasks)
// as HTTP 200 with body code≠0. Billing limits → never retryable.
// 86/87 are plan creation-count quotas; 88 is "docs module not
// purchased/enabled" — a precondition, not a quota.
{90003086, errs.CategoryAPI, errs.SubtypeQuotaExceeded, false},
{90003087, errs.CategoryAPI, errs.SubtypeQuotaExceeded, false},
{90003088, errs.CategoryAPI, errs.SubtypeFailedPrecondition, false},
}
for _, tc := range cases {
t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) {
Expand Down
12 changes: 12 additions & 0 deletions internal/output/lark_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,14 @@ const (
LarkErrMailSendQuotaTenantExt = 1236009 // tenant daily external recipient count exceeded
LarkErrMailQuota = 1236010 // mail quota limit
LarkErrTenantStorageLimit = 1236013 // tenant storage limit exceeded

// Docs/slides commercial plan quota codes from the cloud-space explorer
// service, passed through verbatim by document backends as HTTP 200 with
// body code≠0 (observed on slides_ai create and drive/v1/import_tasks).
// Not retryable: these are billing-plan limits.
LarkErrDocsCreatePremiumQuota = 90003086 // premium plan document creation count limit reached
LarkErrDocsCreateA2Quota = 90003087 // A2 plan document creation count limit reached
LarkErrDocsModuleUnbundled = 90003088 // tenant has not purchased / enabled the docs module
)

// legacyHints supplies the per-code actionable hint string for the legacy
Expand Down Expand Up @@ -118,6 +126,10 @@ var legacyHints = map[int]string{
"width/height must be >= 20 px; offsets must be >= 0 and less than the anchor cell's width/height",
LarkErrDrivePermApplyRateLimit: "permission-apply quota reached: each user may request access on the same document at most 5 times per day; wait or ask the owner directly",
LarkErrDrivePermApplyNotApplicable: "this document does not accept a permission-apply request (common causes: the document is configured to disallow access requests, the caller already holds the permission, or the target type does not support apply); contact the owner directly",

LarkErrDocsCreatePremiumQuota: "document creation quota of the current plan reached: upgrade the plan or delete documents you no longer need to free quota; retrying will not help",
LarkErrDocsCreateA2Quota: "document creation quota of the current plan reached: upgrade the plan or delete documents you no longer need to free quota; retrying will not help",
LarkErrDocsModuleUnbundled: "the tenant has not purchased or enabled the docs module; ask the tenant admin to enable it before creating documents",
}

// ClassifyLarkError maps a Lark API error code + message to the legacy
Expand Down
130 changes: 127 additions & 3 deletions shortcuts/slides/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,73 @@
package slides

import (
"context"
"encoding/xml"
"fmt"
"io"
"net/url"
"regexp"
"strings"
"time"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/shortcuts/common"
)

const (
// slidesRateLimitMaxRetries is the number of automatic retries (beyond the
// initial request) when the API answers 99991400 "request trigger frequency
// limit". The slides batch paths (+create slide loop, placeholder image
// uploads) fire consecutive POSTs and are the dominant 99991400 producers
// in telemetry; a short backoff absorbs the per-second window without
// masking a genuinely saturated tenant.
slidesRateLimitMaxRetries = 2
)

// slidesRateLimitBaseDelay is the initial backoff delay; subsequent retries
// double it (1s, 2s). Mirrors the wiki +node-create lock-contention pattern
// but with a larger base because frequency limits are per-second windows, not
// sub-second lock races. var (not const) only so tests can shrink it.
var slidesRateLimitBaseDelay = 1 * time.Second

// isRateLimitedErr reports whether err is a typed retryable rate-limit error
// (e.g. 99991400), as classified by errclass.BuildAPIError.
func isRateLimitedErr(err error) bool {
p, ok := errs.ProblemOf(err)
return ok && p.Subtype == errs.SubtypeRateLimit && p.Retryable
}

// retryOnRateLimit runs fn, retrying with exponential backoff (1s, 2s) when it
// returns a retryable rate-limit error. Any other outcome — success or a
// different error — is returned immediately. Progress is announced on errOut
// so a user watching a batch upload understands the pause.
func retryOnRateLimit(ctx context.Context, errOut io.Writer, fn func() error) error {
var lastErr error
for attempt := 0; attempt <= slidesRateLimitMaxRetries; attempt++ {
if attempt > 0 {
delay := slidesRateLimitBaseDelay << uint(attempt-1)
// Report the actual code from the error: the retry predicate matches
// any retryable SubtypeRateLimit, not just 99991400.
code := 0
if p, ok := errs.ProblemOf(lastErr); ok {
code = p.Code
}
fmt.Fprintf(errOut, "Rate limited by the API (%d), retrying (attempt %d/%d) in %v...\n",
code, attempt, slidesRateLimitMaxRetries, delay)
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(delay):
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
lastErr = fn()
if lastErr == nil || !isRateLimitedErr(lastErr) {
return lastErr
}
}
return lastErr
}

// presentationRef holds a parsed --presentation input.
//
// Slides shortcuts accept three input shapes:
Expand Down Expand Up @@ -125,8 +183,30 @@
// around `=`); without it we'd silently leave such placeholders unrewritten.
var imgSrcPlaceholderRegex = regexp.MustCompile(`(?s)<img\b[^>]*?\bsrc\s*=\s*(["'])@([^"']+)(["'])`)

// xmlEntityUnescaper reverses the five XML built-in entities in attribute
// values captured from raw slide XML. strings.Replacer scans left-to-right in
// a single pass, so "&amp;lt;" correctly yields "&lt;" (the leading "&amp;"
// is consumed first), matching XML unescape semantics.
var xmlEntityUnescaper = strings.NewReplacer(
"&lt;", "<",
"&gt;", ">",
"&quot;", `"`,
"&apos;", "'",
"&amp;", "&",
)

// placeholderFilePath converts a raw <img src="@..."> capture into the local
// filesystem path it refers to. The capture comes from well-formed XML where
// a literal & must be written &amp; (the precheck enforces this), so the
// entities are decoded before the path touches Stat/upload. Filesystem paths
// containing & are therefore written as e.g. src="@./Q1&amp;Q2.png".
func placeholderFilePath(raw string) string {
return xmlEntityUnescaper.Replace(strings.TrimSpace(raw))
}

// extractImagePlaceholderPaths returns the de-duplicated list of local paths
// referenced via <img src="@path"> in the given slide XML strings.
// referenced via <img src="@path"> in the given slide XML strings, with XML
// built-in entities decoded (see placeholderFilePath).
//
// Order is preserved (first occurrence wins) so dry-run / progress messages are
// stable across runs.
Expand All @@ -141,7 +221,7 @@
// so we filter it here. Treat as malformed XML and skip.
continue
}
path := strings.TrimSpace(m[2])
path := placeholderFilePath(m[2])
if path == "" || seen[path] {
continue
}
Expand Down Expand Up @@ -280,6 +360,47 @@
return xmlFragment[:m[1]] + "<content/>" + afterOpen
}

// checkXMLWellFormed verifies that fragment parses as well-formed XML, using
// the same parser family as the backend (Go encoding/xml). Syntax only —
// element names and attributes are NOT checked against the SML schema, so
// anything passing here can still be rejected server-side for semantic
// reasons; conversely nothing rejected here could ever have succeeded, which
// keeps the false-positive risk at zero.
//
// The backend reports these failures as an opaque 3350001/4001000
// "invalid param" with no position info; catching them locally turns the
// dominant real-world causes (bare & in text, unclosed tags, attribute
// quoting) into actionable messages with a line number.
//
// An <?xml ?> declaration is rejected explicitly: the rendering backend does
// not accept processing instructions on slide fragments (rejects with
// "?xml not provide the implement"). encoding/xml surfaces it as a regular
// ProcInst token, so it needs its own check.
//
// Multiple top-level elements are deliberately allowed — insertion fragments
// may legitimately carry sibling elements.
func checkXMLWellFormed(fragment string) error {
dec := xml.NewDecoder(strings.NewReader(fragment))
for {
tok, err := dec.Token()
if err == io.EOF {
return nil
}
if err != nil {
if syn, ok := err.(*xml.SyntaxError); ok {
return errs.NewValidationError(errs.SubtypeInvalidArgument,
"XML not well-formed at line %d: %s (escape literal & as &amp; and < as &lt; in text)",
syn.Line, syn.Msg)
}
return errs.NewValidationError(errs.SubtypeInvalidArgument, "XML not well-formed: %v", err)

Check warning on line 395 in shortcuts/slides/helpers.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/slides/helpers.go#L395

Added line #L395 was not covered by tests
}
if pi, ok := tok.(xml.ProcInst); ok && strings.EqualFold(pi.Target, "xml") {
return errs.NewValidationError(errs.SubtypeInvalidArgument,
"XML must not contain an <?xml ?> declaration (the slides backend rejects it); remove it and start at the root element")
}
}
}

// replaceImagePlaceholders rewrites <img src="@path"> occurrences in the input
// XML by looking up each path in tokens. Paths missing from the map are left
// untouched (callers should ensure the map is complete).
Expand All @@ -294,7 +415,10 @@
// Mismatched quotes — see extractImagePlaceholderPaths.
return match
}
token, ok := tokens[strings.TrimSpace(path)]
// tokens is keyed by the decoded filesystem path (see
// extractImagePlaceholderPaths), while oldQuoted below must use the
// raw capture so the literal XML text is what gets replaced.
token, ok := tokens[placeholderFilePath(path)]
if !ok {
return match
}
Expand Down
Loading
Loading