From dcb67c32a5e246f9df64a18399c6a13051db7f30 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Sat, 10 May 2025 14:14:57 -0700 Subject: [PATCH] Log server information on error. - The goal of this is to enable us to better diagnose server issues when they occur. - Added tests because why not. --- packages/server/src/core/client.ts | 45 +++- .../server/src/utils/errorReporting.test.ts | 220 ++++++++++++++++++ packages/server/src/utils/errorReporting.ts | 117 ++++++++++ 3 files changed, 377 insertions(+), 5 deletions(-) create mode 100644 packages/server/src/utils/errorReporting.test.ts create mode 100644 packages/server/src/utils/errorReporting.ts diff --git a/packages/server/src/core/client.ts b/packages/server/src/core/client.ts index 4c20c0ad..b8d47476 100644 --- a/packages/server/src/core/client.ts +++ b/packages/server/src/core/client.ts @@ -22,6 +22,7 @@ import { getCoreSystemPrompt } from './prompts.js'; import { ReadManyFilesTool } from '../tools/read-many-files.js'; import { getResponseText } from '../utils/generateContentResponseUtilities.js'; import { checkNextSpeaker } from '../utils/nextSpeakerChecker.js'; +import { reportError } from '../utils/errorReporting.js'; export class GeminiClient { private client: GoogleGenAI; @@ -87,8 +88,8 @@ export class GeminiClient { ); } } catch (error) { + // Not using reportError here as it's a startup/config phase, not a chat/generation phase error. console.error('Error reading full file context:', error); - // Optionally add an error message part to the context initialParts.push({ text: '\n--- Error reading full file context ---', }); @@ -125,7 +126,12 @@ export class GeminiClient { history, }); } catch (error) { - console.error('Error initializing Gemini chat session:', error); + await reportError( + error, + 'Error initializing Gemini chat session.', + history, + 'startChat', + ); const message = error instanceof Error ? error.message : 'Unknown error.'; throw new Error(`Failed to initialize chat: ${message}`); } @@ -185,18 +191,47 @@ export class GeminiClient { }); const text = getResponseText(result); if (!text) { - throw new Error('API returned an empty response.'); + const error = new Error( + 'API returned an empty response for generateJson.', + ); + await reportError( + error, + 'Error in generateJson: API returned an empty response.', + contents, + 'generateJson-empty-response', + ); + throw error; } try { return JSON.parse(text); } catch (parseError) { - console.error('Failed to parse JSON response:', text); + await reportError( + parseError, + 'Failed to parse JSON response from generateJson.', + { + responseTextFailedToParse: text, + originalRequestContents: contents, + }, + 'generateJson-parse', + ); throw new Error( `Failed to parse API response as JSON: ${parseError instanceof Error ? parseError.message : String(parseError)}`, ); } } catch (error) { - console.error('Error generating JSON content:', error); + // Avoid double reporting for the empty response case handled above + if ( + error instanceof Error && + error.message === 'API returned an empty response for generateJson.' + ) { + throw error; + } + await reportError( + error, + 'Error generating JSON content via API.', + contents, + 'generateJson-api', + ); const message = error instanceof Error ? error.message : 'Unknown API error.'; throw new Error(`Failed to generate JSON content: ${message}`); diff --git a/packages/server/src/utils/errorReporting.test.ts b/packages/server/src/utils/errorReporting.test.ts new file mode 100644 index 00000000..1faba5f6 --- /dev/null +++ b/packages/server/src/utils/errorReporting.test.ts @@ -0,0 +1,220 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; + +// Use a type alias for SpyInstance as it's not directly exported +type SpyInstance = ReturnType; +import { reportError } from './errorReporting.js'; +import fs from 'node:fs/promises'; +import os from 'node:os'; + +// Mock dependencies +vi.mock('node:fs/promises'); +vi.mock('node:os'); + +describe('reportError', () => { + let consoleErrorSpy: SpyInstance; + const MOCK_TMP_DIR = '/tmp'; + const MOCK_TIMESTAMP = '2025-01-01T00-00-00-000Z'; + + beforeEach(() => { + vi.resetAllMocks(); + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + (os.tmpdir as Mock).mockReturnValue(MOCK_TMP_DIR); + vi.spyOn(Date.prototype, 'toISOString').mockReturnValue(MOCK_TIMESTAMP); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + vi.restoreAllMocks(); + }); + + const getExpectedReportPath = (type: string) => + `${MOCK_TMP_DIR}/gemini-client-error-${type}-${MOCK_TIMESTAMP}.json`; + + it('should generate a report and log the path', async () => { + const error = new Error('Test error'); + error.stack = 'Test stack'; + const baseMessage = 'An error occurred.'; + const context = { data: 'test context' }; + const type = 'test-type'; + const expectedReportPath = getExpectedReportPath(type); + + (fs.writeFile as Mock).mockResolvedValue(undefined); + + await reportError(error, baseMessage, context, type); + + expect(os.tmpdir).toHaveBeenCalledTimes(1); + expect(fs.writeFile).toHaveBeenCalledWith( + expectedReportPath, + JSON.stringify( + { + error: { message: 'Test error', stack: error.stack }, + context, + }, + null, + 2, + ), + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Full report available at: ${expectedReportPath}`, + ); + }); + + it('should handle errors that are plain objects with a message property', async () => { + const error = { message: 'Test plain object error' }; + const baseMessage = 'Another error.'; + const type = 'general'; + const expectedReportPath = getExpectedReportPath(type); + + (fs.writeFile as Mock).mockResolvedValue(undefined); + await reportError(error, baseMessage); + + expect(fs.writeFile).toHaveBeenCalledWith( + expectedReportPath, + JSON.stringify( + { + error: { message: 'Test plain object error' }, + }, + null, + 2, + ), + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Full report available at: ${expectedReportPath}`, + ); + }); + + it('should handle string errors', async () => { + const error = 'Just a string error'; + const baseMessage = 'String error occurred.'; + const type = 'general'; + const expectedReportPath = getExpectedReportPath(type); + + (fs.writeFile as Mock).mockResolvedValue(undefined); + await reportError(error, baseMessage); + + expect(fs.writeFile).toHaveBeenCalledWith( + expectedReportPath, + JSON.stringify( + { + error: { message: 'Just a string error' }, + }, + null, + 2, + ), + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Full report available at: ${expectedReportPath}`, + ); + }); + + it('should log fallback message if writing report fails', async () => { + const error = new Error('Main error'); + const baseMessage = 'Failed operation.'; + const writeError = new Error('Failed to write file'); + const context = ['some context']; + const type = 'general'; + const expectedReportPath = getExpectedReportPath(type); + + (fs.writeFile as Mock).mockRejectedValue(writeError); + + await reportError(error, baseMessage, context, type); + + expect(fs.writeFile).toHaveBeenCalledWith( + expectedReportPath, + expect.any(String), + ); // It still tries to write + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Additionally, failed to write detailed error report:`, + writeError, + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Original error that triggered report generation:', + error, + ); + expect(consoleErrorSpy).toHaveBeenCalledWith('Original context:', context); + }); + + it('should handle stringification failure of report content (e.g. BigInt in context)', async () => { + const error = new Error('Main error'); + error.stack = 'Main stack'; + const baseMessage = 'Failed operation with BigInt.'; + const context = { a: BigInt(1) }; // BigInt cannot be stringified by JSON.stringify + const type = 'bigint-fail'; + const stringifyError = new TypeError( + 'Do not know how to serialize a BigInt', + ); + const expectedMinimalReportPath = getExpectedReportPath(type); + + // Simulate JSON.stringify throwing an error for the full report + const originalJsonStringify = JSON.stringify; + let callCount = 0; + vi.spyOn(JSON, 'stringify').mockImplementation((value, replacer, space) => { + callCount++; + if (callCount === 1) { + // First call is for the full report content + throw stringifyError; + } + // Subsequent calls (for minimal report) should succeed + return originalJsonStringify(value, replacer, space); + }); + + (fs.writeFile as Mock).mockResolvedValue(undefined); // Mock for the minimal report write + + await reportError(error, baseMessage, context, type); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Could not stringify report content (likely due to context):`, + stringifyError, + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Original error that triggered report generation:', + error, + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Original context could not be stringified or included in report.', + ); + // Check that it attempts to write a minimal report + expect(fs.writeFile).toHaveBeenCalledWith( + expectedMinimalReportPath, + originalJsonStringify( + { error: { message: error.message, stack: error.stack } }, + null, + 2, + ), + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Partial report (excluding context) available at: ${expectedMinimalReportPath}`, + ); + }); + + it('should generate a report without context if context is not provided', async () => { + const error = new Error('Error without context'); + error.stack = 'No context stack'; + const baseMessage = 'Simple error.'; + const type = 'general'; + const expectedReportPath = getExpectedReportPath(type); + + (fs.writeFile as Mock).mockResolvedValue(undefined); + await reportError(error, baseMessage, undefined, type); + + expect(fs.writeFile).toHaveBeenCalledWith( + expectedReportPath, + JSON.stringify( + { + error: { message: 'Error without context', stack: error.stack }, + }, + null, + 2, + ), + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `${baseMessage} Full report available at: ${expectedReportPath}`, + ); + }); +}); diff --git a/packages/server/src/utils/errorReporting.ts b/packages/server/src/utils/errorReporting.ts new file mode 100644 index 00000000..41ce3468 --- /dev/null +++ b/packages/server/src/utils/errorReporting.ts @@ -0,0 +1,117 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { Content } from '@google/genai'; + +interface ErrorReportData { + error: { message: string; stack?: string } | { message: string }; + context?: unknown; + additionalInfo?: Record; +} + +/** + * Generates an error report, writes it to a temporary file, and logs information to console.error. + * @param error The error object. + * @param context The relevant context (e.g., chat history, request contents). + * @param type A string to identify the type of error (e.g., 'startChat', 'generateJson-api'). + * @param baseMessage The initial message to log to console.error before the report path. + */ +export async function reportError( + error: Error | unknown, + baseMessage: string, + context?: Content[] | Record | unknown[], + type = 'general', +): Promise { + const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + const reportFileName = `gemini-client-error-${type}-${timestamp}.json`; + const reportPath = path.join(os.tmpdir(), reportFileName); + + let errorToReport: { message: string; stack?: string }; + if (error instanceof Error) { + errorToReport = { message: error.message, stack: error.stack }; + } else if ( + typeof error === 'object' && + error !== null && + 'message' in error + ) { + errorToReport = { + message: String((error as { message: unknown }).message), + }; + } else { + errorToReport = { message: String(error) }; + } + + const reportContent: ErrorReportData = { error: errorToReport }; + + if (context) { + reportContent.context = context; + } + + let stringifiedReportContent: string; + try { + stringifiedReportContent = JSON.stringify(reportContent, null, 2); + } catch (stringifyError) { + // This can happen if context contains something like BigInt + console.error( + `${baseMessage} Could not stringify report content (likely due to context):`, + stringifyError, + ); + console.error('Original error that triggered report generation:', error); + if (context) { + console.error( + 'Original context could not be stringified or included in report.', + ); + } + // Fallback: try to report only the error if context was the issue + try { + const minimalReportContent = { error: errorToReport }; + stringifiedReportContent = JSON.stringify(minimalReportContent, null, 2); + // Still try to write the minimal report + await fs.writeFile(reportPath, stringifiedReportContent); + console.error( + `${baseMessage} Partial report (excluding context) available at: ${reportPath}`, + ); + } catch (minimalWriteError) { + console.error( + `${baseMessage} Failed to write even a minimal error report:`, + minimalWriteError, + ); + } + return; + } + + try { + await fs.writeFile(reportPath, stringifiedReportContent); + console.error(`${baseMessage} Full report available at: ${reportPath}`); + } catch (writeError) { + console.error( + `${baseMessage} Additionally, failed to write detailed error report:`, + writeError, + ); + // Log the original error as a fallback if report writing fails + console.error('Original error that triggered report generation:', error); + if (context) { + // Context was stringifiable, but writing the file failed. + // We already have stringifiedReportContent, but it might be too large for console. + // So, we try to log the original context object, and if that fails, its stringified version (truncated). + try { + console.error('Original context:', context); + } catch { + try { + console.error( + 'Original context (stringified, truncated):', + JSON.stringify(context).substring(0, 1000), + ); + } catch { + console.error('Original context could not be logged or stringified.'); + } + } + } + } +}