diff --git a/packages/server/utils/src/ap-logger.ts b/packages/server/utils/src/ap-logger.ts index d2224993c1f..84b0449ff25 100644 --- a/packages/server/utils/src/ap-logger.ts +++ b/packages/server/utils/src/ap-logger.ts @@ -29,13 +29,14 @@ function buildLogger(bindings: Record): ApLogger { }, info(...args: unknown[]) { try { - const { message, fields } = normalizePinoArgs(args) + const { message, fields, err } = normalizePinoArgsWithError(args) + const safeFields = err ? { error: stringifyError(err), ...fields } : fields const wide = wideEvent.current() if (wide) { - wide.info(message ?? 'log', { ...bindings, ...fields }) + wide.info(message ?? 'log', { ...bindings, ...safeFields }) } else { - log.info({ msg: message, ...bindings, ...fields }) + log.info({ msg: message, ...bindings, ...safeFields }) } } catch { @@ -44,13 +45,14 @@ function buildLogger(bindings: Record): ApLogger { }, warn(...args: unknown[]) { try { - const { message, fields } = normalizePinoArgs(args) + const { message, fields, err } = normalizePinoArgsWithError(args) + const safeFields = err ? { error: stringifyError(err), ...fields } : fields const wide = wideEvent.current() if (wide) { - wide.warn(message ?? 'log', { ...bindings, ...fields }) + wide.warn(message ?? 'log', { ...bindings, ...safeFields }) } else { - log.warn({ msg: message, ...bindings, ...fields }) + log.warn({ msg: message, ...bindings, ...safeFields }) } } catch { @@ -68,7 +70,7 @@ function buildLogger(bindings: Record): ApLogger { } else { if (err) { - log.error({ msg: message ?? err.message, error: `${err.message}\n${err.stack ?? ''}`, ...bindings, ...fields }) + log.error({ msg: message ?? err.message, error: stringifyError(err), ...bindings, ...fields }) } else { log.error({ msg: message, ...bindings, ...fields }) @@ -88,7 +90,7 @@ function buildLogger(bindings: Record): ApLogger { } else { if (err) { - log.error({ msg: message ?? err.message, error: `${err.message}\n${err.stack ?? ''}`, ...bindings, ...fields }) + log.error({ msg: message ?? err.message, error: stringifyError(err), ...bindings, ...fields }) } else { log.error({ msg: message, ...bindings, ...fields }) @@ -101,8 +103,9 @@ function buildLogger(bindings: Record): ApLogger { }, debug(...args: unknown[]) { try { - const { message, fields } = normalizePinoArgs(args) - log.debug({ msg: message, ...bindings, ...fields }) + const { message, fields, err } = normalizePinoArgsWithError(args) + const safeFields = err ? { error: stringifyError(err), ...fields } : fields + log.debug({ msg: message, ...bindings, ...safeFields }) } catch { // never throw @@ -110,8 +113,9 @@ function buildLogger(bindings: Record): ApLogger { }, trace(...args: unknown[]) { try { - const { message, fields } = normalizePinoArgs(args) - log.debug({ msg: message, ...bindings, ...fields }) + const { message, fields, err } = normalizePinoArgsWithError(args) + const safeFields = err ? { error: stringifyError(err), ...fields } : fields + log.debug({ msg: message, ...bindings, ...safeFields }) } catch { // never throw @@ -128,17 +132,8 @@ function isRecord(value: unknown): value is Record { return value !== null && typeof value === 'object' && !Array.isArray(value) } -function normalizePinoArgs(args: unknown[]): { message: string | undefined, fields: Record } { - const first = args[0] - if (typeof first === 'string') { - return { message: first, fields: {} } - } - if (isRecord(first)) { - const second = args[1] - const message = typeof second === 'string' ? second : undefined - return { message, fields: first } - } - return { message: String(first ?? ''), fields: {} } +function stringifyError(err: Error): string { + return `${err.message}\n${err.stack ?? ''}` } function normalizePinoArgsWithError(args: unknown[]): { message: string | undefined, fields: Record, err: Error | undefined } { diff --git a/packages/server/utils/test/ap-logger.test.ts b/packages/server/utils/test/ap-logger.test.ts index d4982cb1946..fea1963dcda 100644 --- a/packages/server/utils/test/ap-logger.test.ts +++ b/packages/server/utils/test/ap-logger.test.ts @@ -204,4 +204,58 @@ describe('apLogger', () => { apLogger.setCurrentLevel('info') // restore }) }) + + describe('Error fields are stringified at every level', () => { + function makeCyclicAxiosLikeError(): Error { + const err = new Error('Request failed with status code 521') + const request: Record = { host: 'console.activepieces.com' } + const response: Record = { status: 521, request } + request['res'] = response + Object.assign(err, { isAxiosError: true, code: 'ERR_BAD_RESPONSE', request, response }) + return err + } + + it('warn({ error: Error }) sends a string error field to the wide event, never the raw object', () => { + ambientState.active = true + const logger = apLogger.create({}) + logger.warn({ error: makeCyclicAxiosLikeError(), platform: { id: 'p1' } }, 'enrollment failed') + expect(spies.wideWarnSpy).toHaveBeenCalledOnce() + const [msg, ctx] = spies.wideWarnSpy.mock.calls[0] + expect(msg).toBe('enrollment failed') + expect(typeof ctx.error).toBe('string') + expect(ctx.error).toContain('Request failed with status code 521') + expect(ctx.platform).toEqual({ id: 'p1' }) + }) + + it('warn({ err: Error }) removes err and emits the canonical string error field', () => { + ambientState.active = true + const logger = apLogger.create({}) + logger.warn({ err: makeCyclicAxiosLikeError() }, 'failed') + const [, ctx] = spies.wideWarnSpy.mock.calls[0] + expect(typeof ctx.error).toBe('string') + expect(ctx.err).toBeUndefined() + }) + + it('info({ error: Error }) sends a string error field to the wide event', () => { + ambientState.active = true + const logger = apLogger.create({}) + logger.info({ error: makeCyclicAxiosLikeError() }, 'soft failure') + const [, ctx] = spies.wideInfoSpy.mock.calls[0] + expect(typeof ctx.error).toBe('string') + }) + + it('warn({ error: Error }) without ambient wide event logs a string error field', () => { + const logger = apLogger.create({}) + logger.warn({ error: makeCyclicAxiosLikeError() }, 'failed') + const arg = spies.logWarnSpy.mock.calls[0][0] + expect(typeof arg.error).toBe('string') + }) + + it('debug({ error: Error }) logs a string error field', () => { + const logger = apLogger.create({}) + logger.debug({ error: makeCyclicAxiosLikeError() }, 'debugging') + const arg = spies.logDebugSpy.mock.calls[0][0] + expect(typeof arg.error).toBe('string') + }) + }) })