Merged
Conversation
…tring regex Address Copilot review comment about the regex potentially leaking password fragments when passwords contain unescaped '@'. The regex is correct for spec-compliant URIs — per RFC 3986, '@' in credentials MUST be percent-encoded as '%40'. Added an explicit comment documenting this assumption.
Address Copilot review comment about the setTimeout timer in handleGetAISuggestions never being cleared. The returned cleanup function was ignored since this is an event handler, not a React effect. Fix: Store the timer ID in a useRef (stage3TipsTimerRef), clear it at the start of handleGetAISuggestions (re-trigger) and in handleCancelAI (cancel), and remove the dead return statement. This prevents stale tips/error cards from flashing when the user cancels within 1 second or rapidly re-triggers the AI request.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR contains pre-release updates for version 0.7.1, focusing on improving timer management in the Query Insights feature and enhancing documentation for credential redaction.
Changes:
- Improved timer management in QueryInsightsTab by storing the delayed tips/error card timer in a ref, enabling proper cleanup when users cancel AI requests or restart queries
- Enhanced documentation for
redactCredentialsFromConnectionStringregex to clarify assumptions about RFC 3986 compliant URIs and percent-encoding of the '@' character
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/webviews/documentdb/collectionView/components/queryInsightsTab/QueryInsightsTab.tsx | Refactored timer management by storing timer in a ref (stage3TipsTimerRef) instead of local variable, enabling cleanup in handleCancelAI and when restarting AI suggestions |
| src/documentdb/utils/connectionStringHelpers.ts | Added detailed comments explaining the regex pattern used for credential redaction and RFC 3986 assumptions about percent-encoding |
src/webviews/documentdb/collectionView/components/queryInsightsTab/QueryInsightsTab.tsx
Show resolved
Hide resolved
tnaum-ms
deleted the
dev/tnaum/revisit-0.7.1-release-pre-release-fix-branch
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the two Copilot review comments from PR #517.
Changes
1.
redactCredentialsFromConnectionStringregex — accepted, no code change (e9e2182)The regex is correct for spec-compliant URIs. Per RFC 3986,
@in credentials must be percent-encoded as%40, so the first literal@after the scheme is always the credential delimiter. Added a clarifying comment documenting this assumption.2. Stage 3 tips/error timer never cleared — fixed (
960118e)The
setTimeoutinhandleGetAISuggestionsreturned a cleanup function that was never consumed (event handler return values are ignored). This could cause stale tips/error cards to flash if the user cancelled within 1 second or rapidly re-triggered the AI request.Fix: Store the timer ID in a
useRef(stage3TipsTimerRef), clear it at the start ofhandleGetAISuggestions(re-trigger) and inhandleCancelAI(cancel), and remove the deadreturnstatement.