Skip to content

Commit c5613eb

Browse files
authored
fix(core): Improve safeJoin usage in console logging integration (#16658)
resolves #16657 ## Background In the `consoleLoggingIntegration` (which sends logs to Sentry from console calls), we use a utility called `safeJoin` to join together the console args. https://github.com/getsentry/sentry-javascript/blob/b94f65279c8341a7176fe68186feef58af57e2cb/packages/core/src/utils/string.ts#L68-L94 This utility calls `String(value)` to convert items to a string, which results in objects and arrays being turned into the dreaded `[Object Object]` or `[Array]`. https://github.com/getsentry/sentry-javascript/blob/b94f65279c8341a7176fe68186feef58af57e2cb/packages/core/src/utils/string.ts#L86 A user wrote in with feedback that this was annoying, and I agree! ## Solution Instead of using `String(value)` I chose to create a new helper that uses `JSON.stringify(normalize(X))`, which should result in the entire object or array being properly shown. This required me to grab `normalizeDepth` and `normalizeMaxBreadth` from the client options, but that feels fine because it's easily explainable to users. I added tests to validate this. ## Next Steps We should really refactor our overall `safeJoin` usage. This should be safe in breadcrumbs, but will be a breaking change in `capture-console` as it'll cause issue grouping changes if console calls are being captured as messages/errors.
1 parent 4396196 commit c5613eb

File tree

3 files changed

+62
-9
lines changed

3 files changed

+62
-9
lines changed

dev-packages/browser-integration-tests/suites/public-api/logger/integration/subject.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ console.warn('console.warn', 123, false);
66
console.error('console.error', 123, false);
77
console.assert(false, 'console.assert', 123, false);
88

9+
// Test object and array truncation
10+
console.log('Object:', { key: 'value', nested: { prop: 123 } });
11+
console.log('Array:', [1, 2, 3, 'string']);
12+
console.log('Mixed:', 'prefix', { obj: true }, [4, 5, 6], 'suffix');
13+
914
console.log('');
1015

1116
Sentry.flush();

dev-packages/browser-integration-tests/suites/public-api/logger/integration/test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ sentryTest('should capture console object calls', async ({ getLocalTestUrl, page
1818
expect(envelopeItems[0]).toEqual([
1919
{
2020
type: 'log',
21-
item_count: 8,
21+
item_count: 11,
2222
content_type: 'application/vnd.sentry.items.log+json',
2323
},
2424
{
@@ -107,6 +107,42 @@ sentryTest('should capture console object calls', async ({ getLocalTestUrl, page
107107
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
108108
},
109109
},
110+
{
111+
timestamp: expect.any(Number),
112+
level: 'info',
113+
severity_number: 10,
114+
trace_id: expect.any(String),
115+
body: 'Object: {"key":"value","nested":{"prop":123}}',
116+
attributes: {
117+
'sentry.origin': { value: 'auto.console.logging', type: 'string' },
118+
'sentry.sdk.name': { value: 'sentry.javascript.browser', type: 'string' },
119+
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
120+
},
121+
},
122+
{
123+
timestamp: expect.any(Number),
124+
level: 'info',
125+
severity_number: 10,
126+
trace_id: expect.any(String),
127+
body: 'Array: [1,2,3,"string"]',
128+
attributes: {
129+
'sentry.origin': { value: 'auto.console.logging', type: 'string' },
130+
'sentry.sdk.name': { value: 'sentry.javascript.browser', type: 'string' },
131+
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
132+
},
133+
},
134+
{
135+
timestamp: expect.any(Number),
136+
level: 'info',
137+
severity_number: 10,
138+
trace_id: expect.any(String),
139+
body: 'Mixed: prefix {"obj":true} [4,5,6] suffix',
140+
attributes: {
141+
'sentry.origin': { value: 'auto.console.logging', type: 'string' },
142+
'sentry.sdk.name': { value: 'sentry.javascript.browser', type: 'string' },
143+
'sentry.sdk.version': { value: expect.any(String), type: 'string' },
144+
},
145+
},
110146
{
111147
timestamp: expect.any(Number),
112148
level: 'info',

packages/core/src/logs/console-integration.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import { defineIntegration } from '../integration';
55
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
66
import type { ConsoleLevel } from '../types-hoist/instrument';
77
import type { IntegrationFn } from '../types-hoist/integration';
8+
import { isPrimitive } from '../utils/is';
89
import { CONSOLE_LEVELS, logger } from '../utils/logger';
9-
import { safeJoin } from '../utils/string';
10+
import { normalize } from '../utils/normalize';
1011
import { GLOBAL_OBJ } from '../utils/worldwide';
1112
import { _INTERNAL_captureLog } from './exports';
1213

@@ -32,7 +33,8 @@ const _consoleLoggingIntegration = ((options: Partial<CaptureConsoleOptions> = {
3233
return {
3334
name: INTEGRATION_NAME,
3435
setup(client) {
35-
if (!client.getOptions()._experiments?.enableLogs) {
36+
const { _experiments, normalizeDepth = 3, normalizeMaxBreadth = 1_000 } = client.getOptions();
37+
if (!_experiments?.enableLogs) {
3638
DEBUG_BUILD && logger.warn('`_experiments.enableLogs` is not enabled, ConsoleLogs integration disabled');
3739
return;
3840
}
@@ -45,17 +47,19 @@ const _consoleLoggingIntegration = ((options: Partial<CaptureConsoleOptions> = {
4547
if (level === 'assert') {
4648
if (!args[0]) {
4749
const followingArgs = args.slice(1);
48-
const message =
49-
followingArgs.length > 0 ? `Assertion failed: ${formatConsoleArgs(followingArgs)}` : 'Assertion failed';
50-
_INTERNAL_captureLog({ level: 'error', message, attributes: DEFAULT_ATTRIBUTES });
50+
const assertionMessage =
51+
followingArgs.length > 0
52+
? `Assertion failed: ${formatConsoleArgs(followingArgs, normalizeDepth, normalizeMaxBreadth)}`
53+
: 'Assertion failed';
54+
_INTERNAL_captureLog({ level: 'error', message: assertionMessage, attributes: DEFAULT_ATTRIBUTES });
5155
}
5256
return;
5357
}
5458

5559
const isLevelLog = level === 'log';
5660
_INTERNAL_captureLog({
5761
level: isLevelLog ? 'info' : level,
58-
message: formatConsoleArgs(args),
62+
message: formatConsoleArgs(args, normalizeDepth, normalizeMaxBreadth),
5963
severityNumber: isLevelLog ? 10 : undefined,
6064
attributes: DEFAULT_ATTRIBUTES,
6165
});
@@ -85,8 +89,16 @@ const _consoleLoggingIntegration = ((options: Partial<CaptureConsoleOptions> = {
8589
*/
8690
export const consoleLoggingIntegration = defineIntegration(_consoleLoggingIntegration);
8791

88-
function formatConsoleArgs(values: unknown[]): string {
92+
function formatConsoleArgs(values: unknown[], normalizeDepth: number, normalizeMaxBreadth: number): string {
8993
return 'util' in GLOBAL_OBJ && typeof (GLOBAL_OBJ as GlobalObjectWithUtil).util.format === 'function'
9094
? (GLOBAL_OBJ as GlobalObjectWithUtil).util.format(...values)
91-
: safeJoin(values, ' ');
95+
: safeJoinConsoleArgs(values, normalizeDepth, normalizeMaxBreadth);
96+
}
97+
98+
function safeJoinConsoleArgs(values: unknown[], normalizeDepth: number, normalizeMaxBreadth: number): string {
99+
return values
100+
.map(value =>
101+
isPrimitive(value) ? String(value) : JSON.stringify(normalize(value, normalizeDepth, normalizeMaxBreadth)),
102+
)
103+
.join(' ');
92104
}

0 commit comments

Comments
 (0)