Repository navigation
fix(ui): stop provider delete from surfacing a false error toast - #844
Conversation
Deleting a provider races the providers store update (via the providers-changed IPC event) against ProviderSheet's own post-delete code, so provider.name could read as undefined right after the delete succeeded, throwing and getting reported as a failure. Snapshot the name before the delete call and guard the reactive prop reads that can transiently see an undefined provider.
✅ Deploy Preview for devsydev canceled.
|
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesProvider sheet safety
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
✅ Deploy Preview for images-devsy-sh canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte`:
- Around line 96-98: Update the ProviderSheet provider prop type from Provider
to Provider | undefined, then in runUpdate, runSwitch, handleSetDefault, and
handleInitialize capture provider.name in a local variable before each
asynchronous flow and use that snapshot after every await. Guard the initial
access when provider is absent so in-flight handlers cannot dereference the
mutable provider value or trigger incorrect cleanup and error handling.
🪄 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: CHILL
Plan: Pro Plus
Run ID: d6fb48c3-4224-43d2-9f18-5ed4e427503b
📒 Files selected for processing (1)
desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte
| let versionEntry = $derived( | ||
| provider ? $providerVersions.byProvider[provider.name] : undefined, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Complete null safety for in-flight handlers.
The component still declares provider: Provider at Line 50, although this PR allows provider to become undefined. The conditional markup does not cancel handlers that started earlier. If the provider disappears during an await, runUpdate, runSwitch, handleSetDefault, and handleInitialize can still dereference provider.name, causing false error toasts or failed cleanup.
Change the prop type to Provider | undefined. Snapshot the provider name before each asynchronous flow and use the local name after every await.
🤖 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 `@desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte` around
lines 96 - 98, Update the ProviderSheet provider prop type from Provider to
Provider | undefined, then in runUpdate, runSwitch, handleSetDefault, and
handleInitialize capture provider.name in a local variable before each
asynchronous flow and use that snapshot after every await. Guard the initial
access when provider is absent so in-flight handlers cannot dereference the
mutable provider value or trigger incorrect cleanup and error handling.
handleSetDefault, runUpdate, runSwitch, and handleInitialize read provider.name after an await, same race as the delete handler: if the provider is deleted while one of these is in flight, the reactive prop can go undefined before the post-await read, throwing and surfacing as a misleading error toast.
# Conflicts: # desktop/src/renderer/src/lib/components/provider/ProviderSheet.svelte
Summary
providersstore update (from theproviders-changedIPC event) againstProviderSheet's post-delete code, so the liveproviderprop could read asundefinedright after deletion succeeded, throwing and getting reported asFailed to delete: Cannot read properties of undefined (reading 'name')even though the delete had worked.providerDeleteand use that local copy afterward instead of re-reading the reactive prop.provider(versionEntry, the options-loading effect, and the template) so a transientundefinedprovider renders nothing instead of throwing.Summary by CodeRabbit