Fix circular reference JSON serialization in telemetry logging (#4150)

This commit is contained in:
Bryan Morgan 2025-07-14 16:20:06 -04:00 committed by GitHub
parent 5008aea90d
commit ff3722a3a7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 294 additions and 17 deletions

View File

@ -23,6 +23,7 @@ import {
getCachedGoogleAccount,
getLifetimeGoogleAccounts,
} from '../../utils/user_account.js';
import { safeJsonStringify } from '../../utils/safeJsonStringify.js';
const start_session_event_name = 'start_session';
const new_prompt_event_name = 'new_prompt';
@ -65,7 +66,7 @@ export class ClearcutLogger {
this.events.push([
{
event_time_ms: Date.now(),
source_extension_json: JSON.stringify(event),
source_extension_json: safeJsonStringify(event),
},
]);
}
@ -121,7 +122,7 @@ export class ClearcutLogger {
log_event: eventsToSend,
},
];
const body = JSON.stringify(request);
const body = safeJsonStringify(request);
const options = {
hostname: 'play.googleapis.com',
path: '/log',

View File

@ -0,0 +1,62 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* Integration test to verify circular reference handling with proxy agents
*/
import { describe, it, expect } from 'vitest';
import { ClearcutLogger } from './clearcut-logger/clearcut-logger.js';
import { Config } from '../config/config.js';
describe('Circular Reference Integration Test', () => {
it('should handle HttpsProxyAgent-like circular references in clearcut logging', () => {
// Create a mock config with proxy
const mockConfig = {
getTelemetryEnabled: () => true,
getUsageStatisticsEnabled: () => true,
getSessionId: () => 'test-session',
getModel: () => 'test-model',
getEmbeddingModel: () => 'test-embedding',
getDebugMode: () => false,
getProxy: () => 'http://proxy.example.com:8080',
} as unknown as Config;
// Simulate the structure that causes the circular reference error
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const proxyAgentLike: any = {
sockets: {},
options: { proxy: 'http://proxy.example.com:8080' },
};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const socketLike: any = {
_httpMessage: {
agent: proxyAgentLike,
socket: null,
},
};
socketLike._httpMessage.socket = socketLike; // Create circular reference
proxyAgentLike.sockets['cloudcode-pa.googleapis.com:443'] = [socketLike];
// Create an event that would contain this circular structure
const problematicEvent = {
error: new Error('Network error'),
function_args: {
filePath: '/test/file.txt',
httpAgent: proxyAgentLike, // This would cause the circular reference
},
};
// Test that ClearcutLogger can handle this
const logger = ClearcutLogger.getInstance(mockConfig);
expect(() => {
logger?.enqueueLogEvent(problematicEvent);
}).not.toThrow();
});
});

View File

@ -0,0 +1,119 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* Test to verify circular reference handling in telemetry logging
*/
import { describe, it, expect } from 'vitest';
import { logToolCall } from './loggers.js';
import { ToolCallEvent } from './types.js';
import { Config } from '../config/config.js';
import { CompletedToolCall } from '../core/coreToolScheduler.js';
import { ToolCallRequestInfo, ToolCallResponseInfo } from '../core/turn.js';
import { Tool } from '../tools/tools.js';
describe('Circular Reference Handling', () => {
it('should handle circular references in tool function arguments', () => {
// Create a mock config
const mockConfig = {
getTelemetryEnabled: () => true,
getUsageStatisticsEnabled: () => true,
getSessionId: () => 'test-session',
getModel: () => 'test-model',
getEmbeddingModel: () => 'test-embedding',
getDebugMode: () => false,
} as unknown as Config;
// Create an object with circular references (similar to HttpsProxyAgent)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const circularObject: any = {
sockets: {},
agent: null,
};
circularObject.agent = circularObject; // Create circular reference
circularObject.sockets['test-host'] = [
{ _httpMessage: { agent: circularObject } },
];
// Create a mock CompletedToolCall with circular references in function_args
const mockRequest: ToolCallRequestInfo = {
callId: 'test-call-id',
name: 'ReadFile',
args: circularObject, // This would cause the original error
isClientInitiated: false,
prompt_id: 'test-prompt-id',
};
const mockResponse: ToolCallResponseInfo = {
callId: 'test-call-id',
responseParts: [{ text: 'test result' }],
resultDisplay: undefined,
error: undefined, // undefined means success
};
const mockCompletedToolCall: CompletedToolCall = {
status: 'success',
request: mockRequest,
response: mockResponse,
tool: {} as Tool,
durationMs: 100,
};
// Create a tool call event with circular references in function_args
const event = new ToolCallEvent(mockCompletedToolCall);
// This should not throw an error
expect(() => {
logToolCall(mockConfig, event);
}).not.toThrow();
});
it('should handle normal objects without circular references', () => {
const mockConfig = {
getTelemetryEnabled: () => true,
getUsageStatisticsEnabled: () => true,
getSessionId: () => 'test-session',
getModel: () => 'test-model',
getEmbeddingModel: () => 'test-embedding',
getDebugMode: () => false,
} as unknown as Config;
const normalObject = {
filePath: '/test/path',
options: { encoding: 'utf8' },
};
const mockRequest: ToolCallRequestInfo = {
callId: 'test-call-id',
name: 'ReadFile',
args: normalObject,
isClientInitiated: false,
prompt_id: 'test-prompt-id',
};
const mockResponse: ToolCallResponseInfo = {
callId: 'test-call-id',
responseParts: [{ text: 'test result' }],
resultDisplay: undefined,
error: undefined, // undefined means success
};
const mockCompletedToolCall: CompletedToolCall = {
status: 'success',
request: mockRequest,
response: mockResponse,
tool: {} as Tool,
durationMs: 100,
};
const event = new ToolCallEvent(mockCompletedToolCall);
expect(() => {
logToolCall(mockConfig, event);
}).not.toThrow();
});
});

View File

@ -35,6 +35,7 @@ import {
import { isTelemetrySdkInitialized } from './sdk.js';
import { uiTelemetryService, UiEvent } from './uiTelemetry.js';
import { ClearcutLogger } from './clearcut-logger/clearcut-logger.js';
import { safeJsonStringify } from '../utils/safeJsonStringify.js';
const shouldLogUserPrompts = (config: Config): boolean =>
config.getTelemetryLogPromptsEnabled();
@ -115,7 +116,7 @@ export function logToolCall(config: Config, event: ToolCallEvent): void {
...event,
'event.name': EVENT_TOOL_CALL,
'event.timestamp': new Date().toISOString(),
function_args: JSON.stringify(event.function_args, null, 2),
function_args: safeJsonStringify(event.function_args, 2),
};
if (event.error) {
attributes['error.message'] = event.error;

View File

@ -44,20 +44,9 @@ export function isProQuotaExceededError(error: unknown): boolean {
// - "Quota exceeded for quota metric 'Gemini 2.5-preview Pro Requests'"
// We use string methods instead of regex to avoid ReDoS vulnerabilities
const checkMessage = (message: string): boolean => {
console.log('[DEBUG] isProQuotaExceededError checking message:', message);
const result =
message.includes("Quota exceeded for quota metric 'Gemini") &&
message.includes("Pro Requests'");
console.log('[DEBUG] isProQuotaExceededError result:', result);
return result;
};
// Log the full error object to understand its structure
console.log(
'[DEBUG] isProQuotaExceededError - full error object:',
JSON.stringify(error, null, 2),
);
const checkMessage = (message: string): boolean =>
message.includes("Quota exceeded for quota metric 'Gemini") &&
message.includes("Pro Requests'");
if (typeof error === 'string') {
return checkMessage(error);

View File

@ -0,0 +1,73 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import { safeJsonStringify } from './safeJsonStringify.js';
describe('safeJsonStringify', () => {
it('should stringify normal objects without issues', () => {
const obj = { name: 'test', value: 42 };
const result = safeJsonStringify(obj);
expect(result).toBe('{"name":"test","value":42}');
});
it('should handle circular references by replacing them with [Circular]', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = { name: 'test' };
obj.circular = obj; // Create circular reference
const result = safeJsonStringify(obj);
expect(result).toBe('{"name":"test","circular":"[Circular]"}');
});
it('should handle complex circular structures like HttpsProxyAgent', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const agent: any = {
sockets: {},
options: { host: 'example.com' },
};
agent.sockets['example.com'] = [{ agent }];
const result = safeJsonStringify(agent);
expect(result).toContain('[Circular]');
expect(result).toContain('example.com');
});
it('should respect the space parameter for formatting', () => {
const obj = { name: 'test', value: 42 };
const result = safeJsonStringify(obj, 2);
expect(result).toBe('{\n "name": "test",\n "value": 42\n}');
});
it('should handle circular references with formatting', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const obj: any = { name: 'test' };
obj.circular = obj;
const result = safeJsonStringify(obj, 2);
expect(result).toBe('{\n "name": "test",\n "circular": "[Circular]"\n}');
});
it('should handle arrays with circular references', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const arr: any[] = [{ id: 1 }];
arr[0].parent = arr; // Create circular reference
const result = safeJsonStringify(arr);
expect(result).toBe('[{"id":1,"parent":"[Circular]"}]');
});
it('should handle null and undefined values', () => {
expect(safeJsonStringify(null)).toBe('null');
expect(safeJsonStringify(undefined)).toBe(undefined);
});
it('should handle primitive values', () => {
expect(safeJsonStringify('test')).toBe('"test"');
expect(safeJsonStringify(42)).toBe('42');
expect(safeJsonStringify(true)).toBe('true');
});
});

View File

@ -0,0 +1,32 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* Safely stringifies an object to JSON, handling circular references by replacing them with [Circular].
*
* @param obj - The object to stringify
* @param space - Optional space parameter for formatting (defaults to no formatting)
* @returns JSON string with circular references replaced by [Circular]
*/
export function safeJsonStringify(
obj: unknown,
space?: string | number,
): string {
const seen = new WeakSet();
return JSON.stringify(
obj,
(key, value) => {
if (typeof value === 'object' && value !== null) {
if (seen.has(value)) {
return '[Circular]';
}
seen.add(value);
}
return value;
},
space,
);
}