Fix URI scheme error in LoggerChannelClient.deregisterLogger #294001
+95
−2
Conversation
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
LoggerChannelClient.deregisterLogger accepted only URI but the base interface accepts URI | string. Callers passing string logger IDs (e.g. mcpServer) sent raw strings over IPC, causing URI.revive to treat the string as a scheme and fail validation. Fix: accept URI | string and convert via toResource() before IPC, matching the pattern already used by setVisibility. Also enrich the _validateUri error message with the actual scheme value for future telemetry diagnosis.
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @jriekenMatched files:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a high-volume unhandled error caused by LoggerChannelClient.deregisterLogger sending string logger IDs over IPC (where the main process attempted URI.revive(string) and failed URI validation). Also improves URI validation diagnostics and adds internal guidance for handling telemetry-driven error fixes.
Changes:
- Widen
LoggerChannelClient.deregisterLoggerto acceptURI | stringand serialize viatoResource(...)before IPC. - Enrich
_validateUri’s “illegal scheme” error to include the invalid scheme value (truncated) and its length. - Add internal skill/prompt docs for investigating and fixing telemetry-reported unhandled errors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/platform/log/common/logIpc.ts | Fixes producer-side IPC payload for deregisterLogger by converting string IDs to a log file URI first. |
| src/vs/base/common/uri.ts | Adds more diagnostic detail to URI scheme validation errors to aid future telemetry triage. |
| .github/skills/fix-errors/SKILL.md | Adds internal guidelines for addressing unhandled error telemetry issues. |
| .github/prompts/fix-error.prompt.md | Adds an agent prompt for the “fix unhandled error from telemetry” workflow. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…al characters and total count
Member
rzhao271
approved these changes
Feb 9, 2026
DonJayamanne
approved these changes
Feb 9, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes #293503
Problem
LoggerChannelClient.deregisterLoggernarrowed its parameter type to onlyURI, but the base interfaceILoggerService.deregisterLoggeracceptsURI | string. Callers passing string logger IDs (e.g., MCP server passingthis._loggerId) sent raw strings over IPC. On the main process,URI.revive(stringValue)treated the entire string as a URI scheme, which failed_validateUriwith[UriError]: Scheme contains illegal characters.This was the #1 unhandled error by volume: 1.4M hits, 567K affected users.
Fix
Producer fix (
src/vs/platform/log/common/logIpc.ts): ChangedderegisterLoggerto acceptURI | stringand callthis.toResource(idOrResource)before sending over IPC — matching the pattern already used bysetVisibility.Error enrichment (
src/vs/base/common/uri.ts): Enhanced the_validateUrierror message to include the actual invalid scheme value (truncated to 50 chars) and its length, so future telemetry from any call site reveals what data was passed.