Skip to content

Commit feca47c

Browse files
mydeamydea
committed
fix(utils): Keep logger on carrier
Today, if you have a pluggable integration, it will incorrectly use it's own logger, which will generally be disabled. This is because we inline all the logger code into the bundle, but the logger is never enabled.
1 parent b7d1544 commit feca47c

File tree

13 files changed

+238
-86
lines changed

13 files changed

+238
-86
lines changed

‎dev-packages/browser-integration-tests/suites/feedback/attachTo/init.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import * as Sentry from '@sentry/browser';
2+
// Import this separately so that generatePlugin can handle it for CDN scenarios
3+
import { feedbackIntegration } from '@sentry/browser';
4+
5+
const feedback = feedbackIntegration({
6+
autoInject: false,
7+
});
8+
9+
window.Sentry = Sentry;
10+
window.feedback = feedback;
11+
12+
13+
Sentry.init({
14+
dsn: 'https://[email protected]/1337',
15+
integrations: [
16+
feedback,
17+
],
18+
});
19+
20+
feedback.attachTo('#custom-feedback-buttom');

‎dev-packages/browser-integration-tests/suites/feedback/attachTo/template.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button type="button" id="custom-feedback-buttom">Show feedback!</button>
8+
</body>
9+
</html>

‎dev-packages/browser-integration-tests/suites/feedback/attachTo/test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { TEST_HOST, sentryTest } from '../../../utils/fixtures';
4+
import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../utils/helpers';
5+
6+
sentryTest('should capture feedback with custom button', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipFeedbackTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const feedbackRequestPromise = page.waitForResponse(res => {
12+
const req = res.request();
13+
14+
const postData = req.postData();
15+
if (!postData) {
16+
return false;
17+
}
18+
19+
try {
20+
return getEnvelopeType(req) === 'feedback';
21+
} catch (err) {
22+
return false;
23+
}
24+
});
25+
26+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
27+
return route.fulfill({
28+
status: 200,
29+
contentType: 'application/json',
30+
body: JSON.stringify({ id: 'test-id' }),
31+
});
32+
});
33+
34+
const url = await getLocalTestUrl({ testDir: __dirname });
35+
36+
await page.goto(url);
37+
await page.locator('#custom-feedback-buttom').click();
38+
await page.waitForSelector(':visible:text-is("Report a Bug")');
39+
40+
expect(await page.locator(':visible:text-is("Report a Bug")').count()).toEqual(1);
41+
await page.locator('[name="name"]').fill('Jane Doe');
42+
await page.locator('[name="email"]').fill('[email protected]');
43+
await page.locator('[name="message"]').fill('my example feedback');
44+
await page.locator('[data-sentry-feedback] .btn--primary').click();
45+
46+
const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request());
47+
expect(feedbackEvent).toEqual({
48+
type: 'feedback',
49+
breadcrumbs: expect.any(Array),
50+
contexts: {
51+
feedback: {
52+
contact_email: '[email protected]',
53+
message: 'my example feedback',
54+
name: 'Jane Doe',
55+
source: 'widget',
56+
url: `${TEST_HOST}/index.html`,
57+
},
58+
trace: {
59+
trace_id: expect.stringMatching(/\w{32}/),
60+
span_id: expect.stringMatching(/\w{16}/),
61+
},
62+
},
63+
level: 'info',
64+
timestamp: expect.any(Number),
65+
event_id: expect.stringMatching(/\w{32}/),
66+
environment: 'production',
67+
tags: {},
68+
sdk: {
69+
integrations: expect.arrayContaining(['Feedback']),
70+
version: expect.any(String),
71+
name: 'sentry.javascript.browser',
72+
packages: expect.anything(),
73+
},
74+
request: {
75+
url: `${TEST_HOST}/index.html`,
76+
headers: {
77+
'User-Agent': expect.stringContaining(''),
78+
},
79+
},
80+
platform: 'javascript',
81+
});
82+
});

‎dev-packages/browser-integration-tests/suites/feedback/logger/init.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import * as Sentry from '@sentry/browser';
2+
// Import this separately so that generatePlugin can handle it for CDN scenarios
3+
import { feedbackIntegration } from '@sentry/browser';
4+
5+
const feedback = feedbackIntegration({
6+
autoInject: false,
7+
});
8+
9+
window.Sentry = Sentry;
10+
window.feedback = feedback;
11+
12+
13+
Sentry.init({
14+
dsn: 'https://[email protected]/1337',
15+
debug: true,
16+
integrations: [
17+
feedback,
18+
],
19+
});
20+
21+
// This should log an error!
22+
feedback.attachTo('#does-not-exist');

‎dev-packages/browser-integration-tests/suites/feedback/logger/test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import { shouldSkipFeedbackTest } from '../../../utils/helpers';
5+
6+
/**
7+
* This test is mostly relevant for ensuring that the logger works in all combinations of CDN bundles.
8+
* Even if feedback is included via the CDN, this test ensures that the logger is working correctly.
9+
*/
10+
sentryTest('should log error correctly', async ({ getLocalTestUrl, page }) => {
11+
if (shouldSkipFeedbackTest()) {
12+
sentryTest.skip();
13+
}
14+
15+
const messages: string[] = [];
16+
17+
page.on('console', message => {
18+
messages.push(message.text());
19+
});
20+
21+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
22+
return route.fulfill({
23+
status: 200,
24+
contentType: 'application/json',
25+
body: JSON.stringify({ id: 'test-id' }),
26+
});
27+
});
28+
29+
const url = await getLocalTestUrl({ testDir: __dirname });
30+
31+
await page.goto(url);
32+
33+
expect(messages).toContain('Sentry Logger [log]: Integration installed: Feedback');
34+
expect(messages).toContain('Sentry Logger [error]: [Feedback] Unable to attach to target element');
35+
});

‎dev-packages/browser-integration-tests/suites/integrations/httpclient/httpClientIntegration/init.js

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎dev-packages/browser-integration-tests/suites/integrations/httpclient/httpClientIntegration/subject.js

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎dev-packages/browser-integration-tests/suites/integrations/httpclient/httpClientIntegration/test.ts

Lines changed: 0 additions & 64 deletions
This file was deleted.

‎dev-packages/browser-integration-tests/suites/replay/logger/init.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
window.Replay = Sentry.replayIntegration({
5+
flushMinDelay: 200,
6+
flushMaxDelay: 200,
7+
minReplayDuration: 0,
8+
});
9+
10+
Sentry.init({
11+
dsn: 'https://[email protected]/1337',
12+
sampleRate: 0,
13+
replaysSessionSampleRate: 1.0,
14+
replaysOnErrorSampleRate: 0.0,
15+
debug: true,
16+
17+
integrations: [window.Replay],
18+
});

‎dev-packages/browser-integration-tests/suites/replay/logger/test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../utils/fixtures';
4+
import { shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers';
5+
6+
sentryTest('should output logger messages', async ({ getLocalTestPath, page }) => {
7+
if (shouldSkipReplayTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const messages: string[] = [];
12+
13+
page.on('console', message => {
14+
messages.push(message.text());
15+
});
16+
17+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
18+
return route.fulfill({
19+
status: 200,
20+
contentType: 'application/json',
21+
body: JSON.stringify({ id: 'test-id' }),
22+
});
23+
});
24+
25+
const reqPromise0 = waitForReplayRequest(page, 0);
26+
27+
const url = await getLocalTestPath({ testDir: __dirname });
28+
29+
await Promise.all([page.goto(url), reqPromise0]);
30+
31+
expect(messages).toContain('Sentry Logger [log]: Integration installed: Replay');
32+
expect(messages).toContain('Sentry Logger [info]: [Replay] Creating new session');
33+
expect(messages).toContain('Sentry Logger [info]: [Replay] Starting replay in session mode');
34+
expect(messages).toContain('Sentry Logger [info]: [Replay] Using compression worker');
35+
});

‎dev-packages/browser-integration-tests/utils/generatePlugin.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ const useLoader = bundleKey.startsWith('loader');
3030
const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record<string, string> = {
3131
httpClientIntegration: 'httpclient',
3232
captureConsoleIntegration: 'captureconsole',
33-
CaptureConsole: 'captureconsole',
3433
debugIntegration: 'debug',
3534
rewriteFramesIntegration: 'rewriteframes',
3635
contextLinesIntegration: 'contextlines',

‎packages/utils/src/logger.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ConsoleLevel } from '@sentry/types';
22

33
import { DEBUG_BUILD } from './debug-build';
4-
import { GLOBAL_OBJ } from './worldwide';
4+
import { GLOBAL_OBJ, getGlobalSingleton } from './worldwide';
55

66
/** Prefix for logging strings */
77
const PREFIX = 'Sentry Logger ';
@@ -97,4 +97,17 @@ function makeLogger(): Logger {
9797
return logger as Logger;
9898
}
9999

100-
export const logger = makeLogger();
100+
/**
101+
* This is a logger singleton which either logs things or no-ops if logging is not enabled.
102+
* The logger is a singleton on the carrier, to ensure that a consistent logger is used throughout the SDK.
103+
*/
104+
export const logger: Logger = new Proxy(
105+
{},
106+
{
107+
get: (_target, prop: ConsoleLevel) => {
108+
const logger = getGlobalSingleton('logger', makeLogger);
109+
110+
return logger[prop];
111+
},
112+
},
113+
) as Logger;

‎packages/utils/src/worldwide.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import type { Client, MetricsAggregator, Scope } from '@sentry/types';
1616

1717
import type { SdkSource } from './env';
18+
import type { logger } from './logger';
1819
import { SDK_VERSION } from './version';
1920

2021
interface SentryCarrier {
@@ -25,6 +26,7 @@ interface SentryCarrier {
2526
defaultIsolationScope?: Scope;
2627
defaultCurrentScope?: Scope;
2728
globalMetricsAggregators?: WeakMap<Client, MetricsAggregator> | undefined;
29+
logger?: typeof logger;
2830

2931
/** Overwrites TextEncoder used in `@sentry/utils`, need for `[email protected]` and older */
3032
encodePolyfill?: (input: string) => Uint8Array;

0 commit comments

Comments
 (0)