Skip to content

Commit d0e49e1

Browse files
bubbmon233bubbmon233
committed
mail: refine local rate limits
1 parent a140235 commit d0e49e1

15 files changed

Lines changed: 890 additions & 98 deletions

‎internal/client/api_errors.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"io"
1212
"strings"
1313

14+
"github.com/larksuite/cli/internal/mailratelimit"
1415
"github.com/larksuite/cli/internal/output"
1516
)
1617

@@ -23,8 +24,7 @@ func WrapDoAPIError(err error) error {
2324
if err == nil {
2425
return nil
2526
}
26-
var exitErr *output.ExitError
27-
if errors.As(err, &exitErr) {
27+
if mailratelimit.IsLocalRateLimit(err) {
2828
return err
2929
}
3030
if isJSONDecodeError(err, false) {

‎internal/client/api_errors_test.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,11 @@ func TestWrapDoAPIError_SyntaxErrorIsAPIDiagnostic(t *testing.T) {
4949
}
5050
}
5151

52-
func TestWrapDoAPIError_PreservesExitError(t *testing.T) {
53-
original := output.ErrAPI(output.LarkErrRateLimit, "rate limited", map[string]any{"retry_after_ms": 100})
52+
func TestWrapDoAPIError_PreservesLocalMailRateLimit(t *testing.T) {
53+
original := output.ErrAPI(output.LarkErrRateLimit, "rate limited", map[string]any{
54+
"source": "local_mailratelimit",
55+
"retry_after_ms": 100,
56+
})
5457
err := WrapDoAPIError(original)
5558
if err != original {
5659
t.Fatalf("WrapDoAPIError returned %p, want original %p", err, original)
@@ -61,6 +64,18 @@ func TestWrapDoAPIError_PreservesExitError(t *testing.T) {
6164
}
6265
}
6366

67+
func TestWrapDoAPIError_WrapsOtherExitErrorsAsNetwork(t *testing.T) {
68+
original := output.ErrAuth("no access token available")
69+
err := WrapDoAPIError(original)
70+
if err == original {
71+
t.Fatal("expected non-local mail ExitError to be wrapped")
72+
}
73+
var exitErr *output.ExitError
74+
if !errors.As(err, &exitErr) || exitErr.Code != output.ExitNetwork {
75+
t.Fatalf("err = %v, want network ExitError", err)
76+
}
77+
}
78+
6479
func TestWrapJSONResponseParseError_UnexpectedEOFIsAPIDiagnostic(t *testing.T) {
6580
err := WrapJSONResponseParseError(io.ErrUnexpectedEOF, []byte("{"))
6681
if err == nil {

‎internal/client/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ func (c *APIClient) paginateLoop(ctx context.Context, request RawApiRequest, opt
324324
ExtraOpts: request.ExtraOpts,
325325
})
326326
if err != nil {
327-
if page == 1 {
327+
if page == 1 || mailratelimit.IsLocalRateLimit(err) {
328328
return nil, err
329329
}
330330
fmt.Fprintf(c.ErrOut, "[page %d] error, stopping pagination\n", page)

‎internal/client/client_test.go‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,48 @@ func TestPaginateAll_PageLimitStopsPagination(t *testing.T) {
247247
}
248248
}
249249

250+
func TestPaginateAll_ReturnsMailRateLimitAfterFirstPage(t *testing.T) {
251+
now := time.Unix(100, 0)
252+
rule := mailratelimit.Rule{
253+
Method: "GET",
254+
CanonicalPath: "/open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages",
255+
Window: 2 * time.Second,
256+
Limit: 1,
257+
Scope: mailratelimit.ScopeApp,
258+
}
259+
restore := mailratelimit.SetDefaultLimiterForTest(mailratelimit.NewLimiterForDir(t.TempDir(), []mailratelimit.Rule{rule}, func() time.Time { return now }))
260+
defer restore()
261+
262+
apiCalls := 0
263+
ac, _ := newTestAPIClient(t, roundTripFunc(func(req *http.Request) (*http.Response, error) {
264+
apiCalls++
265+
return jsonResponse(map[string]interface{}{
266+
"code": 0, "msg": "ok",
267+
"data": map[string]interface{}{
268+
"items": []interface{}{map[string]interface{}{"id": apiCalls}},
269+
"has_more": true,
270+
"page_token": "next",
271+
},
272+
}), nil
273+
}))
274+
275+
_, err := ac.PaginateAll(context.Background(), RawApiRequest{
276+
Method: "GET",
277+
URL: "/open-apis/mail/v1/user_mailboxes/me/messages",
278+
As: core.AsBot,
279+
}, PaginationOptions{PageLimit: 0, PageDelay: -1})
280+
if err == nil {
281+
t.Fatal("expected local mail rate limit")
282+
}
283+
var exitErr *output.ExitError
284+
if !errors.As(err, &exitErr) || exitErr.Detail == nil || exitErr.Detail.Type != "rate_limit" {
285+
t.Fatalf("err = %v, want rate_limit ExitError", err)
286+
}
287+
if apiCalls != 1 {
288+
t.Fatalf("api calls = %d, want 1", apiCalls)
289+
}
290+
}
291+
250292
func TestPaginateAll_NaturalEndClearsPageToken(t *testing.T) {
251293
apiCalls := 0
252294
rt := roundTripFunc(func(req *http.Request) (*http.Response, error) {
@@ -499,7 +541,7 @@ func TestDoSDKRequest_MailRateLimitRunsBeforeTokenAndSDK(t *testing.T) {
499541
httpCalls := 0
500542
ac, _ := newTestAPIClient(t, roundTripFunc(func(req *http.Request) (*http.Response, error) {
501543
httpCalls++
502-
return jsonResponse(map[string]interface{}{"code": 0, "msg": "ok"})
544+
return jsonResponse(map[string]interface{}{"code": 0, "msg": "ok"}), nil
503545
}))
504546
resolver := &countingTokenResolver{}
505547
ac.Credential = credential.NewCredentialProvider(nil, nil, resolver, nil)
@@ -543,7 +585,7 @@ func TestDoSDKRequest_NonMailAndUnconfiguredMailStillSend(t *testing.T) {
543585
httpCalls := 0
544586
ac, _ := newTestAPIClient(t, roundTripFunc(func(req *http.Request) (*http.Response, error) {
545587
httpCalls++
546-
return jsonResponse(map[string]interface{}{"code": 0, "msg": "ok"})
588+
return jsonResponse(map[string]interface{}{"code": 0, "msg": "ok"}), nil
547589
}))
548590

549591
for _, path := range []string{

‎internal/mailratelimit/canonical.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const mailAPIPrefix = "/open-apis/mail/"
1515

1616
type compiledRule struct {
1717
rule *Rule
18+
method string
1819
pattern *regexp.Regexp
1920
}
2021

@@ -23,11 +24,11 @@ var compiledBuiltinRules = compileRules(builtinRules)
2324
func Canonicalize(method, rawPath string) (string, *Rule, bool) {
2425
method = normalizeMethod(method)
2526
path := normalizePath(rawPath)
26-
if !strings.HasPrefix(path, mailAPIPrefix) {
27+
if !isMailAPIPath(path) {
2728
return "", nil, false
2829
}
2930
for _, entry := range compiledBuiltinRules {
30-
if method != normalizeMethod(entry.rule.Method) {
31+
if method != entry.method {
3132
continue
3233
}
3334
if path == entry.rule.CanonicalPath || entry.pattern.MatchString(path) {
@@ -37,6 +38,10 @@ func Canonicalize(method, rawPath string) (string, *Rule, bool) {
3738
return "", nil, false
3839
}
3940

41+
func isMailAPIPath(path string) bool {
42+
return strings.HasPrefix(path, mailAPIPrefix)
43+
}
44+
4045
func normalizePath(rawPath string) string {
4146
rawPath = strings.TrimSpace(rawPath)
4247
if rawPath == "" {
@@ -57,6 +62,7 @@ func compileRules(rules []Rule) []compiledRule {
5762
rule := &rules[i]
5863
compiled = append(compiled, compiledRule{
5964
rule: rule,
65+
method: normalizeMethod(rule.Method),
6066
pattern: regexp.MustCompile("^" + canonicalPattern(rule.CanonicalPath) + "$"),
6167
})
6268
}

‎internal/mailratelimit/canonical_test.go‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package mailratelimit
55

66
import (
7+
"strings"
78
"testing"
89
"time"
910
)
@@ -35,6 +36,13 @@ func TestCanonicalize(t *testing.T) {
3536
want: "/open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/:message_id",
3637
wantMatch: true,
3738
},
39+
{
40+
name: "relative path query and fragment canonicalize",
41+
method: "GET",
42+
path: "/open-apis/mail/v1/user_mailboxes/me/messages/msg_1?format=metadata#body",
43+
want: "/open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/:message_id",
44+
wantMatch: true,
45+
},
3846
{
3947
name: "concrete batch get canonicalizes",
4048
method: "POST",
@@ -77,34 +85,66 @@ func TestCanonicalize(t *testing.T) {
7785
}
7886

7987
func TestBuiltinRulesUseConfirmedThreshold(t *testing.T) {
80-
want := map[string]bool{
81-
"GET /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/:message_id": false,
82-
"POST /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/batch_get": false,
83-
"GET /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages": false,
84-
"POST /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/search": false,
88+
type threshold struct {
89+
window time.Duration
90+
limit int
8591
}
86-
if len(builtinRules) != len(want) {
87-
t.Fatalf("builtinRules len = %d, want %d", len(builtinRules), len(want))
92+
want := map[string][]threshold{
93+
"GET /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/:message_id": {
94+
{window: time.Minute, limit: 100},
95+
},
96+
"POST /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages/batch_get": {
97+
{window: time.Second, limit: 10},
98+
},
99+
"GET /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/messages": {
100+
{window: time.Second, limit: 10},
101+
},
102+
"POST /open-apis/mail/v1/user_mailboxes/:user_mailbox_id/search": {
103+
{window: time.Minute, limit: 1000},
104+
{window: time.Second, limit: 50},
105+
},
88106
}
107+
seen := make(map[string][]threshold)
89108
for _, rule := range builtinRules {
90109
key := rule.Method + " " + rule.CanonicalPath
91110
if _, ok := want[key]; !ok {
92111
t.Fatalf("unexpected builtin rule %s", key)
93112
}
94-
if rule.Window != time.Minute {
95-
t.Fatalf("%s window = %s, want 1m", key, rule.Window)
113+
if rule.Window <= 0 {
114+
t.Fatalf("%s window must be positive", key)
96115
}
97-
if rule.Limit != 180 {
98-
t.Fatalf("%s limit = %d, want 180", key, rule.Limit)
116+
if rule.Limit <= 0 {
117+
t.Fatalf("%s limit must be positive", key)
99118
}
100119
if rule.Scope != ScopeApp {
101120
t.Fatalf("%s scope = %q, want %q", key, rule.Scope, ScopeApp)
102121
}
103-
want[key] = true
122+
if rule.Method == "" {
123+
t.Fatalf("%s method must not be empty", key)
124+
}
125+
if !strings.HasPrefix(rule.CanonicalPath, "/open-apis/mail/") {
126+
t.Fatalf("%s canonical path must be under /open-apis/mail/", key)
127+
}
128+
seen[key] = append(seen[key], threshold{window: rule.Window, limit: rule.Limit})
129+
}
130+
if len(builtinRules) != 5 {
131+
t.Fatalf("builtinRules len = %d, want 5", len(builtinRules))
104132
}
105-
for key, seen := range want {
106-
if !seen {
133+
for key, thresholds := range want {
134+
if len(seen[key]) != len(thresholds) {
107135
t.Fatalf("missing builtin rule %s", key)
108136
}
137+
for _, threshold := range thresholds {
138+
found := false
139+
for _, got := range seen[key] {
140+
if got == threshold {
141+
found = true
142+
break
143+
}
144+
}
145+
if !found {
146+
t.Fatalf("%s missing threshold window=%s limit=%d; got %#v", key, threshold.window, threshold.limit, seen[key])
147+
}
148+
}
109149
}
110150
}

‎internal/mailratelimit/errors.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,53 @@
44
package mailratelimit
55

66
import (
7+
"errors"
78
"fmt"
89
"math"
910
"time"
1011

1112
"github.com/larksuite/cli/internal/output"
1213
)
1314

15+
const errorSource = "local_mailratelimit"
16+
1417
func newRateLimitError(rule *Rule, retryAfter time.Duration) error {
1518
retryAfterMs := RetryAfterMs(retryAfter)
1619
windowSeconds := int(math.Ceil(rule.Window.Seconds()))
1720
msg := fmt.Sprintf("local mail rate limit: %s %s exceeded %d requests per %ds",
1821
rule.Method, rule.CanonicalPath, rule.Limit, windowSeconds)
1922
return output.ErrAPI(output.LarkErrRateLimit, msg, map[string]any{
20-
"source": "local_mailratelimit",
23+
"source": errorSource,
2124
"retry_after_ms": retryAfterMs,
2225
"method": rule.Method,
2326
"api_path": rule.CanonicalPath,
2427
})
2528
}
2629

30+
func newUnsupportedScopeError(rule *Rule) error {
31+
return output.ErrWithHint(output.ExitInternal, "internal",
32+
fmt.Sprintf("unsupported mail rate limit scope %q for %s %s", rule.Scope, rule.Method, rule.CanonicalPath),
33+
"fix the builtin mail rate limit rule configuration")
34+
}
35+
36+
func newInvalidRuleError(rule *Rule, reason string) error {
37+
return output.ErrWithHint(output.ExitInternal, "internal",
38+
fmt.Sprintf("invalid mail rate limit rule for %s %s: %s", rule.Method, rule.CanonicalPath, reason),
39+
"fix the builtin mail rate limit rule configuration")
40+
}
41+
42+
func IsLocalRateLimit(err error) bool {
43+
var exitErr *output.ExitError
44+
if !errors.As(err, &exitErr) || exitErr.Detail == nil {
45+
return false
46+
}
47+
if exitErr.Detail.Type != "rate_limit" || exitErr.Detail.Code != output.LarkErrRateLimit {
48+
return false
49+
}
50+
detail, ok := exitErr.Detail.Detail.(map[string]any)
51+
return ok && detail["source"] == errorSource
52+
}
53+
2754
func RetryAfterMs(d time.Duration) int64 {
2855
if d <= 0 {
2956
return 1

0 commit comments

Comments
 (0)