From 0b8301e289de261f1770ed265dd08ccd27536bfe Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 26 Apr 2026 12:03:20 +0800 Subject: [PATCH] fix(telemetry): use safeJsonStringify in FileExporter to avoid circular reference crash 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. --- .../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 00000000000..40a871de1ab --- /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 55fe641035b..def6e91f441 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 {