Skip to content

Commit af88862

Browse files
authored
fix(logging): name the driver cause and redact bound params in logged errors (#8565)
* fix(logging): name the driver cause and redact bound params in logged errors * fix(logging): redact bound params in colorized, nested, and exported errors * fix(logging): report the cause of the error the line reports
1 parent 1ebdccd commit af88862

3 files changed

Lines changed: 259 additions & 47 deletions

File tree

‎apps/sim/lib/workflows/persistence/utils.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,10 @@ export async function loadDeployedWorkflowState(
295295
await resolveWorkspaceId(workflowId, providedWorkspaceId)
296296
)
297297
} catch (error) {
298-
logger.error(`Error loading deployed workflow state ${workflowId}:`, error)
298+
// An undeployed workflow is an outcome each caller handles, not a load failure.
299+
if (!(error instanceof NoActiveDeploymentError)) {
300+
logger.error(`Error loading deployed workflow state ${workflowId}:`, error)
301+
}
299302
throw error
300303
}
301304
}

‎packages/logger/src/index.test.ts‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,4 +188,141 @@ describe('Logger', () => {
188188
expect(parsed.metadataError).toBe(true)
189189
})
190190
})
191+
192+
describe('wrapped driver errors', () => {
193+
const createEnabledLogger = () =>
194+
new Logger('Test', { enabled: true, colorize: false, logLevel: LogLevel.DEBUG })
195+
196+
/**
197+
* Mirrors Drizzle's `DrizzleQueryError`: SQL plus bound values in the message, and `query`,
198+
* `params` and `cause` as own enumerable properties, wrapping the driver error.
199+
*/
200+
const queryError = (params: string) => {
201+
const cause = Object.assign(new Error('canceling statement due to statement timeout'), {
202+
name: 'PostgresError',
203+
code: '57014',
204+
detail: `Key (email)=(${params}) already exists.`,
205+
})
206+
const query = 'select "id" from "user_table_rows" where "table_id" = $1 limit $2'
207+
const error = Object.assign(new Error(`Failed query: ${query}\nparams: ${params}`), {
208+
query,
209+
params: params.split(','),
210+
cause,
211+
})
212+
error.name = 'DrizzleQueryError'
213+
return error
214+
}
215+
216+
const consoleOutput = () =>
217+
[...consoleLogSpy.mock.calls, ...consoleErrorSpy.mock.calls].flat().join(' ')
218+
219+
test('names the deepest cause and its code on the line', () => {
220+
createEnabledLogger().error('Failed to query rows:', { error: queryError('tbl_1,52') })
221+
222+
const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string)
223+
expect(parsed.errorCause).toBe('PostgresError: canceling statement due to statement timeout')
224+
expect(parsed.errorCode).toBe('57014')
225+
})
226+
227+
test('reports the cause of the same error the line reports when given two', () => {
228+
const conflict = Object.assign(new Error('duplicate key value'), {
229+
name: 'PostgresError',
230+
code: '23505',
231+
})
232+
const second = new Error('Failed query: insert into "t" values ($1)', { cause: conflict })
233+
234+
createEnabledLogger().error('Retry failed', queryError('tbl_1'), second)
235+
236+
const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string)
237+
expect(parsed.error).toBe(second.message)
238+
expect(parsed.errorCause).toBe('PostgresError: duplicate key value')
239+
expect(parsed.errorCode).toBe('23505')
240+
})
241+
242+
test.each([
243+
['an object field', (error: Error) => [{ error }]],
244+
['a bare argument', (error: Error) => [error]],
245+
])('keeps bound values out of the message and stack when passed as %s', (_, args) => {
246+
createEnabledLogger().error(
247+
'Failed to query rows:',
248+
...args(queryError('alice@example.com,52'))
249+
)
250+
251+
const line = consoleErrorSpy.mock.calls[0][0] as string
252+
expect(line).not.toContain('alice@example.com')
253+
const parsed = JSON.parse(line)
254+
expect(parsed.error).toContain('Failed query: select "id"')
255+
expect(parsed.error).toContain('params: [redacted]')
256+
expect(parsed.stack).toContain('params: [redacted]')
257+
expect(parsed.stack).toMatch(/\n\s+at /)
258+
})
259+
260+
test.each([
261+
['under another key', (error: Error) => ({ dbError: error })],
262+
['nested inside metadata', (error: Error) => ({ details: { attempt: 2, error } })],
263+
])('keeps bound values out of an error logged %s', (_, arg) => {
264+
createEnabledLogger().error('Insert failed', arg(queryError('alice@example.com')))
265+
266+
expect(consoleOutput()).not.toContain('alice@example.com')
267+
})
268+
269+
test.each([
270+
['an object field', (error: Error) => [{ error }]],
271+
['a bare argument', (error: Error) => [error]],
272+
['nested inside metadata', (error: Error) => [{ details: { error } }]],
273+
])('keeps bound values out of colorized output when passed as %s', (_, args) => {
274+
new Logger('Test', { enabled: true, colorize: true, logLevel: LogLevel.DEBUG }).error(
275+
'Failed to query rows:',
276+
...args(queryError('alice@example.com'))
277+
)
278+
279+
const output = consoleOutput()
280+
expect(output).toContain('params: [redacted]')
281+
expect(output).not.toContain('alice@example.com')
282+
})
283+
284+
test.each([
285+
['a bare argument', (error: Error) => [error]],
286+
['an object field', (error: Error) => [{ error }]],
287+
])('keeps bound values out of the exported log record when passed as %s', (_, args) => {
288+
const emit = vi.fn()
289+
const getLoggerSpy = vi
290+
.spyOn(logs, 'getLogger')
291+
.mockReturnValue({ emit, enabled: () => true })
292+
try {
293+
createEnabledLogger().error(
294+
'Failed to query rows:',
295+
...args(queryError('alice@example.com'))
296+
)
297+
298+
const { attributes } = emit.mock.calls[0][0]
299+
expect(JSON.stringify(attributes)).not.toContain('alice@example.com')
300+
expect(attributes['error.cause']).toBe(
301+
'PostgresError: canceling statement due to statement timeout'
302+
)
303+
} finally {
304+
getLoggerSpy.mockRestore()
305+
}
306+
})
307+
308+
test('emits a line for a nested error that references itself', () => {
309+
const error = Object.assign(new Error('self-referencing failure'), {
310+
context: {} as Record<string, unknown>,
311+
})
312+
error.context.error = error
313+
314+
expect(() => createEnabledLogger().error('Failed', { details: { error } })).not.toThrow()
315+
const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string)
316+
expect(parsed.details.error.message).toBe('self-referencing failure')
317+
expect(parsed.details.error.context.error).toBe('[Circular]')
318+
})
319+
320+
test('leaves an unwrapped error without cause fields', () => {
321+
createEnabledLogger().error('Request failed', { error: new Error('plain failure') })
322+
323+
const parsed = JSON.parse(consoleErrorSpy.mock.calls[0][0] as string)
324+
expect(parsed.error).toBe('plain failure')
325+
expect(parsed).not.toHaveProperty('errorCause')
326+
})
327+
})
191328
})

0 commit comments

Comments
 (0)