-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: improve Qdrant connection error handling for external drives #6718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add detailed error logging for external drive paths detection - Improve error messages for Qdrant connection failures - Add pre-connection check to validate Qdrant accessibility - Handle Internal Server Error (500) responses with specific messages - Add comprehensive test coverage for new error scenarios Fixes #6717
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
| this.qdrantUrl = parsedUrl | ||
|
|
||
| // Log workspace path for debugging external drive issues | ||
| console.log(`[QdrantVectorStore] Initializing with workspace path: "${workspacePath}"`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this level of console.log usage intentional? While helpful for debugging, it might be too verbose for production. Consider using a debug flag or a logger with configurable levels to avoid cluttering the console.
| this.qdrantUrl = parsedUrl | ||
|
|
||
| // Log workspace path for debugging external drive issues | ||
| console.log(`[QdrantVectorStore] Initializing with workspace path: "${workspacePath}"`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workspace path being logged here could expose sensitive directory structures. Should we consider sanitizing or limiting what path information is logged for security reasons?
| console.log(`[QdrantVectorStore] Initializing with workspace path: "${workspacePath}"`) | ||
|
|
||
| // Check if workspace path is on an external drive (common patterns) | ||
| const isExternalDrive = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This external drive detection logic might miss some edge cases. For example:
- Network drives on Windows (\server\share)
- Symlinked directories that point to external drives
- Case sensitivity on different file systems
Is this intentional, or should we expand the detection logic?
| this.vectorSize = vectorSize | ||
| this.collectionName = `ws-${hash.substring(0, 16)}` | ||
|
|
||
| console.log(`[QdrantVectorStore] Generated collection name: "${this.collectionName}" from workspace path`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting this log prefix as a constant to ensure consistency and make future changes easier. For example: const LOG_PREFIX = '[QdrantVectorStore]';
| }) | ||
| }) | ||
|
|
||
| describe("External Drive Detection and Logging", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test coverage! Consider adding test cases for the edge cases mentioned earlier:
- Network drive paths (\server\share)
- Symlinked directories
- Mixed case drive letters on Windows
This would ensure the external drive detection is robust across all scenarios.
|
This is too specific, closing for now |
Fixes #6717
This PR improves error handling when the Roo Code extension fails to connect to Qdrant for projects on external drives.
Changes
Testing
Important
Improves Qdrant connection error handling in
QdrantVectorStore, with detailed logging and pre-connection validation, especially for external drives.QdrantVectorStorefor Qdrant connection issues, especially for external drives.initialize().initialize().qdrant-client.ts.qdrant-client.spec.tsfor enhanced error handling scenarios.This description was created by
for 2ec84d9. You can customize this summary. It will automatically update as commits are pushed.