Skip to content

Commit 0697b81

Browse files
committed
fix(slides): local XML precheck, 99991400 backoff, quota code registration
Lands the four agreed fixes from the 2026-06-08 slides write-path telemetry analysis: 1. Local XML well-formedness precheck (shortcuts/slides/) - checkXMLWellFormed: pure syntax validation via stdlib encoding/xml (same parser family as the backend, false-positive risk ~0); explicitly rejects <?xml ?> declarations; deliberately allows multiple top-level elements (legal in block_insert fragments) - wired into +create --slides (at Validate, so a bad slide no longer leaves a half-built deck) and +replace-slide --parts replacement/insertion; errors carry line numbers + escaping guidance, rejected locally with zero API calls 2. 99991400 rate-limit backoff (retryOnRateLimit) - the code was registered Retryable:true but no slides loop actually retried, so one frequency-window hit aborted the whole batch - up to 2 retries with 1s/2s backoff, announced on stderr, context-cancellable; wired into the +create slide POST loop and uploadSlidesMedia (+media-upload and the placeholder upload loop) - upload switched to UploadDriveMediaAllTyped (retry match requires typed errors; aligns with the slides typed migration) 3. Quota codes 90003086/87/88 registered (internal/errclass + output) - cloud-space explorer billing codes passed through as HTTP 200 with body code!=0; previously fell to SubtypeUnknown - now CategoryAPI + quota_exceeded, not retryable, with per-code hints (upgrade plan / free quota / ask admin to enable the module) 4. lark-slides skill tag-whitelist ban (skills/lark-slides/) - quick-ref: never write tags outside the whitelist, name the six confirmed-rejected tags (audio/video/timeline/animation/trigger/ header), substitution table, escaping rules - removed <?xml ?> declarations from all examples (contradicted backend behavior and the new precheck) Tested with unit + httpmock integration tests, plus live verification against the real feishu.cn API: all precheck negatives rejected locally, no false positives on real create/replace, and 18 concurrent uploads hit 3 real 99991400 responses which all retried and succeeded (18/18).
1 parent e794fd5 commit 0697b81

13 files changed

Lines changed: 532 additions & 18 deletions

internal/errclass/codemeta_drive.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,19 @@ import "github.com/larksuite/cli/errs"
1212
var driveCodeMeta = map[int]CodeMeta{
1313
1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload)
1414
1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters"
15+
16+
// Commercial plan codes returned by the cloud-space explorer service
17+
// and passed through verbatim by document backends (observed via
18+
// slides_ai create — engine log "A2 create quota exceeded, code=90003087"
19+
// — and drive/v1/import_tasks, both as HTTP 200 with body code≠0).
20+
// Plan/billing limits: retrying can never succeed. 90003086/90003087 are
21+
// plan creation-count quotas (upgrade the plan or free quota); 90003088 is
22+
// not a quota at all — the tenant has not purchased/enabled the docs
23+
// module, so it maps to FailedPrecondition (admin must enable the module)
24+
// rather than QuotaExceeded, whose default hint suggests freeing quota.
25+
90003086: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded}, // premium plan creation count limit reached
26+
90003087: {Category: errs.CategoryAPI, Subtype: errs.SubtypeQuotaExceeded}, // A2 plan creation count limit reached
27+
90003088: {Category: errs.CategoryAPI, Subtype: errs.SubtypeFailedPrecondition}, // unbundle: tenant has not purchased / been granted the docs module
1528
}
1629

1730
func init() { mergeCodeMeta(driveCodeMeta, "drive") }

internal/errclass/codemeta_drive_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ func TestLookupCodeMeta_DriveCodes(t *testing.T) {
2727
// 1069302: comment endpoint's opaque "Invalid or missing parameters"
2828
// (shortcuts/drive/drive_add_comment.go) → API-side parameter rejection.
2929
{1069302, errs.CategoryAPI, errs.SubtypeInvalidParameters, false},
30+
// 9000308x: cloud-space explorer commercial plan codes, passed
31+
// through by document backends (slides_ai create, drive import_tasks)
32+
// as HTTP 200 with body code≠0. Billing limits → never retryable.
33+
// 86/87 are plan creation-count quotas; 88 is "docs module not
34+
// purchased/enabled" — a precondition, not a quota.
35+
{90003086, errs.CategoryAPI, errs.SubtypeQuotaExceeded, false},
36+
{90003087, errs.CategoryAPI, errs.SubtypeQuotaExceeded, false},
37+
{90003088, errs.CategoryAPI, errs.SubtypeFailedPrecondition, false},
3038
}
3139
for _, tc := range cases {
3240
t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) {

internal/output/lark_errors.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,14 @@ const (
8484
LarkErrMailSendQuotaTenantExt = 1236009 // tenant daily external recipient count exceeded
8585
LarkErrMailQuota = 1236010 // mail quota limit
8686
LarkErrTenantStorageLimit = 1236013 // tenant storage limit exceeded
87+
88+
// Docs/slides commercial plan quota codes from the cloud-space explorer
89+
// service, passed through verbatim by document backends as HTTP 200 with
90+
// body code≠0 (observed on slides_ai create and drive/v1/import_tasks).
91+
// Not retryable: these are billing-plan limits.
92+
LarkErrDocsCreatePremiumQuota = 90003086 // premium plan document creation count limit reached
93+
LarkErrDocsCreateA2Quota = 90003087 // A2 plan document creation count limit reached
94+
LarkErrDocsModuleUnbundled = 90003088 // tenant has not purchased / enabled the docs module
8795
)
8896

8997
// legacyHints supplies the per-code actionable hint string for the legacy
@@ -118,6 +126,10 @@ var legacyHints = map[int]string{
118126
"width/height must be >= 20 px; offsets must be >= 0 and less than the anchor cell's width/height",
119127
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",
120128
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",
129+
130+
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",
131+
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",
132+
LarkErrDocsModuleUnbundled: "the tenant has not purchased or enabled the docs module; ask the tenant admin to enable it before creating documents",
121133
}
122134

123135
// ClassifyLarkError maps a Lark API error code + message to the legacy

shortcuts/slides/helpers.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,67 @@
44
package slides
55

66
import (
7+
"context"
8+
"encoding/xml"
79
"fmt"
10+
"io"
811
"net/url"
912
"regexp"
1013
"strings"
14+
"time"
1115

1216
"github.com/larksuite/cli/errs"
1317
"github.com/larksuite/cli/shortcuts/common"
1418
)
1519

20+
const (
21+
// slidesRateLimitMaxRetries is the number of automatic retries (beyond the
22+
// initial request) when the API answers 99991400 "request trigger frequency
23+
// limit". The slides batch paths (+create slide loop, placeholder image
24+
// uploads) fire consecutive POSTs and are the dominant 99991400 producers
25+
// in telemetry; a short backoff absorbs the per-second window without
26+
// masking a genuinely saturated tenant.
27+
slidesRateLimitMaxRetries = 2
28+
)
29+
30+
// slidesRateLimitBaseDelay is the initial backoff delay; subsequent retries
31+
// double it (1s, 2s). Mirrors the wiki +node-create lock-contention pattern
32+
// but with a larger base because frequency limits are per-second windows, not
33+
// sub-second lock races. var (not const) only so tests can shrink it.
34+
var slidesRateLimitBaseDelay = 1 * time.Second
35+
36+
// isRateLimitedErr reports whether err is a typed retryable rate-limit error
37+
// (e.g. 99991400), as classified by errclass.BuildAPIError.
38+
func isRateLimitedErr(err error) bool {
39+
p, ok := errs.ProblemOf(err)
40+
return ok && p.Subtype == errs.SubtypeRateLimit && p.Retryable
41+
}
42+
43+
// retryOnRateLimit runs fn, retrying with exponential backoff (1s, 2s) when it
44+
// returns a retryable rate-limit error. Any other outcome — success or a
45+
// different error — is returned immediately. Progress is announced on errOut
46+
// so a user watching a batch upload understands the pause.
47+
func retryOnRateLimit(ctx context.Context, errOut io.Writer, fn func() error) error {
48+
var lastErr error
49+
for attempt := 0; attempt <= slidesRateLimitMaxRetries; attempt++ {
50+
if attempt > 0 {
51+
delay := slidesRateLimitBaseDelay << uint(attempt-1)
52+
fmt.Fprintf(errOut, "Rate limited by the API (99991400), retrying (attempt %d/%d) in %v...\n",
53+
attempt, slidesRateLimitMaxRetries, delay)
54+
select {
55+
case <-ctx.Done():
56+
return ctx.Err()
57+
case <-time.After(delay):
58+
}
59+
}
60+
lastErr = fn()
61+
if lastErr == nil || !isRateLimitedErr(lastErr) {
62+
return lastErr
63+
}
64+
}
65+
return lastErr
66+
}
67+
1668
// presentationRef holds a parsed --presentation input.
1769
//
1870
// Slides shortcuts accept three input shapes:
@@ -280,6 +332,47 @@ func ensureShapeHasContent(xmlFragment string) string {
280332
return xmlFragment[:m[1]] + "<content/>" + afterOpen
281333
}
282334

335+
// checkXMLWellFormed verifies that fragment parses as well-formed XML, using
336+
// the same parser family as the backend (Go encoding/xml). Syntax only —
337+
// element names and attributes are NOT checked against the SML schema, so
338+
// anything passing here can still be rejected server-side for semantic
339+
// reasons; conversely nothing rejected here could ever have succeeded, which
340+
// keeps the false-positive risk at zero.
341+
//
342+
// The backend reports these failures as an opaque 3350001/4001000
343+
// "invalid param" with no position info; catching them locally turns the
344+
// dominant real-world causes (bare & in text, unclosed tags, attribute
345+
// quoting) into actionable messages with a line number.
346+
//
347+
// An <?xml ?> declaration is rejected explicitly: the rendering backend does
348+
// not accept processing instructions on slide fragments (rejects with
349+
// "?xml not provide the implement"). encoding/xml surfaces it as a regular
350+
// ProcInst token, so it needs its own check.
351+
//
352+
// Multiple top-level elements are deliberately allowed — insertion fragments
353+
// may legitimately carry sibling elements.
354+
func checkXMLWellFormed(fragment string) error {
355+
dec := xml.NewDecoder(strings.NewReader(fragment))
356+
for {
357+
tok, err := dec.Token()
358+
if err == io.EOF {
359+
return nil
360+
}
361+
if err != nil {
362+
if syn, ok := err.(*xml.SyntaxError); ok {
363+
return errs.NewValidationError(errs.SubtypeInvalidArgument,
364+
"XML not well-formed at line %d: %s (escape literal & as &amp; and < as &lt; in text)",
365+
syn.Line, syn.Msg)
366+
}
367+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "XML not well-formed: %v", err)
368+
}
369+
if pi, ok := tok.(xml.ProcInst); ok && strings.EqualFold(pi.Target, "xml") {
370+
return errs.NewValidationError(errs.SubtypeInvalidArgument,
371+
"XML must not contain an <?xml ?> declaration (the slides backend rejects it); remove it and start at the root element")
372+
}
373+
}
374+
}
375+
283376
// replaceImagePlaceholders rewrites <img src="@path"> occurrences in the input
284377
// XML by looking up each path in tokens. Paths missing from the map are left
285378
// untouched (callers should ensure the map is complete).

shortcuts/slides/helpers_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@
44
package slides
55

66
import (
7+
"bytes"
8+
"context"
9+
"errors"
710
"reflect"
811
"strings"
912
"testing"
13+
"time"
14+
15+
"github.com/larksuite/cli/errs"
1016
)
1117

1218
func TestParsePresentationRef(t *testing.T) {
@@ -413,3 +419,145 @@ func TestEnsureXMLRootID(t *testing.T) {
413419
})
414420
}
415421
}
422+
423+
func TestCheckXMLWellFormed(t *testing.T) {
424+
t.Parallel()
425+
426+
tests := []struct {
427+
name string
428+
in string
429+
wantErr string
430+
}{
431+
{name: "simple element", in: `<shape type="rect"><content/></shape>`},
432+
{name: "nested with attributes", in: `<slide><shape type="text"><content><p>hi</p></content></shape></slide>`},
433+
// Insertion fragments may carry sibling top-level elements; the decoder
434+
// must not enforce a single document element.
435+
{name: "multiple top-level elements", in: `<p>a</p><p>b</p>`},
436+
{name: "escaped entities", in: `<p>A &amp; B &lt;tag&gt; &quot;q&quot;</p>`},
437+
{name: "CDATA with raw ampersand", in: `<p><![CDATA[a & b < c]]></p>`},
438+
{name: "comment", in: `<!-- note --><shape/>`},
439+
{name: "img placeholder attr", in: `<img src="@./local.png" width="100"/>`},
440+
{name: "unicode text", in: `<p>项目汇报 🎯</p>`},
441+
442+
// Top CLI-path failure cause in engine logs: bare & in text.
443+
{name: "bare ampersand", in: `<p>Q & A</p>`, wantErr: "line 1"},
444+
{name: "bare ampersand multiline", in: "<slide>\n<p>R&D</p>\n</slide>", wantErr: "line 2"},
445+
{name: "unclosed tag", in: `<shape><content></shape>`, wantErr: "not well-formed"},
446+
{name: "unquoted attribute", in: `<shape type=rect/>`, wantErr: "not well-formed"},
447+
{name: "stray closing tag", in: `<p>hi</p></div>`, wantErr: "not well-formed"},
448+
{name: "undefined entity", in: `<p>a&nbsp;b</p>`, wantErr: "not well-formed"},
449+
450+
// nodeserver rejects processing instructions ("?xml not provide the
451+
// implement"); reject the declaration locally regardless of position.
452+
{name: "xml declaration", in: `<?xml version="1.0"?><shape/>`, wantErr: "declaration"},
453+
{name: "xml declaration with encoding", in: `<?xml version="1.0" encoding="UTF-8"?><slide/>`, wantErr: "declaration"},
454+
{name: "uppercase xml declaration", in: `<?XML version="1.0"?><shape/>`, wantErr: "declaration"},
455+
}
456+
457+
for _, tt := range tests {
458+
t.Run(tt.name, func(t *testing.T) {
459+
t.Parallel()
460+
err := checkXMLWellFormed(tt.in)
461+
if tt.wantErr == "" {
462+
if err != nil {
463+
t.Fatalf("unexpected err: %v", err)
464+
}
465+
return
466+
}
467+
if err == nil {
468+
t.Fatalf("want error containing %q, got nil", tt.wantErr)
469+
}
470+
if !strings.Contains(err.Error(), tt.wantErr) {
471+
t.Fatalf("want error containing %q, got %q", tt.wantErr, err.Error())
472+
}
473+
})
474+
}
475+
}
476+
477+
// TestRetryOnRateLimit verifies the 99991400 backoff helper: retryable
478+
// rate-limit errors are retried with backoff, anything else returns
479+
// immediately, and exhaustion surfaces the last rate-limit error.
480+
//
481+
// Not parallel: shrinks the package-level slidesRateLimitBaseDelay.
482+
func TestRetryOnRateLimit(t *testing.T) {
483+
restore := slidesRateLimitBaseDelay
484+
slidesRateLimitBaseDelay = time.Millisecond
485+
t.Cleanup(func() { slidesRateLimitBaseDelay = restore })
486+
487+
rateLimitErr := func() error {
488+
return errs.NewAPIError(errs.SubtypeRateLimit, "request trigger frequency limit").WithRetryable()
489+
}
490+
491+
t.Run("success without retry", func(t *testing.T) {
492+
var errOut bytes.Buffer
493+
calls := 0
494+
err := retryOnRateLimit(context.Background(), &errOut, func() error {
495+
calls++
496+
return nil
497+
})
498+
if err != nil || calls != 1 {
499+
t.Fatalf("err=%v calls=%d, want nil/1", err, calls)
500+
}
501+
if errOut.Len() != 0 {
502+
t.Fatalf("no retry message expected, got: %s", errOut.String())
503+
}
504+
})
505+
506+
t.Run("succeeds after transient rate limit", func(t *testing.T) {
507+
var errOut bytes.Buffer
508+
calls := 0
509+
err := retryOnRateLimit(context.Background(), &errOut, func() error {
510+
calls++
511+
if calls <= 2 {
512+
return rateLimitErr()
513+
}
514+
return nil
515+
})
516+
if err != nil || calls != 3 {
517+
t.Fatalf("err=%v calls=%d, want nil/3", err, calls)
518+
}
519+
if !strings.Contains(errOut.String(), "retrying") {
520+
t.Fatalf("expected retry announcement, got: %s", errOut.String())
521+
}
522+
})
523+
524+
t.Run("exhaustion returns last rate-limit error", func(t *testing.T) {
525+
var errOut bytes.Buffer
526+
calls := 0
527+
err := retryOnRateLimit(context.Background(), &errOut, func() error {
528+
calls++
529+
return rateLimitErr()
530+
})
531+
if err == nil || !isRateLimitedErr(err) {
532+
t.Fatalf("want rate-limit error after exhaustion, got: %v", err)
533+
}
534+
if calls != slidesRateLimitMaxRetries+1 {
535+
t.Fatalf("calls=%d, want %d", calls, slidesRateLimitMaxRetries+1)
536+
}
537+
})
538+
539+
t.Run("non-rate-limit error returns immediately", func(t *testing.T) {
540+
var errOut bytes.Buffer
541+
calls := 0
542+
boom := errs.NewAPIError(errs.SubtypeNotFound, "not found")
543+
err := retryOnRateLimit(context.Background(), &errOut, func() error {
544+
calls++
545+
return boom
546+
})
547+
if !errors.Is(err, boom) || calls != 1 {
548+
t.Fatalf("err=%v calls=%d, want boom/1", err, calls)
549+
}
550+
})
551+
552+
t.Run("cancelled context aborts the backoff wait", func(t *testing.T) {
553+
var errOut bytes.Buffer
554+
ctx, cancel := context.WithCancel(context.Background())
555+
cancel()
556+
err := retryOnRateLimit(ctx, &errOut, func() error {
557+
return rateLimitErr()
558+
})
559+
if !errors.Is(err, context.Canceled) {
560+
t.Fatalf("want context.Canceled, got: %v", err)
561+
}
562+
})
563+
}

shortcuts/slides/slides_create.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ var SlidesCreate = common.Shortcut{
5050
if len(slides) > maxSlidesPerCreate {
5151
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate).WithParam("--slides")
5252
}
53+
// Well-formedness precheck before any API call: a syntax error in
54+
// slide N would otherwise create the presentation and then fail on
55+
// the slide POST with an opaque backend "invalid param", leaving a
56+
// partially-built deck behind.
57+
for i, slideXML := range slides {
58+
if err := checkXMLWellFormed(slideXML); err != nil {
59+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides[%d]: %v", i, err).WithParam("--slides").WithCause(err)
60+
}
61+
}
5362
// Validate placeholder paths up front so we don't create a presentation
5463
// only to fail mid-way on a missing local file.
5564
for _, path := range extractImagePlaceholderPaths(slides) {
@@ -183,14 +192,22 @@ var SlidesCreate = common.Shortcut{
183192

184193
var slideIDs []string
185194
for i, slideXML := range slides {
186-
slideData, err := runtime.CallAPITyped(
187-
"POST",
188-
slideURL,
189-
map[string]interface{}{"revision_id": -1},
190-
map[string]interface{}{
191-
"slide": map[string]interface{}{"content": slideXML},
192-
},
193-
)
195+
var slideData map[string]interface{}
196+
// Consecutive slide POSTs are the main 99991400 producer in
197+
// telemetry; absorb the per-second frequency window with a
198+
// short backoff instead of aborting the whole batch.
199+
err := retryOnRateLimit(ctx, runtime.IO().ErrOut, func() error {
200+
var callErr error
201+
slideData, callErr = runtime.CallAPITyped(
202+
"POST",
203+
slideURL,
204+
map[string]interface{}{"revision_id": -1},
205+
map[string]interface{}{
206+
"slide": map[string]interface{}{"content": slideXML},
207+
},
208+
)
209+
return callErr
210+
})
194211
if err != nil {
195212
return appendSlidesProgressHint(err, fmt.Sprintf("adding slide %d/%d failed; presentation %s was created, %d slide(s) added before failure", i+1, len(slides), presentationID, i))
196213
}

0 commit comments

Comments
 (0)