fix(desktop): show error toast when external URL fails to open#583
fix(desktop): show error toast when external URL fails to open#583
Conversation
📝 WalkthroughWalkthroughAdded explicit error handling and user feedback when opening external URLs: Electron shell calls now log failures, the TRPC mutation wraps shell.openExternal in try/catch and throws a TRPCError on failure, and the renderer shows a toast error when the open fails. (46 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Renderer
participant TRPC as "TRPC Procedure"
participant Main as "Electron Main"
participant Shell as "shell.openExternal"
Note over Renderer,TRPC `#D3EAFD`: User action triggers open URL
Renderer->>TRPC: call external.openUrl(input)
TRPC->>Main: invoke shell.openExternal(input)
Main->>Shell: shell.openExternal(url)
alt success
Shell-->>Main: resolved
Main-->>TRPC: success
TRPC-->>Renderer: success response
else failure
Shell-->>Main: rejected (error)
Main-->>Main: log error (.catch)
Main-->>TRPC: throw TRPCError (INTERNAL_SERVER_ERROR)
TRPC-->>Renderer: procedure error
Renderer->>Renderer: show toast.error(error.message)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/lib/electron-app/factories/windows/create.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/electron-app/factories/windows/create.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/electron-app/factories/windows/create.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/electron-app/factories/windows/create.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/lib/electron-app/factories/windows/create.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Boundary layers (tRPC procedures, API routes) own error handling - domain utilities return data or throw specific errors
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/src/lib/electron-app/factories/app/setup.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/lib/electron-app/factories/windows/create.ts (1)
12-14: LGTM! Good defensive error handling.The
.catch()handler properly logs external URL opening failures with context, following the prefixed console logging pattern.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
1-1: LGTM! Toast import for user feedback.The toast import enables user-facing error notifications when URL opening fails.
137-152: User feedback implementation looks good, but dual-path handling adds complexity.The error handling correctly covers both mutation response errors and mutation rejection. However, the dual-path pattern (
.then()checkingresult.success+.catch()for rejection) is necessary only because theopenUrlmutation returns a structured result instead of following the standard tRPC error pattern.The implementation works correctly and provides good user feedback with toast notifications and detailed error logging.
Note: If the
openUrlmutation were refactored to throwTRPCErroron failure (per tRPC best practices), this could be simplified to a single.catch()handler. See comment onapps/desktop/src/lib/trpc/routers/external/index.ts.apps/desktop/src/lib/electron-app/factories/app/setup.ts (1)
61-63: LGTM! Consistent error handling.The
.catch()handler properly logs external URL opening failures, consistent with the pattern used increate.ts.
- Add proper error handling to shell.openExternal() calls - Return success/error from openUrl tRPC mutation - Show toast notification when URL opening fails - Log errors with context for debugging Fixes #561
Kitenite
force-pushed
the
silent-failing
branch
from
b1e0002 to
465f439
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/external/index.ts (1)
32-32: Consider adding error handling forshell.openPathfor consistency.
shell.openPathreturns aPromise<string>where a non-empty string indicates an error message. Currently, this error is silently ignored. For consistency with the newopenUrlerror handling, consider checking the result:- await shell.openPath(filePath); + const error = await shell.openPath(filePath); + if (error) { + console.error("[external/openPathInApp] Failed to open path:", filePath, error); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: error, + }); + }This would provide consistent error feedback across all external operations, though it's outside the immediate scope of this PR.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/lib/electron-app/factories/windows/create.tsapps/desktop/src/lib/trpc/routers/external/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/lib/electron-app/factories/app/setup.ts
- apps/desktop/src/lib/electron-app/factories/windows/create.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/external/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/lib/trpc/routers/external/index.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/lib/trpc/routers/external/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/lib/trpc/routers/external/index.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Use TRPCError with appropriate error codes: NOT_FOUND, UNAUTHORIZED, FORBIDDEN, BAD_REQUEST, INTERNAL_SERVER_ERROR, NOT_IMPLEMENTED
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : tRPC procedures and API route handlers should validate and delegate; keep orchestrators thin
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Applies to apps/api/**/*.{ts,tsx} : Extract business logic from tRPC procedures into utility functions when logic exceeds ~50 lines, is used by multiple procedures, or needs independent testing
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
📚 Learning: 2026-01-02T06:50:28.662Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.662Z
Learning: Boundary layers (tRPC procedures, API routes) own error handling - domain utilities return data or throw specific errors
Applied to files:
apps/desktop/src/lib/trpc/routers/external/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
apps/desktop/src/lib/trpc/routers/external/index.ts (2)
2-2: LGTM!The
TRPCErrorimport is correctly added to support the new error handling pattern. This follows the coding guidelines for using tRPC error handling in boundary layers.
41-53: LGTM! Error handling follows tRPC conventions and coding guidelines.The implementation correctly:
- Wraps
shell.openExternalin try/catch- Uses prefixed console logging with context (
[external/openUrl])- Throws
TRPCErrorwithINTERNAL_SERVER_ERRORcode per coding guidelines- Extracts error message with proper type narrowing
This addresses the PR objective of surfacing errors when opening external URLs fails. Based on learnings, boundary layers own error handling by throwing typed errors.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
Changes
openUrltRPC mutation now returns{ success, error }instead of void.catch()handlers to fire-and-forgetshell.openExternal()callsFixes #561
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.