Don't throw 400 errors for invalid event payloads (#61)

This commit is contained in:
Chris Estreich 2025-06-03 13:25:22 -07:00 committed by GitHub
parent b6af87b9fd
commit f97ca9cda8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 431 additions and 44 deletions

View file

@ -137,8 +137,7 @@
"typescript": "^5.8.3",
"typescript-eslint": "^8.33.0",
"vite-tsconfig-paths": "^5.1.4",
"vitest": "^3.1.4",
"vitest-fail-on-console": "^0.7.1"
"vitest": "^3.1.4"
},
"lint-staged": {
"*.{js,jsx,ts,tsx,json,css,md}": [

21
pnpm-lock.yaml generated
View file

@ -336,9 +336,6 @@ importers:
vitest:
specifier: ^3.1.4
version: 3.1.4(@types/debug@4.1.12)(@types/node@22.15.27)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0)
vitest-fail-on-console:
specifier: ^0.7.1
version: 0.7.1(vite@6.3.5(@types/node@22.15.27)(jiti@2.4.2)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0))(vitest@3.1.4(@types/debug@4.1.12)(@types/node@22.15.27)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0))
packages:
@ -4354,10 +4351,6 @@ packages:
resolution: {integrity: sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA==}
engines: {node: '>=10'}
chalk@5.3.0:
resolution: {integrity: sha512-dLitG79d+GV1Nb/VYcCDFivJeK1hiukt9QjRNVOsUtTy1rR1YJsmpGGTZ3qJos+uw7WmWF4wUwBd9jxjocFC2w==}
engines: {node: ^12.17.0 || ^14.13 || >=16.0.0}
chalk@5.4.1:
resolution: {integrity: sha512-zgVZuo2WcZgfUEmsn6eO3kINexW8RAE4maiQ8QNs8CtpPCSyMiYsULR3HQYkm3w8FIA3SberyMJMSldGsW+U3w==}
engines: {node: ^12.17.0 || ^14.13 || >=16.0.0}
@ -8790,12 +8783,6 @@ packages:
yaml:
optional: true
vitest-fail-on-console@0.7.1:
resolution: {integrity: sha512-/PjuonFu7CwUVrKaiQPIGXOtiEv2/Gz3o8MbLmovX9TGDxoRCctRC8CA9zJMRUd6AvwGu/V5a3znObTmlPNTgw==}
peerDependencies:
vite: '>=4.5.2'
vitest: '>=0.26.2'
vitest@3.1.4:
resolution: {integrity: sha512-Ta56rT7uWxCSJXlBtKgIlApJnT6e6IGmTYxYcmxjJ4ujuZDI59GUQgVDObXXJujOmPDBYXHK1qmaGtneu6TNIQ==}
engines: {node: ^18.0.0 || ^20.0.0 || >=22.0.0}
@ -13594,8 +13581,6 @@ snapshots:
ansi-styles: 4.3.0
supports-color: 7.2.0
chalk@5.3.0: {}
chalk@5.4.1: {}
char-regex@1.0.2: {}
@ -18765,12 +18750,6 @@ snapshots:
tsx: 4.19.4
yaml: 2.8.0
vitest-fail-on-console@0.7.1(vite@6.3.5(@types/node@22.15.27)(jiti@2.4.2)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0))(vitest@3.1.4(@types/debug@4.1.12)(@types/node@22.15.27)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0)):
dependencies:
chalk: 5.3.0
vite: 6.3.5(@types/node@22.15.27)(jiti@2.4.2)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0)
vitest: 3.1.4(@types/debug@4.1.12)(@types/node@22.15.27)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0)
vitest@3.1.4(@types/debug@4.1.12)(@types/node@22.15.27)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(terser@5.40.0)(tsx@4.19.4)(yaml@2.8.0):
dependencies:
'@vitest/expect': 3.1.4

View file

@ -0,0 +1,412 @@
// pnpm test src/app/api/events/__tests__/route.test.ts
import { NextRequest } from 'next/server';
import { auth } from '@clerk/nextjs/server';
import { captureEvent } from '@/actions/analytics';
import { POST } from '../route';
vi.mock('@clerk/nextjs/server', () => ({
auth: vi.fn(),
}));
vi.mock('@/actions/analytics', () => ({
captureEvent: vi.fn(),
}));
const mockAuth = vi.mocked(auth);
const mockCaptureEvent = vi.mocked(captureEvent);
describe('/api/events POST', () => {
describe('authentication', () => {
it('should return 401 when user is not authenticated', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockAuth.mockResolvedValue({ userId: null, orgId: null } as any);
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify({ type: 'test-event' }),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(401);
expect(data).toEqual({ error: 'Unauthorized: User required' });
});
it('should return 401 when organization is not provided', async () => {
mockAuth.mockResolvedValue({
userId: 'test-user-id',
orgId: null,
} as any); // eslint-disable-line @typescript-eslint/no-explicit-any
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify({ type: 'test-event' }),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(401);
expect(data).toEqual({ error: 'Unauthorized: Organization required' });
});
});
describe('schema validation', () => {
beforeEach(() => {
// Set up default successful auth and captureEvent.
mockAuth.mockResolvedValue({
userId: 'test-user-id',
orgId: 'test-org-id',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
mockCaptureEvent.mockResolvedValue(undefined);
});
it('should handle valid telemetry events successfully', async () => {
const validEvent = {
type: 'Task Created',
properties: {
appName: 'test-app',
appVersion: '1.0.0',
vscodeVersion: '1.80.0',
platform: 'darwin',
editorName: 'vscode',
language: 'typescript',
mode: 'code',
taskId: 'task-123',
apiProvider: 'anthropic',
modelId: 'claude-3-sonnet',
},
};
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(validEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
expect(mockCaptureEvent).toHaveBeenCalledWith({
id: expect.any(String),
orgId: 'test-org-id',
userId: 'test-user-id',
timestamp: expect.any(Number),
event: validEvent,
});
});
it('should handle LLM completion events successfully', async () => {
const validLLMEvent = {
type: 'LLM Completion',
properties: {
appName: 'test-app',
appVersion: '1.0.0',
vscodeVersion: '1.80.0',
platform: 'darwin',
editorName: 'vscode',
language: 'typescript',
mode: 'code',
inputTokens: 100,
outputTokens: 50,
cost: 0.01,
},
};
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(validLLMEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
expect(mockCaptureEvent).toHaveBeenCalledWith({
id: expect.any(String),
orgId: 'test-org-id',
userId: 'test-user-id',
timestamp: expect.any(Number),
event: validLLMEvent,
});
});
it('should handle schema validation errors gracefully without failing', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Invalid event - wrong type name.
const invalidEvent = {
type: 'Invalid Event Type',
properties: {
appName: 'test-app',
invalidProperty: 'should not be here',
},
};
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(invalidEvent),
});
const response = await POST(request);
const data = await response.json();
// Should still succeed with a 200 response.
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
// Should log the validation error.
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining('Invalid telemetry event:'),
);
// Should still attempt to capture the raw payload.
expect(mockCaptureEvent).toHaveBeenCalledWith({
id: expect.any(String),
orgId: 'test-org-id',
userId: 'test-user-id',
timestamp: expect.any(Number),
event: invalidEvent, // Raw payload used instead of validated data
});
consoleErrorSpy.mockRestore();
});
it('should handle missing required properties gracefully', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Invalid event - missing required properties for LLM Completion.
const invalidLLMEvent = {
type: 'LLM Completion',
properties: {
appName: 'test-app',
// Missing required inputTokens, outputTokens, etc.
},
};
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(invalidLLMEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
expect(consoleErrorSpy).toHaveBeenCalled();
expect(mockCaptureEvent).toHaveBeenCalledWith(
expect.objectContaining({
event: invalidLLMEvent,
}),
);
consoleErrorSpy.mockRestore();
});
it('should continue processing even with completely malformed data', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const malformedEvent = {
someRandomField: 'random value',
anotherField: 123,
nested: { data: 'here' },
};
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(malformedEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
expect(consoleErrorSpy).toHaveBeenCalled();
expect(mockCaptureEvent).toHaveBeenCalledWith(
expect.objectContaining({ event: malformedEvent }),
);
consoleErrorSpy.mockRestore();
});
});
describe('error handling', () => {
beforeEach(() => {
mockAuth.mockResolvedValue({
userId: 'test-user-id',
orgId: 'test-org-id',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
});
it('should return 500 when captureEvent fails', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const validEvent = {
type: 'Task Created',
properties: {
appName: 'test-app',
appVersion: '1.0.0',
vscodeVersion: '1.80.0',
platform: 'darwin',
editorName: 'vscode',
language: 'typescript',
mode: 'code',
},
};
const captureError = new Error('Database connection failed');
mockCaptureEvent.mockRejectedValue(captureError);
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(validEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(500);
expect(data).toEqual({
success: false,
error: 'Database connection failed',
});
expect(consoleErrorSpy).toHaveBeenCalledWith(captureError);
consoleErrorSpy.mockRestore();
});
it('should handle non-Error exceptions in captureEvent', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const validEvent = {
type: 'Task Created',
properties: {
appName: 'test-app',
appVersion: '1.0.0',
vscodeVersion: '1.80.0',
platform: 'darwin',
editorName: 'vscode',
language: 'typescript',
mode: 'code',
},
};
mockCaptureEvent.mockRejectedValue('String error');
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(validEvent),
});
const response = await POST(request);
const data = await response.json();
expect(response.status).toBe(500);
expect(data).toEqual({
success: false,
error: 'Unknown error',
});
consoleErrorSpy.mockRestore();
});
});
describe('integration scenarios', () => {
beforeEach(() => {
mockAuth.mockResolvedValue({
userId: 'test-user-id',
orgId: 'test-org-id',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
});
it('should handle schema validation failure followed by captureEvent success', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const invalidEvent = { invalidField: 'test' };
mockCaptureEvent.mockResolvedValue(undefined);
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(invalidEvent),
});
const response = await POST(request);
const data = await response.json();
// Should succeed despite validation failure.
expect(response.status).toBe(200);
expect(data).toEqual({ success: true, id: expect.any(String) });
// Should log validation error but continue.
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining('Invalid telemetry event:'),
);
// Should attempt to capture raw event.
expect(mockCaptureEvent).toHaveBeenCalledWith(
expect.objectContaining({
event: invalidEvent,
}),
);
consoleErrorSpy.mockRestore();
});
it('should handle schema validation failure followed by captureEvent failure', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const invalidEvent = { invalidField: 'test' };
mockCaptureEvent.mockRejectedValue(new Error('Capture failed'));
const request = new NextRequest('http://localhost/api/events', {
method: 'POST',
body: JSON.stringify(invalidEvent),
});
const response = await POST(request);
const data = await response.json();
// Should fail with 500 due to captureEvent failure.
expect(response.status).toBe(500);
expect(data).toEqual({
success: false,
error: 'Capture failed',
});
// Should log both validation error and capture error.
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining('Invalid telemetry event:'),
);
expect(consoleErrorSpy).toHaveBeenCalledWith(expect.any(Error));
consoleErrorSpy.mockRestore();
});
});
});

View file

@ -2,7 +2,10 @@ import { auth } from '@clerk/nextjs/server';
import { NextRequest, NextResponse } from 'next/server';
import { v4 as uuidv4 } from 'uuid';
import { rooCodeTelemetryEventSchema } from '@roo-code/types';
import {
type RooCodeTelemetryEvent,
rooCodeTelemetryEventSchema,
} from '@roo-code/types';
import { captureEvent } from '@/actions/analytics';
@ -25,19 +28,24 @@ export async function POST(request: NextRequest) {
const id = uuidv4();
const timestamp = Math.round(Date.now() / 1000);
const result = rooCodeTelemetryEventSchema.safeParse(await request.json());
const payload = await request.json();
const result = rooCodeTelemetryEventSchema.safeParse(payload);
if (!result.success) {
console.error(result.error);
let event: RooCodeTelemetryEvent;
return NextResponse.json(
{ success: false, error: result.error.message },
{ status: 400 },
);
if (result.success) {
event = result.data;
} else {
// If the event is invalid, log the error and try to insert the raw payload
// (which may fail). Some client don't send some newly required fields, but
// not all of those fields are required for a successful insert.
// Once Sentry is enabled, we can log the error there instead.
event = payload;
console.error(`Invalid telemetry event: ${result.error}`);
}
try {
await captureEvent({ id, orgId, userId, timestamp, event: result.data });
await captureEvent({ id, orgId, userId, timestamp, event });
} catch (error) {
console.error(error);

View file

@ -1,4 +1,4 @@
// npx vitest run src/hooks/__tests__/useAuthState.test.ts
// pnpm test src/hooks/__tests__/useAuthState.test.ts
import { renderHook, act } from '@testing-library/react';
import { useSessionStorage, useMount } from 'react-use';
@ -32,7 +32,6 @@ const mockedUseSearchParams = vi.mocked(useSearchParams);
describe('useAuthState', () => {
beforeEach(() => {
vi.clearAllMocks();
mockSearchParams.clear();
mockedUseSessionStorage.mockImplementation(
@ -216,7 +215,6 @@ describe('useAuthState', () => {
describe('useSetAuthState', () => {
beforeEach(() => {
vi.clearAllMocks();
mockSearchParams.clear();
mockedUseSessionStorage.mockImplementation(

View file

@ -1,13 +1,4 @@
import '@testing-library/jest-dom/vitest';
import failOnConsole from 'vitest-fail-on-console';
failOnConsole({
shouldFailOnDebug: true,
shouldFailOnError: true,
shouldFailOnInfo: true,
shouldFailOnLog: true,
shouldFailOnWarn: true,
});
global.ResizeObserver = vi.fn().mockImplementation(() => ({
observe: vi.fn(),