Report failed item insertions instead of showing false success - #121
Conversation
cm insert and the wizard now count loaded vs failed items after insertion and warn when some items failed, with instructions to retry. Previously showed "Asset cache updated successfully" even when transactions failed (e.g. from RPC rate limiting).
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe changes add detailed insert outcome reporting to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/cm/create.ts`:
- Around line 229-237: Extract the duplicated failed-insert counting and
messaging into a shared helper (e.g., functions named
summarizeAssetCache(assetCache) -> { totalItems, loadedItems, failedItems }
and/or reportFailedInsertions(assetCache, candyMachineDir, logger)) and replace
the block that inspects insertItemsRes.assetCache.assetItems in create.ts with
calls to that helper; update the insert.ts usage to call the same helper so both
commands stay in sync, and standardize the retry message wording to "... again
to retry failed items." across both callers.
In `@src/commands/cm/insert.ts`:
- Around line 58-70: When failedItems > 0 in the insert command, the code
currently logs a warning then returns a success-like result; update the failure
path in the method containing the shown block (where this.log, this.logSuccess
and the returned object are used) to signal a non-zero exit: after logging the
warning call this.error(...) with a clear message (or set process.exitCode = 1)
and then return or throw so callers/CI receive a failing exit code instead of 0;
ensure the returned object or thrown error still includes candyMachineId,
totalItems, itemsInserted and itemsFailed if needed for diagnostics.
In `@test/commands/cm/cm.insert.test.ts`:
- Line 55: The assertion hardcodes the default asset count string "All 50 items
inserted successfully" from create-test-cm, which is brittle; either make the
test supply the explicit flag by adding --assets=50 to the earlier
create-test-cm invocation (so the count is deterministic), or relax the
expectation by changing the assertion on stdout for
expect(stdout).to.include(...) to a substring like 'items inserted successfully'
to avoid depending on the numeric default.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ca424d0-6ef7-401f-b194-03697d2f1cca
📒 Files selected for processing (4)
src/commands/cm/create.tssrc/commands/cm/insert.tstest/commands/cm/cm.full.test.tstest/commands/cm/cm.insert.test.ts
- Extract summarizeAssetCache helper in cm-utils and use it from both
cm create and cm insert so the counting logic stays in sync
- Align retry wording across both callers ("again to retry failed items")
- Signal partial failure with process.exitCode = 1 so CI/scripts can
detect failed inserts
- Pass --assets=50 explicitly in the insert test so the assertion does
not depend on create-test-cm defaults
- Include stdout in runCli failure error so tests that inspect the
error message after a non-zero exit can still see warning output
2 participants
cm insert and the wizard now count loaded vs failed items after insertion and warn when some items failed, with instructions to retry. Previously showed "Asset cache updated successfully" even when transactions failed (e.g. from RPC rate limiting).