fix(drive): harden push failure handling - #2277
Conversation
github-actions
Bot
added
domain/ccm
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughDrive push failure handling now preserves structured retry and diagnostic metadata. Classification distinguishes upload, server, conflict, quota, network, parameter, and local-file errors. Tests and documentation cover updated batch behavior and retry guidance. ChangesDrive push failure handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant DrivePush
participant DriveAPI
participant FailureClassifier
participant BatchResult
DrivePush->>DriveAPI: upload file or create folder
DriveAPI-->>DrivePush: success or typed error
DrivePush->>FailureClassifier: classify failure and extract hints
FailureClassifier-->>DrivePush: category, retryability, and backoff guidance
DrivePush->>BatchResult: record partial or aborted push result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
PR Quality SummaryCI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun. Failed checks
deterministic-gate
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@64995a30f3a68c955b669efab2b2241bad8a9bbd🧩 Skill updatenpx skills add larksuite/cli#fix/drive-push-error-guidance -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2277 +/- ##
==========================================
+ Coverage 76.44% 76.46% +0.01%
==========================================
Files 1013 1013
Lines 111957 111995 +38
==========================================
+ Hits 85588 85632 +44
+ Misses 19859 19856 -3
+ Partials 6510 6507 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/lark-drive/references/lark-drive-push.md (2)
150-151: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDocument the network failure class.
The PR objectives include network failures, but this table documents rate limits, conflicts, and server errors only. A transport failure without an HTTP status does not match
HTTP 5xx, so agents have no documented batch or retry rule. Add the emitted network error class and specify its finite retry policy.Based on the PR objectives, network failures are part of the classification contract.
🤖 Prompt for 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. In `@skills/lark-drive/references/lark-drive-push.md` around lines 150 - 151, Extend the error-classification table in lark-drive-push.md with the emitted network/transport failure class for errors without an HTTP status, and document that it stops the current batch and uses bounded backoff retries. Keep the existing rate-limit, conflict, and server-error entries unchanged.
123-123: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winExpose
retry_after_secondsinitems[]. The client stores the server delay on typed errors, butdrivePushItemdrops it and emits onlyhint. Add an integerretry_after_secondsfield, omit it when unavailable, and document that agents must wait at least this duration before bounded backoff and jitter.🤖 Prompt for 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. In `@skills/lark-drive/references/lark-drive-push.md` at line 123, Update drivePushItem’s failed-item output to propagate the typed error’s server delay as an integer retry_after_seconds field, omitting the field when unavailable while preserving the existing hint and error metadata. Document in the items[] schema that agents must wait at least this duration before applying bounded backoff and jitter.
🤖 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.
Outside diff comments:
In `@skills/lark-drive/references/lark-drive-push.md`:
- Around line 150-151: Extend the error-classification table in
lark-drive-push.md with the emitted network/transport failure class for errors
without an HTTP status, and document that it stops the current batch and uses
bounded backoff retries. Keep the existing rate-limit, conflict, and
server-error entries unchanged.
- Line 123: Update drivePushItem’s failed-item output to propagate the typed
error’s server delay as an integer retry_after_seconds field, omitting the field
when unavailable while preserving the existing hint and error metadata. Document
in the items[] schema that agents must wait at least this duration before
applying bounded backoff and jitter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3220aa9e-bd75-4d97-9331-519528ee6c16
📒 Files selected for processing (3)
shortcuts/drive/drive_push.goshortcuts/drive/drive_push_test.goskills/lark-drive/references/lark-drive-push.md
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_push_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/drive/drive_push.go
|
Superseded by #2279, which contains the complete final Drive push failure-handling diff from this PR (head 64995a3) together with the related Drive export recovery work. The four commits were cherry-picked patch-for-patch; no additional review-suggestion changes such as retry_after_seconds propagation were carried over. Further review and CI follow-up will continue in #2279. |
1 participant
Summary
Harden
drive +pushbatch control so deterministic or shared failures do not fan out across the remaining batch, using only error signals available on the command's real response path.Changes
1062009as the file-localupload_size_mismatchbefore the generic invalid-parameters branch, allowing independent files to continue1663Test Plan
go test ./shortcuts/drive ./internal/errclass -count=1make unit-testmake vetmake fmt-checknode scripts/skill-format-check/index.jsmake quality-gateRelated Issues
Summary by CodeRabbit
Bug Fixes
Documentation