fix(drive): harden export and push failure recovery - #2279
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDrive error metadata now covers multipart, quota, and export failures. Export handling distinguishes rate-limited ChangesDrive error handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DriveAPI
participant createDriveExportTask
participant withDriveExportCreateRecovery
participant WorkbookExport
DriveAPI->>createDriveExportTask: return code 9499 with too many requests
createDriveExportTask->>withDriveExportCreateRecovery: normalize and classify response
withDriveExportCreateRecovery-->>createDriveExportTask: return retryable rate-limit error
createDriveExportTask-->>WorkbookExport: preserve typed error and caller recovery hint
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
48c787a to
a7083ce
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@29087fddabd8d1eb098b28ec132e7701d8d49c4c🧩 Skill updatenpx skills add larksuite/cli#fix/drive-export-error-recovery -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2279 +/- ##
=======================================
Coverage 76.44% 76.45%
=======================================
Files 1013 1013
Lines 111972 112034 +62
=======================================
+ Hits 85601 85658 +57
- Misses 19860 19864 +4
- Partials 6511 6512 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
a7083ce to
91b9925
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_push.go`:
- Around line 611-613: Update the retry-delay conversion in the errs.RetryAfter
handling so fractional durations are rounded up to the next whole second before
assigning decision.RetryAfterSeconds, including saturated durations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 155f1d4f-12c0-457c-bba9-c01d5a1edc69
📒 Files selected for processing (5)
internal/errclass/codemeta_drive.gointernal/errclass/codemeta_test.goshortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goskills/lark-drive/references/lark-drive-push.md
54c001d to
2e6d725
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/drive/drive_pull_test.go`:
- Around line 1139-1152: Update the failed-item assertions in
shortcuts/drive/drive_pull_test.go:1139-1152 and
shortcuts/drive/drive_sync_test.go:427-441. In the pull test, assert the first
item reports action "failed", download phase, HTTP 404, and retryable false; in
the sync test, assert the first driveSyncItem exposes the same failed-download
metadata. Keep the existing continuation and successful b.txt assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 348cd70d-7e87-4238-b6e3-a7aab49e206c
📒 Files selected for processing (4)
shortcuts/drive/drive_pull_test.goshortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_sync_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/drive/drive_push.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/sheets/lark_sheet_workbook_export_test.go`:
- Around line 121-126: Extend the error assertions around errs.ProblemOf(err) to
require problem.Category == errs.CategoryAPI. Use errors.As to extract the
wrapped *errs.APIError, then verify its preserved cause with errors.Is or
Unwrap() against the expected underlying error while retaining the existing
subtype, code, and retryable checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 119acede-3980-4b58-a5ad-c633925761c8
📒 Files selected for processing (9)
internal/errclass/codemeta_drive.gointernal/errclass/codemeta_test.goshortcuts/drive/drive_errors.goshortcuts/drive/drive_export_test.goshortcuts/drive/drive_pull_test.goshortcuts/drive/drive_push_test.goshortcuts/drive/drive_sync_test.goshortcuts/sheets/lark_sheet_workbook_export_test.goskills/lark-drive/references/lark-drive-push.md
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/drive/drive_pull_test.go
- shortcuts/drive/drive_sync_test.go
- internal/errclass/codemeta_drive.go
- internal/errclass/codemeta_test.go
- skills/lark-drive/references/lark-drive-push.md
- shortcuts/drive/drive_export_test.go
- shortcuts/drive/drive_push_test.go
- shortcuts/drive/drive_errors.go
Summary
Harden Drive export and push failure recovery for the error patterns observed in production.
Changes
Export recovery
9499only at the Drive export task boundary using its exact throttling message, leaving the shared classifier unchanged and preservinginvalid_parameterseverywhere else.1069902,1069906,1069914, and1069918.9499rate-limit variant and return ticket-aware resume guidance instead of amplifying throttling.Push failure handling
1062009) file-local so independent files continue, while stopping the batch on shared failures such as resource contention, quota exhaustion, multipart internal errors, and HTTP/network failures.1663as a retryable server error.Tests and docs
1663.Test Plan
go test -race -count=1 ./internal/errclass ./shortcuts/drivemake vetmake fmt-checknode scripts/skill-format-check/index.jsgo test -C lint ./... -count=1go run -C lint . --changed-from origin/main ..make quality-gatemake unit-testpassed all changed and Drive-related packages under-race. The aggregate run encountered the repository's unrelated macOSTempDir RemoveAllflake ininternal/qualitygate/rules(.git: directory not empty); the failing package passed when rerun in isolation. Live rate-limit and destructive push E2E cases were not forced because they would be unsafe or nondeterministic; deterministic command-level HTTP stubs cover the changed behavior.Related Issues
Summary by CodeRabbit
Bug Fixes
Documentation