From 569cfe10fabd02f881a6b1f1211f14094e297270 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Sun, 26 Apr 2026 12:55:39 +0800 Subject: [PATCH] fix(telemetry): use safeJsonStringify in FileExporter to avoid circular reference crash (#3630) When --telemetry-outfile is configured, FileSpanExporter.serialize called JSON.stringify directly on OTel ReadableSpan instances. The spans hold a back-reference to BatchSpanProcessor (._shutdownOnce -> BindOnceFuture._that -> BatchSpanProcessor), which forms a cycle and triggers "TypeError: Converting circular structure to JSON" on every export. Combined with DiagConsoleLogger, the error was repeatedly printed to stderr and polluted the Ink TUI. Switch FileExporter.serialize to the existing safeJsonStringify utility, matching the upstream gemini-cli fix so future merges stay clean. Add a focused regression test that mimics the BatchSpanProcessor cycle shape; broader cycle behavior is already covered by safeJsonStringify.test.ts. Co-authored-by: wenshao --- .../core/src/telemetry/file-exporters.test.ts | 50 +++++++++++++++++++ packages/core/src/telemetry/file-exporters.ts | 3 +- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/telemetry/file-exporters.test.ts diff --git a/packages/core/src/telemetry/file-exporters.test.ts b/packages/core/src/telemetry/file-exporters.test.ts new file mode 100644 index 000000000..40a871de1 --- /dev/null +++ b/packages/core/src/telemetry/file-exporters.test.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { FileSpanExporter } from './file-exporters.js'; + +type SerializeAccess = { serialize: (data: unknown) => string }; + +describe('FileExporter.serialize', () => { + let tmpDir: string; + let exporter: FileSpanExporter; + let serialize: (data: unknown) => string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'file-exporters-test-')); + exporter = new FileSpanExporter(path.join(tmpDir, 'out.jsonl')); + serialize = (exporter as unknown as SerializeAccess).serialize.bind( + exporter, + ); + }); + + afterEach(async () => { + await exporter.shutdown(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + // Regression for upstream PR #4689: a raw JSON.stringify on a ReadableSpan + // crashed because BatchSpanProcessor._shutdownOnce -> BindOnceFuture._that + // forms a cycle. The exporter must delegate to safeJsonStringify so cycles + // become "[Circular]" instead of throwing. + it('does not throw on BatchSpanProcessor-shaped cycle', () => { + const proc: Record = { kind: 'BatchSpanProcessor' }; + const future: Record = { kind: 'BindOnceFuture' }; + proc['_shutdownOnce'] = future; + future['_that'] = proc; + const span = { name: 'span-1', _spanProcessor: proc }; + + expect(() => serialize(span)).not.toThrow(); + const out = serialize(span); + expect(out).toContain('"name": "span-1"'); + expect(out).toContain('"[Circular]"'); + expect(out.endsWith('\n')).toBe(true); + }); +}); diff --git a/packages/core/src/telemetry/file-exporters.ts b/packages/core/src/telemetry/file-exporters.ts index 55fe64103..def6e91f4 100644 --- a/packages/core/src/telemetry/file-exporters.ts +++ b/packages/core/src/telemetry/file-exporters.ts @@ -17,6 +17,7 @@ import type { PushMetricExporter, } from '@opentelemetry/sdk-metrics'; import { AggregationTemporality } from '@opentelemetry/sdk-metrics'; +import { safeJsonStringify } from '../utils/safeJsonStringify.js'; class FileExporter { protected writeStream: fs.WriteStream; @@ -26,7 +27,7 @@ class FileExporter { } protected serialize(data: unknown): string { - return JSON.stringify(data, null, 2) + '\n'; + return safeJsonStringify(data, 2) + '\n'; } shutdown(): Promise {