Skip to content

Commit a9ef122

Browse files
committed
fix: address Copilot review feedback for URI handler diagnostics
- Remove showWarningMessage to avoid double notification (wrapper already shows errors) - Set granular failureStage markers at extractParams/handleRequest boundaries - Remove redundant uriHasQuery and queryFragmentNonEmpty (uriQueryLength covers it)
1 parent 6018a32 commit a9ef122

1 file changed

Lines changed: 7 additions & 13 deletions

File tree

src/vscodeUriHandler.ts

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,21 @@ export async function globalUriHandler(uri: vscode.Uri): Promise<void> {
6868
context.telemetry.properties.uriAuthority = uri.authority;
6969
context.telemetry.properties.uriPathLength = String(uri.path.length);
7070
context.telemetry.properties.uriQueryLength = String((uri.query ?? '').length);
71-
context.telemetry.properties.uriHasQuery = uri.query ? 'true' : 'false';
7271

7372
try {
7473
// Extract and validate parameters
7574
// Note: uri.query is already decoded once by VS Code when creating the vscode.Uri object
75+
context.telemetry.properties.failureStage = 'extractParams';
7676
const params = extractAndValidateParams(context, uri.query);
7777

7878
// Process the URI with user confirmation
79+
context.telemetry.properties.failureStage = 'handleRequest';
7980
await handleConnectionStringRequest(context, params);
8081
} catch (error) {
8182
const errMsg = error instanceof Error ? error.message : String(error);
82-
context.telemetry.properties.failureStage = 'unwrap';
83+
if (!context.telemetry.properties.failureStage) {
84+
context.telemetry.properties.failureStage = 'unknown';
85+
}
8386
throw new Error(l10n.t('Failed to process URI: {0}', errMsg));
8487
}
8588
});
@@ -402,18 +405,9 @@ function extractAndValidateParams(context: IActionContext, queryFragment: string
402405
// Add sensitive values to valuesToMask to prevent sensitive data in logs
403406
maskParamsInTelemetry(context, params);
404407

405-
// Record whether the query was non-empty for diagnostic purposes.
406-
context.telemetry.properties.queryFragmentNonEmpty = queryFragment ? 'true' : 'false';
407-
408408
if (!params.connectionString) {
409-
// Surface a user-visible message for the most common failure case
410-
// instead of an opaque telemetry-only error.
411-
void vscode.window.showWarningMessage(
412-
l10n.t(
413-
'A DocumentDB deep-link was opened without a connection string. Ensure the link includes a connectionString query parameter.',
414-
),
415-
);
416-
throw new Error(l10n.t('The connection string is required.'));
409+
// Throw a descriptive error — the telemetry wrapper will surface it to the user.
410+
throw new Error(l10n.t('A DocumentDB deep-link was opened without a connection string. Ensure the link includes a connectionString query parameter.'));
417411
}
418412

419413
context.telemetry.properties.hasParamConnectionString = params.connectionString ? 'true' : undefined;

0 commit comments

Comments
 (0)