Handle errors when generating connectors#2402
Conversation
📝 WalkthroughWalkthroughThis PR changes ChangesImportOpenAPISpec structured response
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant UIComponent as ImportConnectionForm
participant RpcClient as MiVisualizerRpcClient
participant RpcManager as MiVisualizerRpcManager
UIComponent->>RpcClient: importOpenAPISpec(params)
RpcClient->>RpcManager: sendRequest(importOpenAPISpec)
RpcManager-->>RpcClient: ImportOpenAPISpecResponse
RpcClient-->>UIComponent: result
alt result.isSuccess is false
UIComponent->>UIComponent: setIsFailedImport(true)
UIComponent->>UIComponent: setIsImporting(false)
else success
UIComponent->>UIComponent: waitForEvent / onImportSuccess
end
Related issues: None specified. Related PRs: None specified. Suggested labels: None specified. Suggested reviewers: None specified. 🐰 A hop, a skip, a response defined, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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 (1)
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts (1)
923-943: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWrap the connector copy in error handling.
copy(...)can throw on filesystem/permission/path errors, which makesimportOpenAPISpec()reject instead of returning{ isSuccess: false, errorMessage }. That breaks the method’s error-handling contract in the success path.🤖 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 `@workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts` around lines 923 - 943, The success path in importOpenAPISpec is missing error handling around the connector file copy, so filesystem or permission failures can escape as rejections. Wrap the copy(...) call in a try/catch inside importOpenAPISpec, and on failure return { isSuccess: false, errorMessage } while showing the error via vscode.window.showErrorMessage, keeping the same error-handling contract used for langClient.generateConnector.
🧹 Nitpick comments (2)
workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx (2)
68-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBackend
errorMessageis discarded in the UI.
result.errorMessage(and the error thrown in the catch block) is never surfaced; the UI always renders a generic "Error importing connector. Please try again..." message (Line 130) even though the response now carries a specific reason.🤖 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 `@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx` around lines 68 - 90, The OpenAPI import flow in importWithOpenAPI drops backend-specific failure details, so the UI only shows a generic error. Update the ImportConnectionFromOpenAPI component to capture and store result.errorMessage from importOpenAPISpec and also use the caught error in the catch block, then pass that message into the existing error rendering path instead of always showing the default text. Keep the success path unchanged and make sure the failure state in setIsFailedImport is triggered with the specific reason from the RPC response or thrown exception.
68-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated import-flow logic with
ImportConnectionFromProto.tsx.The
importWithOpenAPI/importWithProtoimplementations (state reset, RPC call, early-return-on-failure, catch handling,waitForEvent) are now nearly identical across both files. Consider extracting a shared hook (e.g.,useImportConnector) to avoid maintaining two copies of this logic.🤖 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 `@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx` around lines 68 - 90, The import flow in importWithOpenAPI is duplicating the same state/reset, RPC call, early-return failure handling, catch logic, and waitForEvent orchestration that already exists in ImportConnectionFromProto.tsx. Extract the shared behavior into a reusable hook such as useImportConnector, and have both importWithOpenAPI and importWithProto delegate to it while keeping only the OpenAPI/Proto-specific inputs (like the RPC method and file path). Preserve the existing success/failure state updates and onImportSuccess callback behavior in the shared implementation.
🤖 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 `@workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts`:
- Around line 923-943: The success path in importOpenAPISpec is missing error
handling around the connector file copy, so filesystem or permission failures
can escape as rejections. Wrap the copy(...) call in a try/catch inside
importOpenAPISpec, and on failure return { isSuccess: false, errorMessage }
while showing the error via vscode.window.showErrorMessage, keeping the same
error-handling contract used for langClient.generateConnector.
---
Nitpick comments:
In
`@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx`:
- Around line 68-90: The OpenAPI import flow in importWithOpenAPI drops
backend-specific failure details, so the UI only shows a generic error. Update
the ImportConnectionFromOpenAPI component to capture and store
result.errorMessage from importOpenAPISpec and also use the caught error in the
catch block, then pass that message into the existing error rendering path
instead of always showing the default text. Keep the success path unchanged and
make sure the failure state in setIsFailedImport is triggered with the specific
reason from the RPC response or thrown exception.
- Around line 68-90: The import flow in importWithOpenAPI is duplicating the
same state/reset, RPC call, early-return failure handling, catch logic, and
waitForEvent orchestration that already exists in ImportConnectionFromProto.tsx.
Extract the shared behavior into a reusable hook such as useImportConnector, and
have both importWithOpenAPI and importWithProto delegate to it while keeping
only the OpenAPI/Proto-specific inputs (like the RPC method and file path).
Preserve the existing success/failure state updates and onImportSuccess callback
behavior in the shared implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce9ef616-6eeb-448f-8265-e4253f366831
📒 Files selected for processing (7)
workspaces/mi/mi-core/src/rpc-types/mi-visualizer/index.tsworkspaces/mi/mi-core/src/rpc-types/mi-visualizer/rpc-type.tsworkspaces/mi/mi-core/src/rpc-types/mi-visualizer/types.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.tsworkspaces/mi/mi-rpc-client/src/rpc-clients/mi-visualizer/rpc-client.tsworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsxworkspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromProto.tsx
$subject
Summary by CodeRabbit
New Features
Bug Fixes