diff --git a/shortcuts/doc/doc_errors.go b/shortcuts/doc/doc_errors.go index 9f9c0419c8..b6726e763a 100644 --- a/shortcuts/doc/doc_errors.go +++ b/shortcuts/doc/doc_errors.go @@ -9,10 +9,17 @@ import ( "net/http" "strings" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) +const ( + docMediaAppScopeHint = "stop retrying now; ask the app developer to apply for the required scope(s), and retry only after they have been approved and enabled" + docMediaRateLimitHint = "the request was rate limited; stop immediate retries and retry later using exponential backoff with jitter" +) + // wrapDocNetworkErr returns err unchanged when it is already a typed errs.* // error (preserving its subtype / code / log_id from the runtime boundary), // and only wraps a raw, unclassified error as a transport-level network error. @@ -23,6 +30,57 @@ func wrapDocNetworkErr(err error, format string, args ...any) error { return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) } +// classifyDocMediaStreamError recovers the two business errors this shortcut +// promises to guide from DoStream's current "HTTP : " transport +// message. This compatibility shim is deliberately local to Doc media reads: +// expand it only if DoStream exposes a structured response body or another Doc +// streaming command adopts the same recovery contract. +func classifyDocMediaStreamError(runtime *common.RuntimeContext, err error) error { + problem, ok := errs.ProblemOf(err) + if runtime == nil || !ok || problem == nil || + problem.Category != errs.CategoryNetwork || + problem.Subtype != errs.SubtypeNetworkTransport || + problem.Code < http.StatusBadRequest { + return err + } + + prefix := fmt.Sprintf("HTTP %d: ", problem.Code) + if !strings.HasPrefix(problem.Message, prefix) { + return err + } + body := strings.TrimSpace(strings.TrimPrefix(problem.Message, prefix)) + if !strings.HasPrefix(body, "{") { + return err + } + + header := make(http.Header) + header.Set("Content-Type", "application/json") + if problem.LogID != "" { + header.Set(larkcore.HttpHeaderKeyLogId, problem.LogID) + } + _, classified := runtime.ClassifyAPIResponse(&larkcore.ApiResp{ + StatusCode: problem.Code, + Header: header, + RawBody: []byte(body), + }) + classifiedProblem, classifiedOK := errs.ProblemOf(classified) + if !classifiedOK || classifiedProblem == nil || + (classifiedProblem.Code != 99991672 && classifiedProblem.Code != 99991400) { + return err + } + + var permissionErr *errs.PermissionError + if errors.As(classified, &permissionErr) { + permissionErr.WithCause(err) + return classified + } + var apiErr *errs.APIError + if errors.As(classified, &apiErr) { + apiErr.WithCause(err) + } + return classified +} + // withDocMediaDownloadRecoveryHint keeps the final download error intact while // adding recovery guidance for media permission and throttling failures. // Whiteboard downloads use a different API and must not be redirected to the @@ -41,15 +99,31 @@ func withDocMediaDownloadRecoveryHint(err error, mediaType string) error { hint := fmt.Sprintf("Direct document media download returned HTTP 403. To preview the image or file content, try `lark-cli docs +media-preview --token %s --output `.", tokenArg) appendDocRecoveryHint(problem, hint) } + if problem.Code == 99991672 && !strings.Contains(problem.Hint, "stop retrying now") { + appendDocRecoveryHint(problem, docMediaAppScopeHint) + } - if docMediaDownloadIsRateLimit(problem) && !strings.Contains(problem.Hint, "exponential backoff") { - const hint = "Document media download was rate limited; stop immediate retries and retry later with exponential backoff." - appendDocRecoveryHint(problem, hint) + if docMediaIsRateLimit(problem) && !strings.Contains(problem.Hint, "exponential backoff") { + appendDocRecoveryHint(problem, docMediaRateLimitHint) + } + return err +} + +func withDocMediaPreviewRecoveryHint(err error) error { + problem, ok := errs.ProblemOf(err) + if !ok || problem == nil { + return err + } + if problem.Code == 99991672 && !strings.Contains(problem.Hint, "stop retrying now") { + appendDocRecoveryHint(problem, docMediaAppScopeHint) + } + if docMediaIsRateLimit(problem) && !strings.Contains(problem.Hint, "exponential backoff") { + appendDocRecoveryHint(problem, docMediaRateLimitHint) } return err } -func docMediaDownloadIsRateLimit(problem *errs.Problem) bool { +func docMediaIsRateLimit(problem *errs.Problem) bool { return problem.Subtype == errs.SubtypeRateLimit || problem.Code == 99991400 || problem.Code == http.StatusTooManyRequests diff --git a/shortcuts/doc/doc_media_download.go b/shortcuts/doc/doc_media_download.go index 1e9e753a3f..4da51c10f8 100644 --- a/shortcuts/doc/doc_media_download.go +++ b/shortcuts/doc/doc_media_download.go @@ -94,6 +94,7 @@ var DocMediaDownload = common.Shortcut{ ApiPath: apiPath, }) if err != nil { + err = classifyDocMediaStreamError(runtime, err) return withDocMediaDownloadRecoveryHint(wrapDocNetworkErr(err, "download failed: %v", err), mediaType) } defer resp.Body.Close() diff --git a/shortcuts/doc/doc_media_preview.go b/shortcuts/doc/doc_media_preview.go index 6bc88d8e47..6b15fa1c15 100644 --- a/shortcuts/doc/doc_media_preview.go +++ b/shortcuts/doc/doc_media_preview.go @@ -65,7 +65,8 @@ var DocMediaPreview = common.Shortcut{ }, }) if err != nil { - return wrapDocNetworkErr(err, "preview failed: %v", err) + err = classifyDocMediaStreamError(runtime, err) + return withDocMediaPreviewRecoveryHint(wrapDocNetworkErr(err, "preview failed: %v", err)) } defer resp.Body.Close() diff --git a/shortcuts/doc/doc_media_test.go b/shortcuts/doc/doc_media_test.go index fd83bcbb2f..e64a571617 100644 --- a/shortcuts/doc/doc_media_test.go +++ b/shortcuts/doc/doc_media_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "net/http" "os" @@ -25,6 +26,8 @@ import ( "github.com/larksuite/cli/shortcuts/common" ) +const expectedDocMediaRateLimitHint = "the request was rate limited; stop immediate retries and retry later using exponential backoff with jitter" + func docsTestConfigWithAppID(appID string) *core.CliConfig { return &core.CliConfig{ AppID: appID, AppSecret: "test-secret", Brand: core.BrandFeishu, @@ -737,10 +740,8 @@ func TestDocMediaDownloadHTTP429SuggestsBackoff(t *testing.T) { if !ok || problem.Category != errs.CategoryNetwork || problem.Code != http.StatusTooManyRequests { t.Fatalf("problem=%+v ok=%v, want network HTTP 429", problem, ok) } - for _, want := range []string{"stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if problem.Hint != expectedDocMediaRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDocMediaRateLimitHint) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) @@ -774,16 +775,48 @@ func TestDocMediaDownloadExportAuthRateLimitPreservesAPIErrorAndSuggestsBackoff( if problem.LogID != "log-doc-auth-limited" || !problem.Retryable { t.Fatalf("problem=%+v, want preserved log_id and retryable", problem) } - for _, want := range []string{"stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if problem.Hint != expectedDocMediaRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDocMediaRateLimitHint) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) } } +func TestDocMediaDownloadExportAuthAppScopeAddsStopRetryingHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("docs-auth-scope-app")) + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/drive/v1/permissions/media_auth_scope/members/auth", + Body: map[string]interface{}{ + "code": 99991672, + "msg": "app scope not enabled", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:document.media:download"}, + }, + }, + }, + }) + + withDocsWorkingDir(t, t.TempDir()) + err := mountAndRunDocs(t, DocMediaDownload, []string{ + "+media-download", + "--token", "media_auth_scope", + "--output", "blocked.bin", + "--as", "bot", + }, f, nil) + var permissionErr *errs.PermissionError + if !errors.As(err, &permissionErr) || permissionErr.Code != 99991672 || permissionErr.Subtype != errs.SubtypeAppScopeNotApplied { + t.Fatalf("error=%T %v, want authorization/app_scope_not_applied/99991672", err, err) + } + for _, want := range []string{"developer console", "stop retrying now", "retry only after", "approved and enabled"} { + if !strings.Contains(permissionErr.Hint, want) { + t.Fatalf("hint=%q, want %q", permissionErr.Hint, want) + } + } +} + func TestDocMediaDownloadTypedRateLimitSuggestsBackoff(t *testing.T) { err := errs.NewAPIError(errs.SubtypeRateLimit, "request trigger frequency limit"). WithCode(99991400). @@ -798,16 +831,107 @@ func TestDocMediaDownloadTypedRateLimitSuggestsBackoff(t *testing.T) { if problem.Category != errs.CategoryAPI || problem.Subtype != errs.SubtypeRateLimit || problem.Code != 99991400 || !problem.Retryable { t.Fatalf("problem=%+v, want preserved API rate-limit metadata", problem) } - for _, want := range []string{"upstream hint", "stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if want := "upstream hint\n" + expectedDocMediaRateLimitHint; problem.Hint != want { + t.Fatalf("hint=%q, want %q", problem.Hint, want) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) } } +func TestDocMediaStreamCommandsClassifyLarkRecoveryErrors(t *testing.T) { + commands := []struct { + name string + shortcut common.Shortcut + args []string + endpoint string + exportAuth bool + }{ + { + name: "media download", + shortcut: DocMediaDownload, + args: []string{"+media-download", "--token", "media_stream_error", "--output", "blocked.bin", "--as", "bot"}, + endpoint: "/open-apis/drive/v1/medias/media_stream_error/download", + exportAuth: true, + }, + { + name: "media preview", + shortcut: DocMediaPreview, + args: []string{"+media-preview", "--token", "media_stream_error", "--output", "blocked.bin", "--as", "bot"}, + endpoint: "/open-apis/drive/v1/medias/media_stream_error/preview_download", + }, + } + errorsToClassify := []struct { + name string + code int + }{ + {name: "app scope not applied", code: 99991672}, + {name: "rate limited", code: 99991400}, + } + + for _, command := range commands { + for _, apiFailure := range errorsToClassify { + t.Run(command.name+"/"+apiFailure.name, func(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("docs-stream-error-app")) + if command.exportAuth { + registerDocMediaExportAuth(reg, "media_stream_error", true) + } + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: command.endpoint, + Status: http.StatusBadRequest, + Body: map[string]interface{}{ + "code": apiFailure.code, + "msg": "media stream API error", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "docs:document.media:download"}, + }, + }, + }, + }) + + withDocsWorkingDir(t, t.TempDir()) + err := mountAndRunDocs(t, command.shortcut, command.args, f, nil) + problem, ok := errs.ProblemOf(err) + if !ok || problem.Code != apiFailure.code { + t.Fatalf("problem=%+v ok=%v, want code=%d", problem, ok, apiFailure.code) + } + var streamErr *errs.NetworkError + if !errors.As(err, &streamErr) || streamErr.Code != http.StatusBadRequest { + t.Fatalf("error chain=%T %v, want original HTTP 400 network error preserved as cause", err, err) + } + + switch apiFailure.code { + case 99991672: + var permissionErr *errs.PermissionError + if !errors.As(err, &permissionErr) || permissionErr.Subtype != errs.SubtypeAppScopeNotApplied { + t.Fatalf("error=%T %v, want authorization/app_scope_not_applied", err, err) + } + if len(permissionErr.MissingScopes) != 1 || permissionErr.MissingScopes[0] != "docs:document.media:download" { + t.Fatalf("missing_scopes=%v, want docs:document.media:download", permissionErr.MissingScopes) + } + if permissionErr.ConsoleURL == "" || !strings.Contains(permissionErr.Hint, "developer console") { + t.Fatalf("console_url=%q hint=%q, want developer-console recovery", permissionErr.ConsoleURL, permissionErr.Hint) + } + for _, want := range []string{"stop retrying now", "retry only after", "approved and enabled"} { + if !strings.Contains(permissionErr.Hint, want) { + t.Fatalf("hint=%q, want %q", permissionErr.Hint, want) + } + } + case 99991400: + if problem.Category != errs.CategoryAPI || problem.Subtype != errs.SubtypeRateLimit || !problem.Retryable { + t.Fatalf("problem=%+v, want retryable api/rate_limit", problem) + } + if problem.Hint != expectedDocMediaRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDocMediaRateLimitHint) + } + } + }) + } + } +} + func TestDocMediaDownloadAppendsExtensionFromContentDispositionFilename(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("docs-download-disposition-app")) registerDocMediaExportAuth(reg, "tok_123", true) diff --git a/shortcuts/drive/drive_download.go b/shortcuts/drive/drive_download.go index b76a645abb..1583d3879a 100644 --- a/shortcuts/drive/drive_download.go +++ b/shortcuts/drive/drive_download.go @@ -255,6 +255,7 @@ var DriveDownload = common.Shortcut{ ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)), }) if err != nil { + err = classifyDriveFileReadStreamError(runtime, err) return withDriveDownloadRecoveryHint(wrapDriveNetworkErr(err, "download failed: %s", err), fileToken) } defer resp.Body.Close() diff --git a/shortcuts/drive/drive_errors.go b/shortcuts/drive/drive_errors.go index e1cbfb0481..06ac471b42 100644 --- a/shortcuts/drive/drive_errors.go +++ b/shortcuts/drive/drive_errors.go @@ -9,8 +9,16 @@ import ( "net/http" "strings" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/shortcuts/common" +) + +const ( + driveFileReadAppScopeHint = "stop retrying now; ask the app developer to apply for the required scope(s), and retry only after they have been approved and enabled" + driveFileReadRateLimitHint = "the request was rate limited; stop immediate retries and retry later using exponential backoff with jitter" ) // wrapDriveNetworkErr returns err unchanged when it is already a typed errs.* @@ -23,6 +31,58 @@ func wrapDriveNetworkErr(err error, format string, args ...any) error { return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) } +// classifyDriveFileReadStreamError recovers the two business errors these +// shortcuts promise to guide from DoStream's current +// "HTTP : " transport message. This compatibility shim is +// deliberately local to Drive download/preview; expand it only if DoStream +// exposes a structured response body or another Drive stream adopts the same +// recovery contract. +func classifyDriveFileReadStreamError(runtime *common.RuntimeContext, err error) error { + problem, ok := errs.ProblemOf(err) + if runtime == nil || !ok || problem == nil || + problem.Category != errs.CategoryNetwork || + problem.Subtype != errs.SubtypeNetworkTransport || + problem.Code < http.StatusBadRequest { + return err + } + + prefix := fmt.Sprintf("HTTP %d: ", problem.Code) + if !strings.HasPrefix(problem.Message, prefix) { + return err + } + body := strings.TrimSpace(strings.TrimPrefix(problem.Message, prefix)) + if !strings.HasPrefix(body, "{") { + return err + } + + header := make(http.Header) + header.Set("Content-Type", "application/json") + if problem.LogID != "" { + header.Set(larkcore.HttpHeaderKeyLogId, problem.LogID) + } + _, classified := runtime.ClassifyAPIResponse(&larkcore.ApiResp{ + StatusCode: problem.Code, + Header: header, + RawBody: []byte(body), + }) + classifiedProblem, classifiedOK := errs.ProblemOf(classified) + if !classifiedOK || classifiedProblem == nil || + (classifiedProblem.Code != 99991672 && classifiedProblem.Code != 99991400) { + return err + } + + var permissionErr *errs.PermissionError + if errors.As(classified, &permissionErr) { + permissionErr.WithCause(err) + return classified + } + var apiErr *errs.APIError + if errors.As(classified, &apiErr) { + apiErr.WithCause(err) + } + return classified +} + // withDriveDownloadForbiddenPreviewHint keeps the HTTP 403 network error from // +download intact while giving callers a preview-based path to view content. func withDriveDownloadForbiddenPreviewHint(err error, _ string) error { @@ -46,7 +106,10 @@ func withDriveDownloadForbiddenPreviewHint(err error, _ string) error { // attaching an actionable recovery path for permission and throttling failures. func withDriveDownloadRecoveryHint(err error, fileToken string) error { err = withDriveDownloadForbiddenPreviewHint(err, fileToken) - if !driveDownloadIsRateLimit(err) { + if problem, ok := errs.ProblemOf(err); ok && problem.Code == 99991672 && !strings.Contains(problem.Hint, "stop retrying now") { + err = appendDriveExportRecoveryHint(err, driveFileReadAppScopeHint) + } + if !driveFileReadIsRateLimit(err) { return err } @@ -54,11 +117,25 @@ func withDriveDownloadRecoveryHint(err error, fileToken string) error { if strings.Contains(problem.Hint, "exponential backoff") { return err } - const hint = "Drive download was rate limited; stop immediate retries and retry later with exponential backoff." - return appendDriveExportRecoveryHint(err, hint) + return appendDriveExportRecoveryHint(err, driveFileReadRateLimitHint) +} + +func withDrivePreviewRecoveryHint(err error) error { + if problem, ok := errs.ProblemOf(err); ok && problem.Code == 99991672 && !strings.Contains(problem.Hint, "stop retrying now") { + err = appendDriveExportRecoveryHint(err, driveFileReadAppScopeHint) + } + if !driveFileReadIsRateLimit(err) { + return err + } + + problem, _ := errs.ProblemOf(err) + if strings.Contains(problem.Hint, "exponential backoff") { + return err + } + return appendDriveExportRecoveryHint(err, driveFileReadRateLimitHint) } -func driveDownloadIsRateLimit(err error) bool { +func driveFileReadIsRateLimit(err error) bool { problem, ok := errs.ProblemOf(err) if !ok || problem == nil { return false diff --git a/shortcuts/drive/drive_io_test.go b/shortcuts/drive/drive_io_test.go index b57800eecb..1baada6343 100644 --- a/shortcuts/drive/drive_io_test.go +++ b/shortcuts/drive/drive_io_test.go @@ -32,6 +32,8 @@ import ( "github.com/larksuite/cli/shortcuts/common" ) +const expectedDriveFileReadRateLimitHint = "the request was rate limited; stop immediate retries and retry later using exponential backoff with jitter" + type driveRoundTripFunc func(*http.Request) (*http.Response, error) // RoundTrip delegates an HTTP request to the test transport function. @@ -1827,10 +1829,8 @@ func TestDriveDownloadHTTP429SuggestsBackoff(t *testing.T) { if !ok || problem.Category != errs.CategoryNetwork || problem.Code != http.StatusTooManyRequests { t.Fatalf("problem=%+v ok=%v, want network HTTP 429", problem, ok) } - for _, want := range []string{"stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if problem.Hint != expectedDriveFileReadRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDriveFileReadRateLimitHint) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) @@ -1864,16 +1864,48 @@ func TestDriveDownloadExportAuthRateLimitPreservesAPIErrorAndSuggestsBackoff(t * if problem.LogID != "log-drive-auth-limited" || !problem.Retryable { t.Fatalf("problem=%+v, want preserved log_id and retryable", problem) } - for _, want := range []string{"stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if problem.Hint != expectedDriveFileReadRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDriveFileReadRateLimitHint) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) } } +func TestDriveDownloadExportAuthAppScopeAddsStopRetryingHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/drive/v1/permissions/file_auth_scope/members/auth", + Body: map[string]interface{}{ + "code": 99991672, + "msg": "app scope not enabled", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "drive:file:download"}, + }, + }, + }, + }) + + withDriveWorkingDir(t, t.TempDir()) + err := mountAndRunDrive(t, DriveDownload, []string{ + "+download", + "--file-token", "file_auth_scope", + "--output", "blocked.bin", + "--as", "bot", + }, f, nil) + var permissionErr *errs.PermissionError + if !errors.As(err, &permissionErr) || permissionErr.Code != 99991672 || permissionErr.Subtype != errs.SubtypeAppScopeNotApplied { + t.Fatalf("error=%T %v, want authorization/app_scope_not_applied/99991672", err, err) + } + for _, want := range []string{"developer console", "stop retrying now", "retry only after", "approved and enabled"} { + if !strings.Contains(permissionErr.Hint, want) { + t.Fatalf("hint=%q, want %q", permissionErr.Hint, want) + } + } +} + func TestDriveDownloadTypedRateLimitSuggestsBackoff(t *testing.T) { err := errs.NewAPIError(errs.SubtypeRateLimit, "request trigger frequency limit"). WithCode(99991400). @@ -1888,16 +1920,91 @@ func TestDriveDownloadTypedRateLimitSuggestsBackoff(t *testing.T) { if problem.Category != errs.CategoryAPI || problem.Subtype != errs.SubtypeRateLimit || problem.Code != 99991400 || !problem.Retryable { t.Fatalf("problem=%+v, want preserved API rate-limit metadata", problem) } - for _, want := range []string{"upstream hint", "stop immediate retries", "retry later with exponential backoff"} { - if !strings.Contains(problem.Hint, want) { - t.Fatalf("hint=%q, want %q", problem.Hint, want) - } + if want := "upstream hint\n" + expectedDriveFileReadRateLimitHint; problem.Hint != want { + t.Fatalf("hint=%q, want %q", problem.Hint, want) } if strings.Contains(problem.Hint, "1 minute") { t.Fatalf("hint=%q, want no fixed retry duration", problem.Hint) } } +func TestDriveDownloadFinalStreamClassifiesLarkRecoveryErrors(t *testing.T) { + tests := []struct { + name string + code int + }{ + {name: "app scope not applied", code: 99991672}, + {name: "rate limited", code: 99991400}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + registerDriveDownloadExportAuth(reg, "file_stream_error", true) + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/drive/v1/files/file_stream_error/download", + Status: http.StatusBadRequest, + Body: map[string]interface{}{ + "code": tt.code, + "msg": "stream API error", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "drive:file:download"}, + }, + }, + }, + Headers: http.Header{ + "Content-Type": []string{"application/json"}, + larkcore.HttpHeaderKeyLogId: []string{"log-drive-stream"}, + }, + }) + + withDriveWorkingDir(t, t.TempDir()) + err := mountAndRunDrive(t, DriveDownload, []string{ + "+download", + "--file-token", "file_stream_error", + "--output", "blocked.bin", + "--as", "bot", + }, f, nil) + problem, ok := errs.ProblemOf(err) + if !ok || problem.Code != tt.code || problem.LogID != "log-drive-stream" { + t.Fatalf("problem=%+v ok=%v, want code=%d and stream log id", problem, ok, tt.code) + } + var streamErr *errs.NetworkError + if !errors.As(err, &streamErr) || streamErr.Code != http.StatusBadRequest { + t.Fatalf("error chain=%T %v, want original HTTP 400 network error preserved as cause", err, err) + } + + switch tt.code { + case 99991672: + var permissionErr *errs.PermissionError + if !errors.As(err, &permissionErr) || permissionErr.Subtype != errs.SubtypeAppScopeNotApplied { + t.Fatalf("error=%T %v, want authorization/app_scope_not_applied", err, err) + } + if len(permissionErr.MissingScopes) != 1 || permissionErr.MissingScopes[0] != "drive:file:download" { + t.Fatalf("missing_scopes=%v, want drive:file:download", permissionErr.MissingScopes) + } + if permissionErr.ConsoleURL == "" || !strings.Contains(permissionErr.Hint, "developer console") { + t.Fatalf("console_url=%q hint=%q, want developer-console recovery", permissionErr.ConsoleURL, permissionErr.Hint) + } + for _, want := range []string{"stop retrying now", "retry only after", "approved and enabled"} { + if !strings.Contains(permissionErr.Hint, want) { + t.Fatalf("hint=%q, want %q", permissionErr.Hint, want) + } + } + case 99991400: + if problem.Category != errs.CategoryAPI || problem.Subtype != errs.SubtypeRateLimit || !problem.Retryable { + t.Fatalf("problem=%+v, want retryable api/rate_limit", problem) + } + if problem.Hint != expectedDriveFileReadRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDriveFileReadRateLimitHint) + } + } + }) + } +} + // TestDriveDownloadDefaultOutputPathSanitizesSlashOnlyNames verifies slash-only names fall back safely. func TestDriveDownloadDefaultOutputPathSanitizesSlashOnlyNames(t *testing.T) { header := http.Header{ diff --git a/shortcuts/drive/drive_preview_common.go b/shortcuts/drive/drive_preview_common.go index 92293b619d..f40e50ab13 100644 --- a/shortcuts/drive/drive_preview_common.go +++ b/shortcuts/drive/drive_preview_common.go @@ -273,7 +273,7 @@ func fetchDrivePreviewCandidates(runtime *common.RuntimeContext, fileToken strin body, ) if err != nil { - return nil, nil, err + return nil, nil, withDrivePreviewRecoveryHint(err) } return data, normalizeDrivePreviewCandidates(data), nil } @@ -464,7 +464,8 @@ func downloadDrivePreviewArtifactWithParams(ctx context.Context, runtime *common resp, err := runtime.DoAPIStream(ctx, apiReq) if err != nil { - return nil, wrapDriveNetworkErr(err, "preview download failed: %s", err) + err = classifyDriveFileReadStreamError(runtime, err) + return nil, withDrivePreviewRecoveryHint(wrapDriveNetworkErr(err, "preview download failed: %s", err)) } defer resp.Body.Close() diff --git a/shortcuts/drive/drive_preview_test.go b/shortcuts/drive/drive_preview_test.go index 06028d0811..40235b53bb 100644 --- a/shortcuts/drive/drive_preview_test.go +++ b/shortcuts/drive/drive_preview_test.go @@ -788,20 +788,112 @@ func TestDrivePreviewListOnlyErrorAddsSourceFileHint(t *testing.T) { } } -// TestDrivePreviewListOnlyRateLimitKeepsOriginalHint verifies retryable API -// errors are not reframed as source_file recovery. -func TestDrivePreviewListOnlyRateLimitKeepsOriginalHint(t *testing.T) { - err := withDrivePreviewSourceFileHint(errs.NewAPIError(errs.SubtypeRateLimit, "request trigger frequency limit").WithCode(99991400).WithRetryable()) +// TestDrivePreviewListOnlyRateLimitAddsBackoffHint verifies retryable API +// errors get generic backoff recovery and are not reframed as a source_file +// fallback. +func TestDrivePreviewListOnlyRateLimitAddsBackoffHint(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: http.MethodPost, + URL: "/open-apis/drive/v1/medias/file_rate_limited/preview_result", + Body: map[string]interface{}{ + "code": 99991400, + "msg": "request trigger frequency limit", + }, + }) + + err := mountAndRunDrive(t, DrivePreview, []string{ + "+preview", + "--file-token", "file_rate_limited", + "--list-only", + "--as", "bot", + }, f, nil) problem, ok := errs.ProblemOf(err) if !ok { t.Fatalf("expected typed error, got %T: %v", err, err) } - if problem.Hint != "" { - t.Fatalf("hint=%q, want empty hint for rate limit", problem.Hint) + if problem.Hint != expectedDriveFileReadRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDriveFileReadRateLimitHint) } if !problem.Retryable { t.Fatal("retryable=false, want true") } + if strings.Contains(problem.Hint, "--type source_file") { + t.Fatalf("hint=%q, want no source_file fallback for rate limit", problem.Hint) + } +} + +func TestDrivePreviewSourceFileStreamClassifiesLarkRecoveryErrors(t *testing.T) { + tests := []struct { + name string + code int + }{ + {name: "app scope not applied", code: 99991672}, + {name: "rate limited", code: 99991400}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, driveTestConfig()) + reg.Register(&httpmock.Stub{ + Method: http.MethodGet, + URL: "/open-apis/drive/v1/medias/file_preview_error/preview_download", + Status: http.StatusBadRequest, + Body: map[string]interface{}{ + "code": tt.code, + "msg": "preview stream API error", + "error": map[string]interface{}{ + "permission_violations": []interface{}{ + map[string]interface{}{"subject": "drive:file:download"}, + }, + }, + }, + }) + + withDriveWorkingDir(t, t.TempDir()) + err := mountAndRunDrive(t, DrivePreview, []string{ + "+preview", + "--file-token", "file_preview_error", + "--type", "source_file", + "--output", "blocked.bin", + "--as", "bot", + }, f, nil) + problem, ok := errs.ProblemOf(err) + if !ok || problem.Code != tt.code { + t.Fatalf("problem=%+v ok=%v, want code=%d", problem, ok, tt.code) + } + var streamErr *errs.NetworkError + if !errors.As(err, &streamErr) || streamErr.Code != http.StatusBadRequest { + t.Fatalf("error chain=%T %v, want original HTTP 400 network error preserved as cause", err, err) + } + + switch tt.code { + case 99991672: + var permissionErr *errs.PermissionError + if !errors.As(err, &permissionErr) || permissionErr.Subtype != errs.SubtypeAppScopeNotApplied { + t.Fatalf("error=%T %v, want authorization/app_scope_not_applied", err, err) + } + if len(permissionErr.MissingScopes) != 1 || permissionErr.MissingScopes[0] != "drive:file:download" { + t.Fatalf("missing_scopes=%v, want drive:file:download", permissionErr.MissingScopes) + } + if permissionErr.ConsoleURL == "" || !strings.Contains(permissionErr.Hint, "developer console") { + t.Fatalf("console_url=%q hint=%q, want developer-console recovery", permissionErr.ConsoleURL, permissionErr.Hint) + } + for _, want := range []string{"stop retrying now", "retry only after", "approved and enabled"} { + if !strings.Contains(permissionErr.Hint, want) { + t.Fatalf("hint=%q, want %q", permissionErr.Hint, want) + } + } + case 99991400: + if problem.Category != errs.CategoryAPI || problem.Subtype != errs.SubtypeRateLimit || !problem.Retryable { + t.Fatalf("problem=%+v, want retryable api/rate_limit", problem) + } + if problem.Hint != expectedDriveFileReadRateLimitHint { + t.Fatalf("hint=%q, want generic rate-limit hint %q", problem.Hint, expectedDriveFileReadRateLimitHint) + } + } + }) + } } // TestDrivePreviewSourceFileHintGuards verifies source_file recovery guidance