feat(telemetry): skip unreadable files during export instead of failing

A corrupt telemetry file no longer aborts the whole export. The stream
reports the file and moves on, keeping events already parsed from it.
Each skip is logged with file:line detail, the completion notification
becomes a warning naming the skip count, and the diagnostic span gains
a file.skipped_count measurement (omitted at zero).

Author: EhabY

src/instrumentation/EVENTS.md

@@ -298,6 +298,7 @@ Emitted by `DiagnosticTelemetry` around each diagnostic command.
 | `interval.count` (measurement)             | speed test only                                                  |
 | `throughput_mbits` (measurement)           | speed test only                                                  |
 | `event.count` (measurement)                | telemetry export only                                            |
+| `file.skipped_count` (measurement)         | telemetry export only; unreadable files skipped, omitted at zero |
 
 ## Deployment
 

src/instrumentation/diagnostics.ts

@@ -2,6 +2,7 @@ import { recordAborted, recordError } from "./outcomes";
 
 import type { SpeedtestResult } from "@repo/shared";
 
+import type { ExportResult } from "../telemetry/export/pipeline";
 import type { ExportFormat } from "../telemetry/export/writers/types";
 import type { TelemetryReporter } from "../telemetry/reporter";
 import type { Span } from "../telemetry/span";
@@ -29,7 +30,7 @@ export interface DiagnosticTrace {
 	error(category?: DiagnosticErrorCategory): void;
 	setRequestedDuration(seconds: number): void;
 	succeedSpeedtest(result: SpeedtestResult): void;
-	succeedExport(format: ExportFormat, eventCount: number): void;
+	succeedExport(format: ExportFormat, result: ExportResult): void;
 }
 
 /** Emits `command.diagnostic.completed` around each diagnostic command. */
@@ -71,8 +72,11 @@ class SpanDiagnosticTrace implements DiagnosticTrace {
 		);
 	}
 
-	public succeedExport(format: ExportFormat, eventCount: number): void {
+	public succeedExport(format: ExportFormat, result: ExportResult): void {
 		this.span.setProperty("format", format);
-		this.span.setMeasurement("event.count", eventCount);
+		this.span.setMeasurement("event.count", result.eventCount);
+		if (result.skippedFileCount > 0) {
+			this.span.setMeasurement("file.skipped_count", result.skippedFileCount);
+		}
 	}
 }

src/telemetry/export/command.ts

@@ -10,6 +10,7 @@ import {
 import {
 	collectTelemetryExport,
 	type ExportRequest,
+	type ExportResult,
 	type ExportRuntime,
 } from "./pipeline";
 import { promptForExport, type ExportChoice } from "./prompts";
@@ -21,8 +22,6 @@ import type { FlushStatus } from "../service";
 
 import type { ExportFormat } from "./writers/types";
 
-const REVEAL_ACTION = "Reveal in File Explorer";
-
 const PROGRESS_OPTIONS = {
 	location: vscode.ProgressLocation.Notification,
 	title: "Exporting Coder telemetry",
@@ -36,7 +35,7 @@ const PROGRESS_OPTIONS = {
 export interface ExportTelemetryObserver {
 	abort(stage: "prompt" | "progress"): void;
 	error(): void;
-	succeedExport(format: ExportFormat, eventCount: number): void;
+	succeedExport(format: ExportFormat, result: ExportResult): void;
 }
 
 export async function runExportTelemetryCommand(
@@ -87,12 +86,15 @@ function exportRuntime(
 			),
 		onCleanupError: (err, target) =>
 			logger.warn("Failed to clean up after telemetry export", target, err),
+		// The skipped file's name and line are in the error message.
+		onFileSkipped: (err) =>
+			logger.warn("Telemetry export skipped an unreadable file", err),
 	};
 }
 
 /** Turns the export result into the matching user-facing notification. */
 async function reportOutcome(
-	result: ProgressResult<number>,
+	result: ProgressResult<ExportResult>,
 	choice: ExportChoice,
 	logger: Logger,
 	observer: ExportTelemetryObserver,
@@ -110,29 +112,59 @@ async function reportOutcome(
 		return;
 	}
 
-	const eventCount = result.value;
-	observer.succeedExport(choice.format, eventCount);
+	const { eventCount, skippedFileCount } = result.value;
+	observer.succeedExport(choice.format, result.value);
 	if (eventCount === 0) {
-		void vscode.window.showInformationMessage(
+		void showExportMessage(
 			`No telemetry events found for ${choice.range.label}.`,
+			skippedFileCount,
+			logger,
 		);
 		return;
 	}
-	await notifyExportSucceeded(choice.outputPath, eventCount, logger);
+	const action = await showExportMessage(
+		`Exported ${formatEventCount(eventCount)} to ${choice.outputPath}.`,
+		skippedFileCount,
+		logger,
+		"Reveal in File Explorer",
+	);
+	if (action === "Reveal in File Explorer") {
+		await revealExportedFile(choice.outputPath, logger);
+	}
 }
 
-async function notifyExportSucceeded(
-	outputPath: string,
-	eventCount: number,
+type ExportAction = "Reveal in File Explorer";
+
+/**
+ * A partial export warns and names the unreadable-file count. "Show Output"
+ * is offered on warnings and handled here; only caller actions are returned.
+ */
+async function showExportMessage(
+	message: string,
+	skippedFileCount: number,
 	logger: Logger,
-): Promise<void> {
-	const action = await vscode.window.showInformationMessage(
-		`Exported ${formatEventCount(eventCount)} to ${outputPath}.`,
-		REVEAL_ACTION,
+	...actions: ExportAction[]
+): Promise<ExportAction | undefined> {
+	if (skippedFileCount === 0) {
+		return vscode.window.showInformationMessage(message, ...actions);
+	}
+	const files = skippedFileCount === 1 ? "file" : "files";
+	const action = await vscode.window.showWarningMessage(
+		`${message} ${skippedFileCount} ${files} could not be read. Check the Coder output for details.`,
+		...actions,
+		"Show Output",
 	);
-	if (action !== REVEAL_ACTION) {
-		return;
+	if (action === "Show Output") {
+		logger.show();
+		return undefined;
 	}
+	return action;
+}
+
+async function revealExportedFile(
+	outputPath: string,
+	logger: Logger,
+): Promise<void> {
 	try {
 		await vscode.commands.executeCommand(
 			"revealFileInOS",

src/telemetry/export/files.ts

@@ -50,10 +50,13 @@ export async function listTelemetryFilesForRange(
 /**
  * Yields events from `filePaths` in order, keeping only those whose timestamp
  * falls inside `range`. Reads line-by-line so memory stays flat on big files.
+ * A file that cannot be read or parsed is reported through `onFileError` and
+ * skipped from its first bad line, so one bad file cannot sink an export.
  */
 export async function* streamTelemetryEvents(
 	filePaths: readonly string[],
 	range: TelemetryDateRange,
+	onFileError: (err: Error, fileName: string) => void,
 ): AsyncIterable<TelemetryEvent> {
 	for (const filePath of filePaths) {
 		const name = path.basename(filePath);
@@ -76,13 +79,15 @@ export async function* streamTelemetryEvents(
 				}
 			}
 		} catch (err) {
-			if (err instanceof TelemetryFileParseError) {
-				throw err;
-			}
 			const at = lineNumber > 0 ? `:${lineNumber}` : "";
-			throw new Error(
-				`Failed to read telemetry file ${name}${at}: ${toError(err).message}`,
-				{ cause: err },
+			onFileError(
+				err instanceof TelemetryFileParseError
+					? err
+					: new Error(
+							`Failed to read telemetry file ${name}${at}: ${toError(err).message}`,
+							{ cause: err },
+						),
+				name,
 			);
 		} finally {
 			try {

src/telemetry/export/pipeline.ts

@@ -17,6 +17,12 @@ export interface ExportRequest {
 	readonly writer: ExportWriter;
 }
 
+export interface ExportResult {
+	readonly eventCount: number;
+	/** Files skipped because they could not be read or parsed. */
+	readonly skippedFileCount: number;
+}
+
 /**
  * Host hooks the export needs: cancellation, flush, progress, and a cleanup
  * callback. Free of VS Code types so the pipeline is testable without a UI.
@@ -27,19 +33,22 @@ export interface ExportRuntime {
 	readonly report: (message: string) => void;
 	/** The pre-export flush did not fully succeed, so recent events may be missing. */
 	readonly onFlushIncomplete: () => void;
+	/** A file could not be read or parsed; the export continues without it (caller logs). */
+	readonly onFileSkipped: (err: Error, fileName: string) => void;
 	/** A temp file, staging dir, or empty export could not be removed (caller logs). */
 	readonly onCleanupError: (err: unknown, target: string) => void;
 }
 
 /**
  * Flushes telemetry, then streams every event in the range to `outputPath`.
- * Returns the number written; an export matching nothing leaves no file
- * behind. Throws on cancellation or write failure.
+ * Returns the count written and any unreadable files skipped; an export
+ * matching nothing leaves no file behind. Throws on cancellation or write
+ * failure.
  */
 export async function collectTelemetryExport(
 	request: ExportRequest,
 	runtime: ExportRuntime,
-): Promise<number> {
+): Promise<ExportResult> {
 	runtime.report("Flushing buffered events...");
 	const flushStatus = await runtime.flushTelemetry();
 	throwIfAborted(runtime.signal);
@@ -53,12 +62,16 @@ export async function collectTelemetryExport(
 		request.range,
 	);
 	if (filePaths.length === 0) {
-		return 0;
+		return { eventCount: 0, skippedFileCount: 0 };
 	}
 
 	runtime.report("Writing export...");
+	let skippedFileCount = 0;
 	const events = abortable(
-		streamTelemetryEvents(filePaths, request.range),
+		streamTelemetryEvents(filePaths, request.range, (err, fileName) => {
+			skippedFileCount += 1;
+			runtime.onFileSkipped(err, fileName);
+		}),
 		runtime.signal,
 	);
 	const eventCount = await request.writer(
@@ -70,7 +83,7 @@ export async function collectTelemetryExport(
 	if (eventCount === 0) {
 		await removeEmptyExport(request.outputPath, runtime.onCleanupError);
 	}
-	return eventCount;
+	return { eventCount, skippedFileCount };
 }
 
 /** Removes the file a writer produced for an export that matched no events. */

test/unit/telemetry/export/command.test.ts

@@ -40,7 +40,7 @@ const OK_FLUSH: FlushStatus = { ok: true, sinks: [] };
 function setup(
 	opts: {
 		choice?: ExportChoice;
-		outcome?: { count: number } | { error: unknown };
+		outcome?: { count: number; skipped?: number } | { error: unknown };
 		revealChoice?: string;
 	} = {},
 ) {
@@ -56,21 +56,26 @@ function setup(
 	if ("error" in outcome) {
 		vi.mocked(collectTelemetryExport).mockRejectedValue(outcome.error);
 	} else {
-		vi.mocked(collectTelemetryExport).mockResolvedValue(outcome.count);
+		vi.mocked(collectTelemetryExport).mockResolvedValue({
+			eventCount: outcome.count,
+			skippedFileCount: outcome.skipped ?? 0,
+		});
 	}
 
 	const observer: ExportTelemetryObserver = {
 		abort: vi.fn(),
 		error: vi.fn(),
 		succeedExport: vi.fn(),
 	};
+	const logger = createMockLogger();
 
 	return {
 		observer,
+		logger,
 		run: () =>
 			runExportTelemetryCommand(
 				TELEMETRY_DIR,
-				createMockLogger(),
+				logger,
 				vi.fn(() => Promise.resolve(OK_FLUSH)),
 				context,
 				observer,
@@ -115,7 +120,7 @@ describe("runExportTelemetryCommand", () => {
 		vi.mocked(collectTelemetryExport).mockImplementation(
 			(_request, runtime) => {
 				onFlushIncomplete = runtime.onFlushIncomplete;
-				return Promise.resolve(2);
+				return Promise.resolve({ eventCount: 2, skippedFileCount: 0 });
 			},
 		);
 
@@ -136,14 +141,42 @@ describe("runExportTelemetryCommand", () => {
 
 			await run();
 
-			expect(observer.succeedExport).toHaveBeenCalledWith("json", count);
+			expect(observer.succeedExport).toHaveBeenCalledWith("json", {
+				eventCount: count,
+				skippedFileCount: 0,
+			});
 
 			expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
 				`${message} ${OUTPUT_PATH}.`,
 				REVEAL_ACTION,
 			);
 		});
 
+		it("warns and names the skip count when files were skipped", async () => {
+			const { run } = setup({ outcome: { count: 2, skipped: 1 } });
+
+			await run();
+
+			expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(
+				`Exported 2 telemetry events to ${OUTPUT_PATH}. 1 file could not be read. Check the Coder output for details.`,
+				REVEAL_ACTION,
+				"Show Output",
+			);
+			expect(vscode.window.showInformationMessage).not.toHaveBeenCalled();
+		});
+
+		it("opens the Coder output when Show Output is clicked", async () => {
+			const { logger, run } = setup({ outcome: { count: 2, skipped: 1 } });
+			vi.mocked(vscode.window.showWarningMessage).mockResolvedValue(
+				"Show Output" as never,
+			);
+
+			await run();
+
+			expect(logger.show).toHaveBeenCalled();
+			expect(vscode.commands.executeCommand).not.toHaveBeenCalled();
+		});
+
 		it("reveals the file when the user clicks the action", async () => {
 			const { run } = setup({ revealChoice: REVEAL_ACTION });
 
@@ -181,12 +214,26 @@ describe("runExportTelemetryCommand", () => {
 
 			await run();
 
-			expect(observer.succeedExport).toHaveBeenCalledWith("json", 0);
+			expect(observer.succeedExport).toHaveBeenCalledWith("json", {
+				eventCount: 0,
+				skippedFileCount: 0,
+			});
 
 			expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
 				"No telemetry events found for Last 24 hours.",
 			);
 		});
+
+		it("warns when every matching file was skipped", async () => {
+			const { run } = setup({ outcome: { count: 0, skipped: 2 } });
+
+			await run();
+
+			expect(vscode.window.showWarningMessage).toHaveBeenCalledWith(
+				"No telemetry events found for Last 24 hours. 2 files could not be read. Check the Coder output for details.",
+				"Show Output",
+			);
+		});
 	});
 
 	describe("failure", () => {