Skip to content

Commit 2a852c6

Browse files
adhityamamallanadhityamamallan
authored
Misc fixes to error logging (#834)
Remove next-logger and register console loggers directly in registerLoggers function Fix messageKey for log messages Fix errorKey to serialize errors correctly Rename all "cause" fields to "error" so that we can serialize backend errors correctly Add logs to listAllDomains for errors Add request URL to RequestError (and update tests)
1 parent 62bf40c commit 2a852c6

File tree

46 files changed

+232
-123
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+232
-123
lines changed

Diff for: ‎next-logger.config.js

-15
This file was deleted.

Diff for: ‎package-lock.json

-33
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎package.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"version": "4.0.0",
44
"private": true,
55
"scripts": {
6-
"dev": "next dev -p ${CADENCE_WEB_PORT} | pino-pretty",
6+
"dev": "next dev -p ${CADENCE_WEB_PORT} | pino-pretty --messageKey message",
77
"build": "NODE_ENV=production && next build",
88
"start": "next start -p ${CADENCE_WEB_PORT}",
99
"lint": "next lint",
@@ -34,7 +34,6 @@
3434
"lodash": "^4.17.21",
3535
"lossless-json": "^4.0.2",
3636
"next": "14.2.16",
37-
"next-logger": "^5.0.0",
3837
"pino": "^9.3.2",
3938
"query-string": "^9.0.0",
4039
"react": "^18.2.0",

Diff for: ‎src/instrumentation.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export async function register() {
1313
// by "Cannot set property message of [object Object] which has only a getter"
1414
logger.error({
1515
message: 'Failed to load configs',
16-
cause: String(e),
16+
error: e,
1717
});
1818
process.exit(1); // use process.exit to exit without an extra error log from instrumentation
1919
}

Diff for: ‎src/route-handlers/cancel-workflow/cancel-workflow.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export async function cancelWorkflow(
4545
return NextResponse.json({} satisfies CancelWorkflowResponse);
4646
} catch (e) {
4747
logger.error<RouteHandlerErrorPayload>(
48-
{ requestParams: decodedParams, cause: e },
48+
{ requestParams: decodedParams, error: e },
4949
'Error cancelling workflow'
5050
);
5151

Diff for: ‎src/route-handlers/describe-cluster/describe-cluster.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export async function describeCluster(
4747
return NextResponse.json(sanitizedRes);
4848
} catch (e) {
4949
logger.error<RouteHandlerErrorPayload>(
50-
{ requestParams: decodedParams, cause: e },
50+
{ requestParams: decodedParams, error: e },
5151
'Error fetching cluster info'
5252
);
5353

Diff for: ‎src/route-handlers/describe-domain/describe-domain.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export async function describeDomain(
2525
return NextResponse.json(res.domain);
2626
} catch (e) {
2727
logger.error<RouteHandlerErrorPayload>(
28-
{ requestParams: decodedParams, cause: e },
28+
{ requestParams: decodedParams, error: e },
2929
'Error fetching domain info'
3030
);
3131

Diff for: ‎src/route-handlers/describe-task-list/describe-task-list.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export async function describeTaskList(
4646
return NextResponse.json({ taskList });
4747
} catch (e) {
4848
logger.error<RouteHandlerErrorPayload>(
49-
{ requestParams: decodedParams, cause: e },
49+
{ requestParams: decodedParams, error: e },
5050
'Error fetching task list'
5151
);
5252

Diff for: ‎src/route-handlers/describe-workflow/describe-workflow.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export default async function describeWorkflow(
140140
);
141141
} else {
142142
logger.error<RouteHandlerErrorPayload>(
143-
{ requestParams: decodedParams, cause: e },
143+
{ requestParams: decodedParams, error: e },
144144
'Error fetching workflow execution info'
145145
);
146146

Diff for: ‎src/route-handlers/fetch-workflow-query-types/fetch-workflow-query-types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export default async function fetchWorkflowQueryTypes(
4444
}
4545

4646
logger.error<RouteHandlerErrorPayload>(
47-
{ requestParams: decodedParams, cause: e },
47+
{ requestParams: decodedParams, error: e },
4848
'Error querying workflow for query types'
4949
);
5050

Diff for: ‎src/route-handlers/get-workflow-history/get-workflow-history.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ export default async function getWorkflowHistory(
6363
}
6464

6565
logger.error<RouteHandlerErrorPayload>(
66-
{ requestParams: decodedParams, cause: e },
66+
{ requestParams: decodedParams, error: e },
6767
'Error fetching workflow history'
6868
);
6969

Diff for: ‎src/route-handlers/list-workflows-basic/list-workflows-basic.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export async function listWorkflowsBasic(
8484
return NextResponse.json(response);
8585
} catch (e) {
8686
logger.error<RouteHandlerErrorPayload>(
87-
{ requestParams: decodedParams, queryParams, cause: e },
87+
{ requestParams: decodedParams, queryParams, error: e },
8888
'Error fetching workflows' +
8989
(e instanceof GRPCError ? ': ' + e.message : '')
9090
);

Diff for: ‎src/route-handlers/list-workflows/list-workflows.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export async function listWorkflows(
6868
return NextResponse.json(response);
6969
} catch (e) {
7070
logger.error<RouteHandlerErrorPayload>(
71-
{ requestParams: decodedParams, queryParams, cause: e },
71+
{ requestParams: decodedParams, queryParams, error: e },
7272
'Error fetching workflows' +
7373
(e instanceof GRPCError ? ': ' + e.message : '')
7474
);

Diff for: ‎src/route-handlers/log-to-server/log-to-server.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export async function logToServer(request: NextRequest) {
99
const { data, error } = logToServerPayloadSchema.safeParse(requestBodyJSON);
1010
if (error) {
1111
logger.error<RouteHandlerErrorPayload>(
12-
{ cause: error },
12+
{ error: error },
1313
'Failed to parse log from browser'
1414
);
1515

Diff for: ‎src/route-handlers/query-workflow/query-workflow.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export async function queryWorkflow(
5858
} satisfies QueryWorkflowResponse);
5959
} catch (e) {
6060
logger.error<RouteHandlerErrorPayload>(
61-
{ requestParams: decodedParams, cause: e },
61+
{ requestParams: decodedParams, error: e },
6262
'Error querying workflow'
6363
);
6464

Diff for: ‎src/route-handlers/terminate-workflow/terminate-workflow.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export async function terminateWorkflow(
4545
return NextResponse.json({} satisfies TerminateWorkflowResponse);
4646
} catch (e) {
4747
logger.error<RouteHandlerErrorPayload>(
48-
{ requestParams: decodedParams, cause: e },
48+
{ requestParams: decodedParams, error: e },
4949
'Error terminating workflow'
5050
);
5151

Diff for: ‎src/route-handlers/update-domain/update-domain.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export async function updateDomain(
5353
return NextResponse.json(res.domain);
5454
} catch (e) {
5555
logger.error<RouteHandlerErrorPayload>(
56-
{ requestParams: decodedParams, cause: e },
56+
{ requestParams: decodedParams, error: e },
5757
'Error updating domain information'
5858
);
5959

Diff for: ‎src/utils/data-formatters/format-pending-workflow-history-event/__tests__/index.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe('formatWorkflowHistoryEvent', () => {
2828
})
2929
).toBe(null);
3030
expect(logger.warn).toHaveBeenCalledWith(
31-
{ cause: expect.any(ZodError) },
31+
{ error: expect.any(ZodError) },
3232
'Failed to format workflow pending event'
3333
);
3434
});

Diff for: ‎src/utils/data-formatters/format-pending-workflow-history-event/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export default function formatPendingWorkflowHistoryEvent(
1313
if (schema) {
1414
const { data, error } = schema.safeParse(event);
1515
if (error) {
16-
logger.warn({ cause: error }, 'Failed to format workflow pending event');
16+
logger.warn({ error: error }, 'Failed to format workflow pending event');
1717
return null;
1818
}
1919
return data ?? null;

Diff for: ‎src/utils/data-formatters/format-workflow-history-event/__tests__/index.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('formatWorkflowHistoryEvent', () => {
2121
})
2222
).toBe(null);
2323
expect(logger.warn).toHaveBeenCalledWith(
24-
{ cause: expect.any(ZodError) },
24+
{ error: expect.any(ZodError) },
2525
'Failed to format workflow event'
2626
);
2727
});

Diff for: ‎src/utils/data-formatters/format-workflow-history-event/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export default function formatWorkflowHistoryEvent(
1313
if (schema) {
1414
const { data, error } = schema.safeParse(event);
1515
if (error) {
16-
logger.warn({ cause: error }, 'Failed to format workflow event');
16+
logger.warn({ error: error }, 'Failed to format workflow event');
1717
return null;
1818
}
1919
return data ?? null;

Diff for: ‎src/utils/logger/console/__tests__/register-console-logger.node.ts

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { type Logger } from '../..';
2+
import registerConsoleLogger from '../register-console-logger';
3+
import { NEXTJS_ERROR_PREFIX } from '../register-console-logger.constants';
4+
import { type ConsoleLogLevel } from '../register-console-logger.types';
5+
6+
describe(registerConsoleLogger.name, () => {
7+
it('should parse a regular console log correctly', () => {
8+
const { log, mockLogger } = setup('info');
9+
log(
10+
' \u001b[32m\u001b[1m✓\u001b[22m\u001b[39m',
11+
'Test message',
12+
{
13+
key: 'value',
14+
},
15+
{ key: 'another-value' }
16+
);
17+
18+
expect(mockLogger.info).toHaveBeenCalledWith(
19+
{
20+
data: [{ key: 'value' }, { key: 'another-value' }],
21+
},
22+
'✓ Test message'
23+
);
24+
});
25+
26+
it('should parse an error log correctly', () => {
27+
const { log, mockLogger } = setup('error');
28+
log(NEXTJS_ERROR_PREFIX, new Error('Something went wrong'));
29+
30+
expect(mockLogger.error).toHaveBeenCalledWith(
31+
{ errors: [new Error('Something went wrong')] },
32+
'Something went wrong'
33+
);
34+
});
35+
36+
it('should parse errors correctly if multiple are logged', () => {
37+
const { log, mockLogger } = setup('error');
38+
log(
39+
NEXTJS_ERROR_PREFIX,
40+
new Error('Something went wrong'),
41+
new Error('Something else went wrong'),
42+
{ key: 'value' }
43+
);
44+
45+
expect(mockLogger.error).toHaveBeenCalledWith(
46+
{
47+
data: [{ key: 'value' }],
48+
errors: [
49+
new Error('Something went wrong'),
50+
new Error('Something else went wrong'),
51+
],
52+
},
53+
'Something went wrong'
54+
);
55+
});
56+
});
57+
58+
function setup(level: ConsoleLogLevel) {
59+
const mockLogger = {
60+
debug: jest.fn(),
61+
info: jest.fn(),
62+
warn: jest.fn(),
63+
error: jest.fn(),
64+
} as unknown as Logger;
65+
66+
return {
67+
log: registerConsoleLogger(mockLogger, level),
68+
mockLogger,
69+
};
70+
}

Diff for: ‎src/utils/logger/console/helpers/strip-escapes-from-next-log.ts

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function stripEscapesFromNextLog(str: string): string {
2+
// This regex matches all ANSI escape sequences that next.js likes to put in the console logs
3+
return str.replace(
4+
/[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g,
5+
''
6+
);
7+
}

Diff for: ‎src/utils/logger/console/register-console-logger.constants.ts

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { type LogLevel } from '..';
2+
3+
import { type ConsoleLogLevel } from './register-console-logger.types';
4+
5+
export const NEXTJS_ERROR_PREFIX = '⨯';
6+
7+
export const CONSOLE_LOG_LEVEL_TO_LOG_LEVEL_MAP: Record<
8+
ConsoleLogLevel,
9+
LogLevel
10+
> = {
11+
error: 'error',
12+
warn: 'warn',
13+
info: 'info',
14+
log: 'info',
15+
debug: 'debug',
16+
};

Diff for: ‎src/utils/logger/console/register-console-logger.ts

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { isString } from 'lodash';
2+
3+
import { type Logger } from '..';
4+
5+
import stripEscapesFromNextLog from './helpers/strip-escapes-from-next-log';
6+
import {
7+
CONSOLE_LOG_LEVEL_TO_LOG_LEVEL_MAP,
8+
NEXTJS_ERROR_PREFIX,
9+
} from './register-console-logger.constants';
10+
import { type ConsoleLogLevel } from './register-console-logger.types';
11+
12+
/**
13+
* Registers a custom logger to override the default console logging used by Next.js.
14+
* See: https://github.com/vercel/next.js/discussions/63787#discussioncomment-11978877
15+
*/
16+
export default function registerConsoleLogger(
17+
logger: Logger,
18+
consoleLogLevel: ConsoleLogLevel
19+
) {
20+
const level = CONSOLE_LOG_LEVEL_TO_LOG_LEVEL_MAP[consoleLogLevel];
21+
22+
return (...args: unknown[]) => {
23+
const messages = args.filter(isString);
24+
const parsedMessage = stripEscapesFromNextLog(messages.join(' ')).trim();
25+
26+
const errors = args.filter((arg) => arg instanceof Error);
27+
28+
const restData = args.filter(
29+
(arg) => !isString(arg) && !(arg instanceof Error)
30+
);
31+
32+
logger[level](
33+
{
34+
...(restData.length > 0 ? { data: restData } : {}),
35+
...(errors.length > 0 ? { errors } : {}),
36+
},
37+
errors[0] instanceof Error &&
38+
(!parsedMessage || parsedMessage === NEXTJS_ERROR_PREFIX)
39+
? errors[0].message
40+
: parsedMessage
41+
);
42+
};
43+
}

Diff for: ‎src/utils/logger/console/register-console-logger.types.ts

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export type ConsoleLogLevel = 'error' | 'info' | 'warn' | 'debug' | 'log';

Diff for: ‎src/utils/logger/index.ts

-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Logger
22
import logger from './pino/pino';
3-
export { default as getNextLogger } from './pino/pino-next-logger';
43

54
// Register logger
65
export { registerLoggers } from './logger-register';

0 commit comments

Comments
 (0)