Skip to content

Commit b816d42

Browse files
Copilotnickytonline
Copilot
and
nickytonline
committed
Implement structured logging with Pino
- Install pino and pino-pretty dependencies - Create logger utility with browser/server compatibility - Replace all console.* calls with structured logging - Add userId and context to API route logs - Update tests to not depend on console.* calls - Tests pass (same baseline as before) - Linting and build successful Co-authored-by: nickytonline <[email protected]>
1 parent 27e4857 commit b816d42

14 files changed

+5098
-5074
lines changed

‎package-lock.json‎

Lines changed: 4778 additions & 4979 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
"lucide-react": "^0.476.0",
4343
"mime": "^4.0.7",
4444
"openai": "^4.103.0",
45+
"pino": "^8.21.0",
4546
"react": "^19.0.0",
4647
"react-dom": "^19.0.0",
4748
"react-markdown": "^10.1.0",
@@ -79,6 +80,7 @@
7980
"eslint-plugin-react-hooks": "^5.2.0",
8081
"eslint-plugin-storybook": "^9.0.16",
8182
"jsdom": "^26.0.0",
83+
"pino-pretty": "^10.3.1",
8284
"playwright": "^1.53.2",
8385
"prettier": "^3.5.3",
8486
"storybook": "^9.0.16",

‎src/components/CodeInterpreterMessage.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import {
1515
createAnnotatedFileUrl,
1616
isImageFile,
1717
} from '@/lib/utils/code-interpreter'
18+
import { createLogger } from '@/lib/logger'
19+
20+
const log = createLogger('code-interpreter-message')
1821

1922
type CodeInterpreterStatus = 'writing' | 'executing' | 'completed' | 'failed'
2023

@@ -209,7 +212,14 @@ export function CodeInterpreterMessage({ args }: CodeInterpreterMessageProps) {
209212
className="max-w-full h-auto rounded-lg shadow-sm"
210213
style={{ maxHeight: '400px' }}
211214
onError={(e) => {
212-
console.error('Failed to load image:', e)
215+
log.error(
216+
{
217+
filename: file.filename,
218+
fileId: file.file_id,
219+
err: e,
220+
},
221+
'Failed to load image',
222+
)
213223
// Hide the image if it fails to load
214224
;(e.target as HTMLImageElement).style.display =
215225
'none'

‎src/components/ServerSelector.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import { Popover, PopoverContent, PopoverTrigger } from './ui/popover'
1515
import { ServerToggle } from './ServerToggle'
1616
import type { Server, Servers } from '../lib/schemas'
1717
import { useDisconnectServer } from '@/hooks/useDisconnectServer'
18+
import { createLogger } from '@/lib/logger'
19+
20+
const log = createLogger('server-selector')
1821

1922
const POMERIUM_ROUTES_ENDPOINT = '/.pomerium/mcp/routes'
2023
const POMERIUM_CONNECT_PATH = '/.pomerium/mcp/connect'
@@ -148,7 +151,13 @@ const ServerSelectionContent = ({
148151
try {
149152
await disconnectMutation.mutateAsync(serverId)
150153
} catch (err) {
151-
console.error('Failed to disconnect from server:', err)
154+
log.error(
155+
{
156+
serverId,
157+
err,
158+
},
159+
'Failed to disconnect from server',
160+
)
152161
}
153162
}
154163
const handleServerClick = (serverId: string) => {
@@ -291,7 +300,7 @@ export function ServerSelector({
291300
onServersChange(newServers)
292301
setIsLoading(false)
293302
} catch (err) {
294-
console.error('Failed to fetch servers:', err)
303+
log.error({ err }, 'Failed to fetch servers')
295304
setError(err instanceof Error ? err.message : 'Failed to fetch servers')
296305
setIsLoading(false)
297306
}

‎src/hooks/useDisconnectServer.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import {
44
disconnectRequestSchema,
55
disconnectResponseSchema,
66
} from '@/lib/schemas'
7+
import { createLogger } from '@/lib/logger'
8+
9+
const log = createLogger('use-disconnect-server')
710

811
const POMERIUM_DISCONNECT_ENDPOINT = '/.pomerium/mcp/routes/disconnect'
912

@@ -58,7 +61,7 @@ export function useDisconnectServer(
5861
onServersChange(updatedServers)
5962
},
6063
onError: (error) => {
61-
console.error('Failed to disconnect from server:', error)
64+
log.error({ err: error }, 'Failed to disconnect from server')
6265
},
6366
})
6467
}

‎src/hooks/useDisconnectServer.tsx.test.tsx‎

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type { Servers } from '@/lib/schemas'
55
import { createQueryClientTestWrapper } from '@/test/utils/react-query-test-utils'
66

77
describe('useDisconnectServer', () => {
8-
// No global console.error spy; will use local spies in specific tests
98
const serverId = 'https://test.example.com'
109
const servers: Servers = {
1110
[serverId]: {
@@ -71,9 +70,6 @@ describe('useDisconnectServer', () => {
7170
})
7271

7372
it('throws error if server is missing or does not need oauth', async () => {
74-
const consoleErrorSpy = vi
75-
.spyOn(console, 'error')
76-
.mockImplementation(() => {})
7773
const onServersChange = vi.fn()
7874
const { result } = renderHook(
7975
() => useDisconnectServer({}, onServersChange),
@@ -93,17 +89,9 @@ describe('useDisconnectServer', () => {
9389
expect(error).toBeInstanceOf(Error)
9490
expect((error as Error).message).toMatch(/Invalid server/)
9591
expect(onServersChange).not.toHaveBeenCalled()
96-
expect(consoleErrorSpy).toHaveBeenCalledWith(
97-
'Failed to disconnect from server:',
98-
expect.any(Error),
99-
)
100-
consoleErrorSpy.mockRestore()
10192
})
10293

10394
it('throws error if server does not need oauth', async () => {
104-
const consoleErrorSpy = vi
105-
.spyOn(console, 'error')
106-
.mockImplementation(() => {})
10795
const noOauthServers: Servers = {
10896
[serverId]: { ...servers[serverId], needs_oauth: false },
10997
}
@@ -125,22 +113,14 @@ describe('useDisconnectServer', () => {
125113
expect(error).toBeInstanceOf(Error)
126114
expect((error as Error).message).toMatch(/Invalid server/)
127115
expect(onServersChange).not.toHaveBeenCalled()
128-
expect(consoleErrorSpy).toHaveBeenCalledWith(
129-
'Failed to disconnect from server:',
130-
expect.any(Error),
131-
)
132-
consoleErrorSpy.mockRestore()
133116
})
134117

135-
it('handles fetch/network error and logs to console.error', async () => {
118+
it('handles fetch/network error', async () => {
136119
vi.stubGlobal(
137120
'fetch',
138121
vi.fn(() => Promise.resolve({ ok: false, status: 500 } as any)),
139122
)
140123
const onServersChange = vi.fn()
141-
const consoleErrorSpy = vi
142-
.spyOn(console, 'error')
143-
.mockImplementation(() => {})
144124
const { result } = renderHook(
145125
() => useDisconnectServer(servers, onServersChange),
146126
{
@@ -158,17 +138,9 @@ describe('useDisconnectServer', () => {
158138
expect(error).toBeInstanceOf(Error)
159139
expect((error as Error).message).toMatch(/Failed to disconnect/)
160140
expect(onServersChange).not.toHaveBeenCalled()
161-
expect(consoleErrorSpy).toHaveBeenCalledWith(
162-
'Failed to disconnect from server:',
163-
expect.any(Error),
164-
)
165-
consoleErrorSpy.mockRestore()
166141
})
167142

168143
it('handles invalid response schema', async () => {
169-
const consoleErrorSpy = vi
170-
.spyOn(console, 'error')
171-
.mockImplementation(() => {})
172144
vi.stubGlobal(
173145
'fetch',
174146
vi.fn(() =>
@@ -195,11 +167,6 @@ describe('useDisconnectServer', () => {
195167
}
196168
expect(error).toBeInstanceOf(Error)
197169
expect(onServersChange).not.toHaveBeenCalled()
198-
expect(consoleErrorSpy).toHaveBeenCalledWith(
199-
'Failed to disconnect from server:',
200-
expect.objectContaining({ name: 'ZodError', issues: expect.any(Array) }),
201-
)
202-
consoleErrorSpy.mockRestore()
203170
})
204171

205172
it('does not mutate the original servers object', async () => {

‎src/hooks/useLocalStorage.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import { useState } from 'react'
2+
import { createLogger } from '@/lib/logger'
3+
4+
const log = createLogger('use-local-storage')
25

36
export function useLocalStorage<T>(key: string, initialValue: T) {
47
// Get from local storage then
@@ -13,7 +16,13 @@ export function useLocalStorage<T>(key: string, initialValue: T) {
1316
const item = window.localStorage.getItem(key)
1417
return item ? JSON.parse(item) : initialValue
1518
} catch (error) {
16-
console.warn(`Error reading localStorage key "${key}":`, error)
19+
log.warn(
20+
{
21+
key,
22+
err: error,
23+
},
24+
'Error reading localStorage key',
25+
)
1726
return initialValue
1827
}
1928
}
@@ -38,7 +47,13 @@ export function useLocalStorage<T>(key: string, initialValue: T) {
3847
window.localStorage.setItem(key, JSON.stringify(valueToStore))
3948
}
4049
} catch (error) {
41-
console.warn(`Error setting localStorage key "${key}":`, error)
50+
log.warn(
51+
{
52+
key,
53+
err: error,
54+
},
55+
'Error setting localStorage key',
56+
)
4257
}
4358
}
4459

‎src/hooks/useStreamingChat.ts‎

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import { generateMessageId } from '../mcp/client'
33
import type { AnnotatedFile } from '@/lib/utils/code-interpreter'
44
import { stopStreamProcessing } from '@/lib/utils/streaming'
55
import { getTimestamp } from '@/lib/utils/date'
6+
import { createLogger } from '@/lib/logger'
7+
8+
const log = createLogger('use-streaming-chat')
69

710
export type AssistantStreamEvent = {
811
type: 'assistant'
@@ -249,7 +252,7 @@ export function useStreamingChat(): UseStreamingChatReturn {
249252
}, [])
250253

251254
const handleError = useCallback((error: Error) => {
252-
console.error('Chat error:', error)
255+
log.error({ err: error }, 'Chat error')
253256
setStreamBuffer((prev: Array<StreamEvent>) => [
254257
...prev,
255258
{
@@ -268,10 +271,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
268271
setRequestId(xRequestId)
269272

270273
if (!response.ok) {
271-
console.error(
272-
'Chat response error:',
273-
response.status,
274-
response.statusText,
274+
log.error(
275+
{
276+
requestId: xRequestId,
277+
status: response.status,
278+
statusText: response.statusText,
279+
},
280+
'Chat response error',
275281
)
276282
setStreamBuffer((prev: Array<StreamEvent>) => [
277283
...prev,
@@ -333,7 +339,7 @@ export function useStreamingChat(): UseStreamingChatReturn {
333339
},
334340
])
335341
} catch (e) {
336-
console.error('Failed to parse error data:', e)
342+
log.error({ err: e }, 'Failed to parse error data')
337343
setStreamBuffer((prev: Array<StreamEvent>) => [
338344
...prev,
339345
{
@@ -349,13 +355,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
349355
try {
350356
const toolStateStr = line.slice(2)
351357
if (!toolStateStr || !toolStateStr.trim()) {
352-
console.warn('Empty tool state string')
358+
log.warn('Empty tool state string')
353359
return
354360
}
355361

356362
const toolState = JSON.parse(toolStateStr)
357363
if (!toolState || typeof toolState !== 'object') {
358-
console.warn('Invalid tool state object')
364+
log.warn('Invalid tool state object')
359365
return
360366
}
361367
if (toolState.type === 'tool_call_completed') {
@@ -597,10 +603,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
597603
? JSON.parse(toolState.delta)
598604
: {}
599605
} catch (e) {
600-
console.error('Failed to parse delta:', {
601-
delta: toolState.delta,
602-
e,
603-
})
606+
log.error(
607+
{
608+
delta: toolState.delta,
609+
err: e,
610+
},
611+
'Failed to parse delta',
612+
)
604613
toolState.delta = {}
605614
}
606615
}
@@ -611,10 +620,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
611620
? JSON.parse(toolState.arguments)
612621
: {}
613622
} catch (e) {
614-
console.error('Failed to parse arguments:', {
615-
arguments: toolState.arguments,
616-
e,
617-
})
623+
log.error(
624+
{
625+
arguments: toolState.arguments,
626+
err: e,
627+
},
628+
'Failed to parse arguments',
629+
)
618630
toolState.arguments = {}
619631
}
620632

@@ -668,7 +680,7 @@ export function useStreamingChat(): UseStreamingChatReturn {
668680
return [...prev, toolEvent]
669681
})
670682
} catch (e) {
671-
console.error('Failed to parse tool state:', e)
683+
log.error({ err: e }, 'Failed to parse tool state')
672684
}
673685
}
674686

@@ -699,13 +711,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
699711
try {
700712
const textChunk = line.slice(2)
701713
if (!textChunk || !textChunk.trim()) {
702-
console.warn('Empty text chunk')
714+
log.warn('Empty text chunk')
703715
return
704716
}
705717

706718
const text = JSON.parse(textChunk)
707719
if (typeof text !== 'string') {
708-
console.warn('Text chunk is not a string:', text)
720+
log.warn({ text }, 'Text chunk is not a string')
709721
return
710722
}
711723

@@ -714,7 +726,7 @@ export function useStreamingChat(): UseStreamingChatReturn {
714726
}
715727
updateAssistantText(text, assistantId)
716728
} catch (e) {
717-
console.error('Failed to parse text chunk:', e)
729+
log.error({ err: e }, 'Failed to parse text chunk')
718730
}
719731
}
720732
}
@@ -758,7 +770,7 @@ export function useStreamingChat(): UseStreamingChatReturn {
758770
return
759771
}
760772

761-
console.error('Error reading stream chunk:', error)
773+
log.error({ err: error }, 'Error reading stream chunk')
762774
setStreamBuffer((prev: Array<StreamEvent>) => [
763775
...prev,
764776
{
@@ -780,13 +792,13 @@ export function useStreamingChat(): UseStreamingChatReturn {
780792

781793
const addUserMessage = useCallback((content: string) => {
782794
if (!content || typeof content !== 'string') {
783-
console.warn('addUserMessage: Invalid content provided', content)
795+
log.warn({ content }, 'addUserMessage: Invalid content provided')
784796
return
785797
}
786798

787799
const trimmedContent = content.trim()
788800
if (!trimmedContent) {
789-
console.warn('addUserMessage: Empty content after trimming')
801+
log.warn('addUserMessage: Empty content after trimming')
790802
return
791803
}
792804

0 commit comments

Comments
 (0)