feat: buffer connection logs and flush them on failure by aqandrew · Pull Request #1100 · coder/vscode-coder

aqandrew · 2026-08-26T21:34:32Z

Implements RFC requirement 13 / DEVEX-669: buffer connection debug logs in memory below the current log level and flush them on a genuine connection failure, so a support bundle captures the detail leading up to the failure without the user having enabled debug logging beforehand.

What this does

Adds a BufferingLogger decorator (src/logging/logBuffer.ts) that wraps the "Coder" output channel and keeps a bounded, in-memory ring of the log lines that sit below the channel's current level — the ones it would otherwise drop. Only below-level lines are buffered, so nothing already written is duplicated.
On a connection failure, flush(reason) re-emits the captured lines into the output channel (each marked [buffered] with its original ISO timestamp and level) at the least-verbose level the channel still persists, so they land on disk and in support bundles.
Wires the buffer into ServiceContainer and adds the coder.connectionLogBuffer.size setting (default 1000, 0 disables).
Flushes only on genuine failures, never on transient drops or intentional teardown:
- a reconnecting WebSocket terminal failure (unrecoverable_close, unrecoverable_http, certificate_error);
- a WorkspaceMonitor socket error;
- an agent reported as disconnected during connection.
A short suppression window (5s) coalesces the burst of signals a single outage often triggers into one flush.
Documents the behavior, config, hard-kill/OOM loss limitation, and SSH log scope in CONTRIBUTING.md.

Scope notes

Extension SSH debug logs that pass through the shared logger are buffered. The CLI ProxyCommand writes its own file logs under coder.proxyLogDirectory, which support bundles already collect from disk, so those are not buffered here.
The buffer lives in memory, so a hard kill or out-of-memory event loses it (documented).

Commits

BufferingLogger + unit tests
container wiring + config
failure-site wiring + tests
docs

Testing

pnpm typecheck, pnpm format:check, and pnpm lint are clean.
Affected/dependent unit suites pass (logBuffer, reconnectingWebSocket, workspaceMonitor, workspaceStateMachine, coderApi, plus the container-mock consumers).

Implementation plan & design decisions

Design

Buffer: bounded entry-count ring; captures only calls whose severity is below the channel's current level; oldest-eviction; live-resizable via config.
Flush target (D3): replay into the existing "Coder" output channel at a level that still persists, with a [buffered] marker plus original level/timestamp, chronologically next to the real failure logs. Support bundles already collect the on-disk VS Code logs, so no separate sink is needed.
Flush reasons (D4): genuine, surfaced connection failures only — reconnecting-socket terminal failures, WorkspaceMonitor.notifyError, and agent disconnected. Never on transient retrying drops or intentional teardown (manual_disconnect, normal_close, replaced, dispose/deactivate/reload). A single isConnectionFailure(reason) predicate gates the socket sites, and a short suppression window coalesces bursts.
SSH scope (D5): buffer extension SSH debug passing through the shared Logger; do not buffer CLI ProxyCommand file logs already handled via coder.proxyLogDirectory.

Decisions

D1: buffer all below-level session logs.
D2: bound by entry count.
D3: replay into the existing Coder output channel at a persisted level with [buffered] marker/original level/timestamp.
D4: flush only on genuine connection failure; not transient or intentional teardown.
D5: buffer extension SSH debug through the shared Logger; not CLI ProxyCommand file logs.

🤖 Generated with Coder Agents. Reviewed and authored on behalf of @aqandrew.

Wraps a Logger and keeps a bounded in-memory ring of entries below the sink's current level (the ones it would drop). flush() replays them into the sink at a level guaranteed to be written, so a connection failure can preserve the debug detail leading up to it without the user having enabled debug logging. Only below-level entries are buffered (no duplication of what the sink already writes); flush is coalesced by a short suppression window.

linear-code · 2026-08-26T21:34:36Z

DEVEX-669

EhabY

Please address the inline comments on remote-client failure wiring, the monitor's failure trigger, and flush suppression. The remaining comments cover simplification, test coverage, and naming.

Review generated with Coder Agents on behalf of @EhabY.

EhabY · 2026-09-07T19:37:39Z

 		deploymentSessionAuth?.token,
 		output,
 		telemetryService,
+		(reason) => serviceContainer.getConnectionLogBuffer().flush(reason),


This only wires the shared client. Please pass the callback to workspaceClient in remote.ts too, since that client owns the remote workspace’s streams. Add a test verifying that its terminal socket failures flush the buffer. HTTP-only clients can still omit the callback.

EhabY · 2026-09-07T19:37:46Z

 			"Got empty error while monitoring workspace",
 		);
 		this.logger.error(message);
+		this.connectionLogBuffer.flush("workspace_monitor_error");


notifyError() is called when a workspace message cannot be parsed or its processing throws. Neither means the connection has failed; the socket can remain connected and deliver the next message.

Please keep the error log but remove this flush and the monitor’s buffer dependency. Terminal socket failures should flush through onConnectionFailure, once it is wired into the remote client.

EhabY · 2026-09-07T19:37:55Z

+		it("flushes the connection log buffer when the socket errors", async () => {
+			const { stream, connectionLogBuffer } = await setup();
+
+			stream.pushError(new Error("socket boom"));


pushError() emits a message with parseError, not a socket error. Please change this test to verify that malformed messages do not flush. Test the positive case through the real terminal-failure callback, so we cover the wiring rather than just a mock’s flush() call.

EhabY · 2026-09-07T19:38:01Z

+		if (now - this.lastFlushMs < this.flushSuppressionMs) {
+			return;
+		}


If failure A flushes, then new logs arrive before failure B one second later, B’s logs remain buffered with no later write scheduled.

Please remove the suppression window and its state. Clearing the buffer already prevents duplicate replay. Add a test showing that two consecutive failures each flush their newly accumulated entries.

EhabY · 2026-09-07T19:38:07Z

+		this.currentLevel = levelSource.getLogLevel();
+		this.levelSubscription = levelSource.onDidChangeLogLevel((level) => {
+			this.currentLevel = level;
+		});


Can we accept a single getLogLevel: () => number callback and call it from both record() and replayEmitter()? That removes LogLevelSource, the cached level, the change subscription, and its cleanup, while keeping the logger independent of VS Code.

EhabY · 2026-09-07T19:38:54Z

+/**
+ * Terminal-failure reasons: the socket has given up and surfaced an error,
+ * rather than dropping transiently and auto-reconnecting. These are the moments
+ * worth flushing the connection log buffer.
+ */
+const CONNECTION_FAILURE_REASONS: ReadonlySet<ConnectionStateReason> = new Set([
+	"unrecoverable_close",
+	"unrecoverable_http",
+	"certificate_error",
+]);
+
+/** Whether a state-transition reason represents a genuine connection failure. */
+export function isConnectionFailure(reason: ConnectionStateReason): boolean {


Please rename these to TERMINAL_CONNECTION_FAILURE_REASONS and isTerminalConnectionFailure. These are connection failures that stop automatic retries, not all connection errors. The comment can simply say: “Connection failures that stop automatic retries.”

EhabY · 2026-09-07T19:39:03Z

+
+type Level = keyof typeof SEVERITY;
+
+const LEVEL_LABEL: Record<Level, string> = {


Please use Readonly<Record<Level, string>> here so the labels cannot be reassigned. SEVERITY is already readonly through as const.

EhabY · 2026-09-07T19:39:08Z

+const CONNECTION_LOG_BUFFER_SIZE_KEY = "coder.connectionLogBuffer.size";
+const DEFAULT_CONNECTION_LOG_BUFFER_SIZE = 1000;


Please move these constants below the imports, before the class.

EhabY · 2026-09-07T19:39:14Z

+	flush(reason: string): void;
+}
+
+interface BufferedEntry {


Could we call this LogEntry? Its fields describe a log entry; none are specific to buffering.

EhabY · 2026-09-07T19:39:22Z

+/**
+ * Wraps a {@link Logger} and keeps a bounded, in-memory ring of entries whose
+ * level is **below the sink's current level** — the ones the sink would
+ * otherwise drop. On a connection failure, {@link flush} replays those entries
+ * into the sink so they persist to disk (and any support bundle), giving Support
+ * the debug detail leading up to the failure without the user having enabled
+ * debug logging beforehand.
+ *
+ * Only below-level entries are buffered, so nothing that the sink already writes
+ * is ever duplicated. Replay is emitted at the least-verbose level the sink
+ * still writes, so the flush lands regardless of the configured level.


Please shorten this to the behavior that matters: “Buffers entries below the current log level and replays them on failure at a level the output channel persists.” The implementation explains the rest. Please also avoid em-dashes in the added comments.

aqandrew added 4 commits August 26, 2026 21:01

feat(logging): wire the connection log buffer into the service container

69efd2f

feat(logging): flush the connection log buffer on connection failures

bbb9d5b

docs: document the connection log buffer

9c3d21b

github-actions Bot assigned aqandrew Aug 26, 2026

aqandrew marked this pull request as ready for review September 1, 2026 03:30

aqandrew requested a review from EhabY September 1, 2026 03:30

EhabY requested changes Sep 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: buffer connection logs and flush them on failure - #1100

feat: buffer connection logs and flush them on failure#1100
aqandrew wants to merge 4 commits into
mainfrom
aqandrew/devex-669-vs-code-add-log-buffer

aqandrew commented Aug 26, 2026

Uh oh!

linear-code Bot commented Aug 26, 2026

Uh oh!

EhabY left a comment

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

EhabY Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		type Level = keyof typeof SEVERITY;

		const LEVEL_LABEL: Record<Level, string> = {

		const CONNECTION_LOG_BUFFER_SIZE_KEY = "coder.connectionLogBuffer.size";
		const DEFAULT_CONNECTION_LOG_BUFFER_SIZE = 1000;

Uh oh!

Conversation

aqandrew commented Aug 26, 2026

What this does

Scope notes

Commits

Testing

Design

Decisions

Uh oh!

linear-code Bot commented Aug 26, 2026

Uh oh!

EhabY left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants