Route jsonrpc connection logging to the output channel instead of console by pujitha24 · Pull Request #1853 · microsoft/vscode-languageserver-node

Pujitha Paladugu (pujitha24) · 2026-09-08T10:49:00Z

Motivation:
BaseLanguageClient's internal jsonrpc Connection was created with a
ConsoleLogger, so protocol-level diagnostics such as "Received response
message without id" (jsonrpc/src/common/connection.ts) only showed up in
VS Code's Developer Tools console. Extensions typically point users at
their own Output channel for troubleshooting, so these messages were
effectively invisible to end users and to extension authors reproducing
bug reports.

Approach:
Add an OutputChannelLogger that implements the jsonrpc Logger interface
(error/warn/info/log) by forwarding to a LogOutputChannel, mapping the
generic log() to outputChannel.info() since LogOutputChannel has no
generic log method. The module-level createConnection() now accepts an
optional lazy accessor (() => LogOutputChannel) and uses
OutputChannelLogger when provided, falling back to the previous
ConsoleLogger otherwise. BaseLanguageClient.createConnection() passes
"() => this.outputChannel" rather than a resolved channel so the output
channel keeps its existing lazy-creation semantics: a channel is still
only created the first time something is actually logged, matching the
behavior of the pre-existing this.error/warn/info/debug client methods
and shouldLogToOutputChannel().

Validation:

npm run compile:client (tsc -b ./client/tsconfig.json) passes with no
errors, type-checked against the real @types/vscode LogOutputChannel
shape.
cd client && npm run lint (eslint) passes clean.
Traced call sites in jsonrpc/src/common/connection.ts to confirm
logger.error/warn/log are only invoked from message-processing paths,
never from connection setup, so the lazy output-channel accessor is
never forced eagerly and a session that never logs still creates no
output channel, preserving prior behavior for that case.
Could not run this repo's client-node-tests integration suite (real
VS Code Insiders launched via @vscode/test-electron, as used in
build/azure-pipelines/darwin/build.yml): the sandbox this change was
developed in has no free disk space to download the ~300MB VS Code
build (ENOSPC), an environment constraint unrelated to this change.
There is no lighter-weight unit-test harness for client.ts in this
repo since everything under client-node-tests only runs inside a real
VS Code Electron process.

Report: #1613
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)

Fixes #1613

…sole Motivation: BaseLanguageClient's internal jsonrpc Connection was created with a ConsoleLogger, so protocol-level diagnostics such as "Received response message without id" (jsonrpc/src/common/connection.ts) only showed up in VS Code's Developer Tools console. Extensions typically point users at their own Output channel for troubleshooting, so these messages were effectively invisible to end users and to extension authors reproducing bug reports. Approach: Add an OutputChannelLogger that implements the jsonrpc Logger interface (error/warn/info/log) by forwarding to a LogOutputChannel, mapping the generic log() to outputChannel.info() since LogOutputChannel has no generic log method. The module-level createConnection() now accepts an optional lazy accessor (() => LogOutputChannel) and uses OutputChannelLogger when provided, falling back to the previous ConsoleLogger otherwise. BaseLanguageClient.createConnection() passes "() => this.outputChannel" rather than a resolved channel so the output channel keeps its existing lazy-creation semantics: a channel is still only created the first time something is actually logged, matching the behavior of the pre-existing this.error/warn/info/debug client methods and shouldLogToOutputChannel(). Validation: - npm run compile:client (tsc -b ./client/tsconfig.json) passes with no errors, type-checked against the real @types/vscode LogOutputChannel shape. - cd client && npm run lint (eslint) passes clean. - Traced call sites in jsonrpc/src/common/connection.ts to confirm logger.error/warn/log are only invoked from message-processing paths, never from connection setup, so the lazy output-channel accessor is never forced eagerly and a session that never logs still creates no output channel, preserving prior behavior for that case. - Could not run this repo's client-node-tests integration suite (real VS Code Insiders launched via @vscode/test-electron, as used in build/azure-pipelines/darwin/build.yml): the sandbox this change was developed in has no free disk space to download the ~300MB VS Code build (ENOSPC), an environment constraint unrelated to this change. There is no lighter-weight unit-test harness for client.ts in this repo since everything under client-node-tests only runs inside a real VS Code Electron process. Report: microsoft#1613 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code)

azure-pipelines · 2026-09-08T10:49:09Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Dirk Bäumer (dbaeumer) · 2026-09-18T14:21:43Z


-function createConnection(input: MessageReader, output: MessageWriter, errorHandler: ConnectionErrorHandler, closeHandler: ConnectionCloseHandler, options?: ConnectionOptions): Connection {
-	const logger = new ConsoleLogger();
+function createConnection(input: MessageReader, output: MessageWriter, errorHandler: ConnectionErrorHandler, closeHandler: ConnectionCloseHandler, getOutputChannel: (() => LogOutputChannel) | undefined, options?: ConnectionOptions): Connection {


Can we make a type / interface ChannelProvider like

interface ChannelProvider {
outputChannel: LogOutputChanneln
}

And then pass the client instead of a function instance.

…nnel closure Per review feedback, createConnection/OutputChannelLogger now take a ChannelProvider ({ outputChannel: LogOutputChannel }) and the client itself is passed in, rather than a bound () => this.outputChannel function. BaseLanguageClient's outputChannel getter already satisfies the interface structurally, so lazy output-channel creation is unchanged. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>

Pujitha Paladugu (pujitha24) · 2026-09-18T15:04:20Z

Dirk Bäumer (@dbaeumer) I've pushed changes addressing your review — the branch is now at 93a5c6e and CI is green. Could you take another look when you have a moment? Happy to keep iterating if anything is still off.

Dirk Bäumer (dbaeumer) · 2026-09-18T15:44:05Z

/AzurePipelines run

azure-pipelines · 2026-09-18T15:44:16Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

🟡 Changes recommended

Logging can recreate the disposed output channel after shutdown, and the behavior lacks regression coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Routes JSON-RPC diagnostics to the language client’s output channel for improved visibility.

Changes:

Adds an output-channel-backed JSON-RPC logger.
Preserves lazy channel creation.
Maps generic logs to info severity.

File summaries

File	Description
`client/src/common/client.ts`	Connects protocol logging to the client output channel.

Review details

Files reviewed: 1/1 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+	outputChannel: LogOutputChannel;
+}
+
+class OutputChannelLogger implements Logger {


Dirk Bäumer (dbaeumer) · 2026-09-18T15:49:56Z

Deleted user (@ghost) see the Copilot review comments.

Per Copilot's review comment, OutputChannelLogger previously dereferenced channelProvider.outputChannel directly, bypassing BaseLanguageClient's shouldLogToOutputChannel() guard. An in-flight JSON-RPC handler rejection after shutdown() could still reach the logger and recreate a disposed output channel for a stopped client. ChannelProvider now exposes error/warn/info (matching the client's existing public logging methods) and OutputChannelLogger calls those with notifications disabled, so the stopped-state guard applies to protocol-level logging too. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>

Pujitha Paladugu (pujitha24) · 2026-09-19T14:58:00Z

Dirk Bäumer (@dbaeumer) I've pushed changes addressing your review — the branch is now at f739ee8 and CI is green. Could you take another look when you have a moment? Happy to keep iterating if anything is still off.

…annel Per Copilot's review comment, adds a test in the 'Server output' suite that triggers a jsonrpc-level diagnostic (a throwing notification handler) and asserts it reaches the client's guarded error() method (showNotification=false) rather than bypassing it, covering the routing fixed in f739ee8. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>

Copilot

Copilot review overview

🟢 Approval recommended

The implementation correctly addresses the reported logging visibility issue with focused integration coverage.

Review effort: Balanced
Findings: 1

Open (1)

The new routing and lazy channel creation are untested, although the existing Server output…

Resolved since last review (1)

This adapter always dereferences outputChannel, bypassing shouldLogToOutputChannel(). After…

Copilot's review flagged that the earlier regression test (1282da8) never verified "no channel is created before the first diagnostic". Investigating client.ts shows the output channel is already created during start() as a side effect of wiring the trace log-level listener, unrelated to diagnostics, so that literal assertion would be false. Instead this adds a test for the actual regression f739ee8 fixed: a diagnostic logged after stop() must not recreate the disposed output channel. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>

Dirk Bäumer (dbaeumer) · 2026-09-21T14:13:53Z

/AzurePipelines run

azure-pipelines · 2026-09-21T14:14:04Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Pujitha Paladugu (pujitha24) · 2026-09-21T14:45:57Z

Dirk Bäumer (@dbaeumer) I've pushed changes addressing your review — the branch is now at dbff547 and CI is green. Could you take another look when you have a moment? Happy to keep iterating if anything is still off.

Copilot

Copilot review overview

🟢 Approval recommended

The focused implementation preserves lifecycle safeguards and includes appropriate integration coverage.

Review effort: Balanced
Findings: 1

Open (1)

The new routing and lazy channel creation are untested, although the existing Server output…

Pujitha Paladugu (pujitha24) · 2026-09-21T16:41:10Z

This is approved with green CI and no outstanding comments as far as I can tell — ready whenever you have a moment. Happy to rebase first if you'd like it freshened.

Dirk Bäumer (dbaeumer) requested changes Sep 18, 2026

View reviewed changes

Dirk Bäumer (dbaeumer) self-assigned this Sep 18, 2026

Dirk Bäumer (dbaeumer) requested a balanced review from Copilot September 18, 2026 15:43

Copilot started reviewing on behalf of Dirk Bäumer (dbaeumer) September 18, 2026 15:44 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread client/src/common/client.ts

Comment thread client/src/common/client.ts

outputChannel: LogOutputChannel;

}

class OutputChannelLogger implements Logger {

Dirk Bäumer (dbaeumer) requested a balanced review from Copilot September 21, 2026 11:13

Copilot started reviewing on behalf of Dirk Bäumer (dbaeumer) September 21, 2026 11:14 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Dirk Bäumer (dbaeumer) self-requested a review September 21, 2026 14:13

Dirk Bäumer (dbaeumer) requested a balanced review from Copilot September 21, 2026 15:16

Copilot started reviewing on behalf of Dirk Bäumer (dbaeumer) September 21, 2026 15:17 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Dirk Bäumer (dbaeumer) approved these changes Sep 21, 2026

View reviewed changes

Sandeep Somavarapu (sandy081) approved these changes Sep 21, 2026

View reviewed changes

TylerLeonhardt approved these changes Sep 21, 2026

View reviewed changes

roblourens approved these changes Sep 21, 2026

View reviewed changes

Dirk Bäumer (dbaeumer) merged commit a7ca284 into microsoft:main Sep 22, 2026
6 checks passed

Conversation

Pujitha Paladugu (pujitha24) commented Sep 8, 2026

Uh oh!

azure-pipelines Bot commented Sep 8, 2026

Uh oh!

Dirk Bäumer (dbaeumer) Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

Pujitha Paladugu (pujitha24) commented Sep 18, 2026

Uh oh!

Dirk Bäumer (dbaeumer) commented Sep 18, 2026

Uh oh!

azure-pipelines Bot commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Dirk Bäumer (dbaeumer) commented Sep 18, 2026

Uh oh!

Pujitha Paladugu (pujitha24) commented Sep 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Dirk Bäumer (dbaeumer) commented Sep 21, 2026

Uh oh!

azure-pipelines Bot commented Sep 21, 2026

Uh oh!

Pujitha Paladugu (pujitha24) commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Pujitha Paladugu (pujitha24) commented Sep 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants