Fix JSON stringify circular structure when logging

src/api/coderApi.ts

@@ -1,4 +1,9 @@
-import { type AxiosInstance } from "axios";
+import {
+	type AxiosResponseHeaders,
+	type AxiosInstance,
+	type AxiosHeaders,
+	type AxiosResponseTransformer,
+} from "axios";
 import { Api } from "coder/site/src/api/api";
 import {
 	type GetInboxNotificationResponse,
@@ -23,6 +28,7 @@ import {
 	type RequestConfigWithMeta,
 	HttpClientLogLevel,
 } from "../logging/types";
+import { serializeValue, sizeOf } from "../logging/utils";
 import { WsLogger } from "../logging/wsLogger";
 import {
 	OneWayWebSocket,
@@ -207,7 +213,24 @@ function addLoggingInterceptors(client: AxiosInstance, logger: Logger) {
 		(config) => {
 			const configWithMeta = config as RequestConfigWithMeta;
 			configWithMeta.metadata = createRequestMeta();
-			logRequest(logger, configWithMeta, getLogLevel());
+
+			config.transformRequest = [
+				...wrapRequestTransform(
+					config.transformRequest || client.defaults.transformRequest || [],
+					configWithMeta,
+				),
+				(data) => {
+					// Log after setting the raw request size
+					logRequest(logger, configWithMeta, getLogLevel());
+					return data;
+				},
+			];
+
+			config.transformResponse = wrapResponseTransform(
+				config.transformResponse || client.defaults.transformResponse || [],
+				configWithMeta,
+			);
+
 			return config;
 		},
 		(error: unknown) => {
@@ -228,6 +251,66 @@ function addLoggingInterceptors(client: AxiosInstance, logger: Logger) {
 	);
 }
 
+function wrapRequestTransform(
+	transformer: AxiosResponseTransformer | AxiosResponseTransformer[],
+	config: RequestConfigWithMeta,
+): AxiosResponseTransformer[] {
+	return [
+		(data: unknown, headers: AxiosHeaders) => {
+			const transformerArray = Array.isArray(transformer)
+				? transformer
+				: [transformer];
+
+			// Transform the request first then estimate the size
+			const result = transformerArray.reduce(
+				(d, fn) => fn.call(config, d, headers),
+				data,
+			);
+
+			config.rawRequestSize = getSize(config.headers, result);
+
+			return result;
+		},
+	];
+}
+
+function wrapResponseTransform(
+	transformer: AxiosResponseTransformer | AxiosResponseTransformer[],
+	config: RequestConfigWithMeta,
+): AxiosResponseTransformer[] {
+	return [
+		(data: unknown, headers: AxiosResponseHeaders, status?: number) => {
+			// estimate the size before transforming the response
+			config.rawResponseSize = getSize(headers, data);
+
+			const transformerArray = Array.isArray(transformer)
+				? transformer
+				: [transformer];
+
+			return transformerArray.reduce(
+				(d, fn) => fn.call(config, d, headers, status),
+				data,
+			);
+		},
+	];
+}
+
+function getSize(headers: AxiosHeaders, data: unknown): number | undefined {
+	const contentLength = headers["content-length"];
+	if (contentLength !== undefined) {
+		return parseInt(contentLength, 10);
+	}
+
+	const size = sizeOf(data);
+	if (size !== undefined) {
+		return size;
+	}
+
+	// Fallback
+	const stringified = serializeValue(data);
+	return stringified === null ? undefined : Buffer.byteLength(stringified);
+}
+
 function getLogLevel(): HttpClientLogLevel {
 	const logLevelStr = vscode.workspace
 		.getConfiguration()

src/logging/formatters.ts

@@ -1,7 +1,6 @@
-import util from "node:util";
 import prettyBytes from "pretty-bytes";
 
-import { sizeOf } from "./utils";
+import { serializeValue } from "./utils";
 
 import type { AxiosRequestConfig } from "axios";
 
@@ -21,38 +20,11 @@ export function formatTime(ms: number): string {
 }
 
 export function formatMethod(method: string | undefined): string {
-	return (method ? method : "GET").toUpperCase();
+	return method?.toUpperCase() || "GET";
 }
 
-/**
- * Formats content-length for display. Returns the header value if available,
- * otherwise estimates size by serializing the data body (prefixed with ~).
- */
-export function formatContentLength(
-	headers: Record<string, unknown>,
-	data: unknown,
-): string {
-	const len = headers["content-length"];
-	if (len && typeof len === "string") {
-		const bytes = parseInt(len, 10);
-		return isNaN(bytes) ? "(? B)" : `(${prettyBytes(bytes)})`;
-	}
-
-	// Estimate from data if no header
-	const size = sizeOf(data);
-	if (size !== undefined) {
-		return `(${prettyBytes(size)})`;
-	}
-
-	if (typeof data === "object") {
-		const stringified = safeStringify(data);
-		if (stringified !== null) {
-			const bytes = Buffer.byteLength(stringified, "utf8");
-			return `(~${prettyBytes(bytes)})`;
-		}
-	}
-
-	return "(? B)";
+export function formatSize(size: number | undefined): string {
+	return size === undefined ? "(? B)" : `(${prettyBytes(size)})`;
 }
 
 export function formatUri(config: AxiosRequestConfig | undefined): string {
@@ -75,25 +47,8 @@ export function formatHeaders(headers: Record<string, unknown>): string {
 
 export function formatBody(body: unknown): string {
 	if (body) {
-		return safeStringify(body) ?? "<invalid body>";
+		return serializeValue(body) ?? "<invalid body>";
 	} else {
 		return "<no body>";
 	}
 }
-
-function safeStringify(data: unknown): string | null {
-	try {
-		return util.inspect(data, {
-			showHidden: false,
-			depth: Infinity,
-			maxArrayLength: Infinity,
-			maxStringLength: Infinity,
-			breakLength: Infinity,
-			compact: true,
-			getters: false, // avoid side-effects
-		});
-	} catch {
-		// Should rarely happen but just in case
-		return null;
-	}
-}

src/logging/httpLogger.ts

@@ -5,9 +5,9 @@ import { getErrorDetail } from "../error";
 
 import {
 	formatBody,
-	formatContentLength,
 	formatHeaders,
 	formatMethod,
+	formatSize,
 	formatTime,
 	formatUri,
 } from "./formatters";
@@ -42,11 +42,10 @@ export function logRequest(
 		return;
 	}
 
-	const { requestId, method, url } = parseConfig(config);
-	const len = formatContentLength(config.headers, config.data);
+	const { requestId, method, url, requestSize } = parseConfig(config);
 
 	const msg = [
-		`→ ${shortId(requestId)} ${method} ${url} ${len}`,
+		`→ ${shortId(requestId)} ${method} ${url} ${requestSize}`,
 		...buildExtraLogs(config.headers, config.data, logLevel),
 	];
 	logger.trace(msg.join("\n"));
@@ -64,11 +63,12 @@ export function logResponse(
 		return;
 	}
 
-	const { requestId, method, url, time } = parseConfig(response.config);
-	const len = formatContentLength(response.headers, response.data);
+	const { requestId, method, url, time, responseSize } = parseConfig(
+		response.config,
+	);
 
 	const msg = [
-		`← ${shortId(requestId)} ${response.status} ${method} ${url} ${len} ${time}`,
+		`← ${shortId(requestId)} ${response.status} ${method} ${url} ${responseSize} ${time}`,
 		...buildExtraLogs(response.headers, response.data, logLevel),
 	];
 	logger.trace(msg.join("\n"));
@@ -150,12 +150,16 @@ function parseConfig(config: RequestConfigWithMeta | undefined): {
 	method: string;
 	url: string;
 	time: string;
+	requestSize: string;
+	responseSize: string;
 } {
 	const meta = config?.metadata;
 	return {
 		requestId: meta?.requestId || "unknown",
 		method: formatMethod(config?.method),
 		url: formatUri(config),
 		time: meta ? formatTime(Date.now() - meta.startedAt) : "?ms",
+		requestSize: formatSize(config?.rawRequestSize),
+		responseSize: formatSize(config?.rawResponseSize),
 	};
 }

src/logging/types.ts

@@ -14,4 +14,6 @@ export interface RequestMeta {
 
 export type RequestConfigWithMeta = InternalAxiosRequestConfig & {
 	metadata?: RequestMeta;
+	rawRequestSize?: number;
+	rawResponseSize?: number;
 };

src/logging/utils.ts

@@ -1,10 +1,15 @@
 import { Buffer } from "node:buffer";
 import crypto from "node:crypto";
+import util from "node:util";
 
 export function shortId(id: string): string {
 	return id.slice(0, 8);
 }
 
+export function createRequestId(): string {
+	return crypto.randomUUID().replace(/-/g, "");
+}
+
 /**
  * Returns the byte size of the data if it can be determined from the data's intrinsic properties,
  * otherwise returns undefined (e.g., for plain objects and arrays that would require serialization).
@@ -13,7 +18,10 @@ export function sizeOf(data: unknown): number | undefined {
 	if (data === null || data === undefined) {
 		return 0;
 	}
-	if (typeof data === "number" || typeof data === "boolean") {
+	if (typeof data === "boolean") {
+		return 4;
+	}
+	if (typeof data === "number") {
 		return 8;
 	}
 	if (typeof data === "string" || typeof data === "bigint") {
@@ -36,6 +44,19 @@ export function sizeOf(data: unknown): number | undefined {
 	return undefined;
 }
 
-export function createRequestId(): string {
-	return crypto.randomUUID().replace(/-/g, "");
+export function serializeValue(data: unknown): string | null {
+	try {
+		return util.inspect(data, {
+			showHidden: false,
+			depth: Infinity,
+			maxArrayLength: Infinity,
+			maxStringLength: Infinity,
+			breakLength: Infinity,
+			compact: true,
+			getters: false, // avoid side-effects
+		});
+	} catch {
+		// Should rarely happen but just in case
+		return null;
+	}
 }

test/unit/logging/formatters.test.ts

@@ -2,9 +2,9 @@ import { describe, expect, it } from "vitest";
 
 import {
 	formatBody,
-	formatContentLength,
 	formatHeaders,
 	formatMethod,
+	formatSize,
 	formatTime,
 	formatUri,
 } from "@/logging/formatters";
@@ -34,40 +34,14 @@ describe("Logging formatters", () => {
 		});
 	});
 
-	describe("formatContentLength", () => {
-		it("uses content-length header when available", () => {
-			const result = formatContentLength({ "content-length": "1024" }, null);
-			expect(result).toContain("1.02 kB");
+	describe("formatSize", () => {
+		it("formats byte sizes using pretty-bytes", () => {
+			expect(formatSize(1024)).toContain("1.02 kB");
+			expect(formatSize(0)).toBe("(0 B)");
 		});
 
-		it("handles invalid content-length header", () => {
-			const result = formatContentLength({ "content-length": "invalid" }, null);
-			expect(result).toContain("?");
-		});
-
-		it.each([
-			["hello", 5],
-			[Buffer.from("test"), 4],
-			[123, 8],
-			[false, 8],
-			[BigInt(1234), 4],
-			[null, 0],
-			[undefined, 0],
-		])("calculates size for %s", (data: unknown, bytes: number) => {
-			const result = formatContentLength({}, data);
-			expect(result).toContain(`${bytes} B`);
-		});
-
-		it("estimates size for objects", () => {
-			const result = formatContentLength({}, { foo: "bar" });
-			expect(result).toMatch(/~\d+/);
-		});
-
-		it("handles circular references safely", () => {
-			const circular: Record<string, unknown> = { a: 1 };
-			circular.self = circular;
-			const result = formatContentLength({}, circular);
-			expect(result).toMatch(/~\d+/);
+		it("returns placeholder for undefined", () => {
+			expect(formatSize(undefined)).toBe("(? B)");
 		});
 	});