feat(markdown): harden create upload failures#1325
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughMarkdown upload helpers now use typed API calls with retry wrappers and backoff. Single- and multipart flows share a callback-based reader lifecycle; multipart parts rebuild form-data per retry. Validation now returns internal invalid-response errors for missing fields. Tests assert typed-problem mapping and retry behavior. ChangesMarkdown Upload Retry & Typed API Flow
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Runtime as runtime.CallAPITyped / retry helpers
participant API as Remote API
CLI->>Runtime: upload_prepare (withMarkdownUploadRetryData)
Runtime->>API: upload_prepare request
API-->>Runtime: response (classified)
CLI->>Runtime: upload_part (withMarkdownUploadRetryVoid) [per chunk]
Runtime->>API: upload_part request (form-data rebuilt each retry)
API-->>Runtime: part response (classified)
CLI->>Runtime: upload_finish (withMarkdownUploadRetryData)
Runtime->>API: upload_finish request
API-->>Runtime: finish response (file_token)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
the
size/M
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1325 +/- ##
==========================================
+ Coverage 71.42% 71.47% +0.04%
==========================================
Files 688 688
Lines 65313 65477 +164
==========================================
+ Hits 46653 46801 +148
- Misses 15021 15031 +10
- Partials 3639 3645 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ca8f40470bb2e46c493a98e741ee2cc40b1f1579🧩 Skill updatenpx skills add larksuite/cli#feat/markdown-create-upload-hardening -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/markdown/helpers.go (1)
530-546:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep impossible
upload_preparechunk plans on the typed invalid-response path.The earlier prepare parser now returns
errs.NewInternalError(...), but these later validations still fall back to legacyoutput.Errorf(...). Impossibleblock_num/block_sizevalues are the same invalid-response class, so this reintroduces the old untyped path for a subset of malformed prepare responses.Suggested change
if session.BlockNum != expectedBlocks { - return output.Errorf( - output.ExitAPI, - "api_error", + return errs.NewInternalError( + errs.SubtypeInvalidResponse, "upload_prepare returned inconsistent chunk plan: block_size=%d, block_num=%d, expected_block_num=%d, payload_size=%d", session.BlockSize, session.BlockNum, expectedBlocks, payloadSize, ) } @@ if session.BlockSize > maxInt { - return output.Errorf(output.ExitAPI, "api_error", "upload prepare failed: invalid block_size returned") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "upload_prepare returned invalid block_size: %d", session.BlockSize) }Based on the PR objective to move malformed upload responses onto typed internal-error handling.
🤖 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 `@shortcuts/markdown/helpers.go` around lines 530 - 546, The validations after computing expectedBlocks currently return legacy untyped errors via output.Errorf (in the checks comparing session.BlockNum to expectedBlocks and session.BlockSize to maxInt); change both to return the typed internal-error path used earlier (errs.NewInternalError or the package's internal error constructor) with the same diagnostic message text so malformed upload_prepare responses are classified as typed internal errors rather than the old output.Errorf path. Target the checks referencing session.BlockSize, session.BlockNum, expectedBlocks, payloadSize and maxInt and replace the output.Errorf returns with errs.NewInternalError(...) (or the project's internal-error helper) preserving the message content.
🤖 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/markdown/helpers.go`:
- Around line 450-455: The parseMarkdownUploadResult and
parseMarkdownMultipartSession call sites must wrap any returned parse errors
with the step-specific markdownUploadProblem so the action-prefixed message is
preserved; change the return paths that currently return
parseMarkdownUploadResult(...) or parseMarkdownMultipartSession(...) directly to
detect a non-nil error result and return markdownUploadResult{} (or the
appropriate zero) together with markdownUploadProblem(err, <action>), using
markdownUploadAllAction for upload_all call sites, markdownUploadPrepareAction
for upload_prepare, and markdownUploadFinishAction for upload_finish so
malformed-success parse errors include the correct action context.
- Around line 713-725: In markdownUploadProblem, add a switch branch for the
scope-denied upload error(s) so scope failures get an upload-specific hint:
locate the switch on p.Code inside markdownUploadProblem and add a case for the
scope-denied error code(s) used by your system (the same code(s) used elsewhere
to indicate missing scopes), and call appendMarkdownProblemHint(err, "The
current token or identity lacks the required document upload scope/capability —
grant the document upload scope or use a token with the appropriate permissions
and retry.") so scope-denied errors receive the same contextual guidance as the
other cases; keep the existing p.Message prefixing behavior and use
appendMarkdownProblemHint as in the other cases.
---
Outside diff comments:
In `@shortcuts/markdown/helpers.go`:
- Around line 530-546: The validations after computing expectedBlocks currently
return legacy untyped errors via output.Errorf (in the checks comparing
session.BlockNum to expectedBlocks and session.BlockSize to maxInt); change both
to return the typed internal-error path used earlier (errs.NewInternalError or
the package's internal error constructor) with the same diagnostic message text
so malformed upload_prepare responses are classified as typed internal errors
rather than the old output.Errorf path. Target the checks referencing
session.BlockSize, session.BlockNum, expectedBlocks, payloadSize and maxInt and
replace the output.Errorf returns with errs.NewInternalError(...) (or the
project's internal-error helper) preserving the message content.
🪄 Autofix (Beta)
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
Run ID: 9f9cfd84-d658-4b64-80ff-16f352a14027
📒 Files selected for processing (2)
shortcuts/markdown/helpers.goshortcuts/markdown/markdown_test.go
fangshuyu-768
force-pushed
the
feat/markdown-create-upload-hardening
branch
from
937a8c1 to
bbfcab6
Compare
github-actions
Bot
added
size/L
wittam-01
approved these changes
Jun 8, 2026
SunPeiYang996
pushed a commit
that referenced
this pull request
Jun 8, 2026
* feat(markdown): harden create upload failures * test(markdown): address AI review follow-ups
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit