Make errorReporting test windows compatible. (#4856)

This commit is contained in:
Tommaso Sciortino 2025-07-25 10:32:23 -07:00 committed by GitHub
parent e500eb5562
commit f379aa833f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 61 additions and 86 deletions

View File

@ -4,36 +4,36 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import fs from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path';
import { reportError } from './errorReporting.js';
// Use a type alias for SpyInstance as it's not directly exported
type SpyInstance = ReturnType<typeof vi.spyOn>;
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';
let testDir: string;
const MOCK_TIMESTAMP = '2025-01-01T00-00-00-000Z';
beforeEach(() => {
beforeEach(async () => {
// Create a temporary directory for logs
testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gemini-report-test-'));
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(() => {
afterEach(async () => {
vi.restoreAllMocks();
// Clean up the temporary directory
await fs.rm(testDir, { recursive: true, force: true });
});
const getExpectedReportPath = (type: string) =>
`${MOCK_TMP_DIR}/gemini-client-error-${type}-${MOCK_TIMESTAMP}.json`;
path.join(testDir, `gemini-client-error-${type}-${MOCK_TIMESTAMP}.json`);
it('should generate a report and log the path', async () => {
const error = new Error('Test error');
@ -43,22 +43,18 @@ describe('reportError', () => {
const type = 'test-type';
const expectedReportPath = getExpectedReportPath(type);
(fs.writeFile as Mock).mockResolvedValue(undefined);
await reportError(error, baseMessage, context, type, testDir);
await reportError(error, baseMessage, context, type);
// Verify the file was written
const reportContent = await fs.readFile(expectedReportPath, 'utf-8');
const parsedReport = JSON.parse(reportContent);
expect(os.tmpdir).toHaveBeenCalledTimes(1);
expect(fs.writeFile).toHaveBeenCalledWith(
expectedReportPath,
JSON.stringify(
{
error: { message: 'Test error', stack: error.stack },
expect(parsedReport).toEqual({
error: { message: 'Test error', stack: 'Test stack' },
context,
},
null,
2,
),
);
});
// Verify the console log
expect(consoleErrorSpy).toHaveBeenCalledWith(
`${baseMessage} Full report available at: ${expectedReportPath}`,
);
@ -70,19 +66,15 @@ describe('reportError', () => {
const type = 'general';
const expectedReportPath = getExpectedReportPath(type);
(fs.writeFile as Mock).mockResolvedValue(undefined);
await reportError(error, baseMessage);
await reportError(error, baseMessage, undefined, type, testDir);
expect(fs.writeFile).toHaveBeenCalledWith(
expectedReportPath,
JSON.stringify(
{
const reportContent = await fs.readFile(expectedReportPath, 'utf-8');
const parsedReport = JSON.parse(reportContent);
expect(parsedReport).toEqual({
error: { message: 'Test plain object error' },
},
null,
2,
),
);
});
expect(consoleErrorSpy).toHaveBeenCalledWith(
`${baseMessage} Full report available at: ${expectedReportPath}`,
);
@ -94,19 +86,15 @@ describe('reportError', () => {
const type = 'general';
const expectedReportPath = getExpectedReportPath(type);
(fs.writeFile as Mock).mockResolvedValue(undefined);
await reportError(error, baseMessage);
await reportError(error, baseMessage, undefined, type, testDir);
expect(fs.writeFile).toHaveBeenCalledWith(
expectedReportPath,
JSON.stringify(
{
const reportContent = await fs.readFile(expectedReportPath, 'utf-8');
const parsedReport = JSON.parse(reportContent);
expect(parsedReport).toEqual({
error: { message: 'Just a string error' },
},
null,
2,
),
);
});
expect(consoleErrorSpy).toHaveBeenCalledWith(
`${baseMessage} Full report available at: ${expectedReportPath}`,
);
@ -115,22 +103,15 @@ describe('reportError', () => {
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);
const nonExistentDir = path.join(testDir, 'non-existent-dir');
(fs.writeFile as Mock).mockRejectedValue(writeError);
await reportError(error, baseMessage, context, type, nonExistentDir);
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.any(Error), // The actual write error
);
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Original error that triggered report generation:',
@ -163,9 +144,7 @@ describe('reportError', () => {
return originalJsonStringify(value, replacer, space);
});
(fs.writeFile as Mock).mockResolvedValue(undefined); // Mock for the minimal report write
await reportError(error, baseMessage, context, type);
await reportError(error, baseMessage, context, type, testDir);
expect(consoleErrorSpy).toHaveBeenCalledWith(
`${baseMessage} Could not stringify report content (likely due to context):`,
@ -178,15 +157,14 @@ describe('reportError', () => {
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,
),
);
// Check that it writes a minimal report
const reportContent = await fs.readFile(expectedMinimalReportPath, 'utf-8');
const parsedReport = JSON.parse(reportContent);
expect(parsedReport).toEqual({
error: { message: error.message, stack: error.stack },
});
expect(consoleErrorSpy).toHaveBeenCalledWith(
`${baseMessage} Partial report (excluding context) available at: ${expectedMinimalReportPath}`,
);
@ -199,19 +177,15 @@ describe('reportError', () => {
const type = 'general';
const expectedReportPath = getExpectedReportPath(type);
(fs.writeFile as Mock).mockResolvedValue(undefined);
await reportError(error, baseMessage, undefined, type);
await reportError(error, baseMessage, undefined, type, testDir);
const reportContent = await fs.readFile(expectedReportPath, 'utf-8');
const parsedReport = JSON.parse(reportContent);
expect(parsedReport).toEqual({
error: { message: 'Error without context', stack: 'No context stack' },
});
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}`,
);

View File

@ -27,10 +27,11 @@ export async function reportError(
baseMessage: string,
context?: Content[] | Record<string, unknown> | unknown[],
type = 'general',
reportingDir = os.tmpdir(), // for testing
): Promise<void> {
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
const reportFileName = `gemini-client-error-${type}-${timestamp}.json`;
const reportPath = path.join(os.tmpdir(), reportFileName);
const reportPath = path.join(reportingDir, reportFileName);
let errorToReport: { message: string; stack?: string };
if (error instanceof Error) {