Skip to content

Commit 621b691

Browse files
authored
feat(replay): Merge packages together & ensure bundles are built (#11552)
This reverts #11342 and instead generates multiple bundles etc. from the one `@sentry-internal/feedback` package. * The `feedback` CDN bundle integration includes the modal integration for now. * There is also a dedicated modal & screenshot integration on the CDN * Also kept the shims etc. that Ryan added in the now-reverted PR * Size limits now seem correct to me! I bumped them (as they now correctly include the feedback modal etc), but you can see the diff to the screenshots being included (it's not too high, really).
1 parent 38b0dae commit 621b691

Some content is hidden

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

59 files changed

+230
-821
lines changed

.craft.yml

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,7 @@ targets:
3232
- name: npm
3333
id: '@sentry-internal/feedback'
3434
includeNames: /^sentry-internal-feedback-\d.*\.tgz$/
35-
## 1.8 Feedback Modal package (browser only)
36-
- name: npm
37-
id: '@sentry-internal/feedback-modal'
38-
includeNames: /^sentry-internal-feedback-modal-\d.*\.tgz$/
39-
## 1.9 Feedback Screenshot package (browser only)
40-
- name: npm
41-
id: '@sentry-internal/feedback-screenshot'
42-
includeNames: /^sentry-internal-feedback-screenshot-\d.*\.tgz$/
43-
## 1.10 ReplayCanvas package (browser only)
35+
## 1.8 ReplayCanvas package (browser only)
4436
- name: npm
4537
id: '@sentry-internal/replay-canvas'
4638
includeNames: /^sentry-internal-replay-canvas-\d.*\.tgz$/

.github/workflows/build.yml

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ jobs:
105105
- 'packages/replay/**'
106106
- 'packages/replay-canvas/**'
107107
- 'packages/feedback/**'
108-
- 'packages/feedback-modal/**'
109-
- 'packages/feedback-screenshot/**'
110108
- 'packages/wasm/**'
111109
browser_integration:
112110
- *shared
@@ -413,6 +411,7 @@ jobs:
413411
${{ github.workspace }}/packages/browser/build/bundles/**
414412
${{ github.workspace }}/packages/replay/build/bundles/**
415413
${{ github.workspace }}/packages/replay-canvas/build/bundles/**
414+
${{ github.workspace }}/packages/feedback/build/bundles/**
416415
${{ github.workspace }}/packages/**/*.tgz
417416
${{ github.workspace }}/packages/aws-serverless/build/aws/dist-serverless/*.zip
418417
@@ -623,21 +622,20 @@ jobs:
623622
- bundle
624623
- bundle_min
625624
- bundle_replay
626-
- bundle_replay_min
627625
- bundle_tracing
628-
- bundle_tracing_min
629626
- bundle_tracing_replay
630-
- bundle_tracing_replay_min
627+
- bundle_tracing_replay_feedback
628+
- bundle_tracing_replay_feedback_min
631629
project:
632630
- chromium
633631
include:
634632
# Only check all projects for esm & full bundle
635633
# We also shard the tests as they take the longest
636-
- bundle: bundle_tracing_replay_min
634+
- bundle: bundle_tracing_replay_feedback_min
637635
project: ''
638636
shard: 1
639637
shards: 2
640-
- bundle: bundle_tracing_replay_min
638+
- bundle: bundle_tracing_replay_feedback_min
641639
project: ''
642640
shard: 2
643641
shards: 2
@@ -654,7 +652,7 @@ jobs:
654652
shards: 3
655653
exclude:
656654
# Do not run the default chromium-only tests
657-
- bundle: bundle_tracing_replay_min
655+
- bundle: bundle_tracing_replay_feedback_min
658656
project: 'chromium'
659657
- bundle: esm
660658
project: 'chromium'

.size-limit.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ module.exports = [
7373
path: 'packages/browser/build/npm/esm/index.js',
7474
import: createImport('init', 'feedbackIntegration', 'feedbackModalIntegration', 'feedbackScreenshotIntegration'),
7575
gzip: true,
76-
limit: '37 KB',
76+
limit: '40 KB',
7777
},
7878
{
7979
name: '@sentry/browser (incl. sendFeedback)',
@@ -143,7 +143,7 @@ module.exports = [
143143
name: 'CDN Bundle (incl. Tracing, Replay, Feedback)',
144144
path: createCDNPath('bundle.tracing.replay.feedback.min.js'),
145145
gzip: true,
146-
limit: '83 KB',
146+
limit: '86 KB',
147147
},
148148
// browser CDN bundles (non-gzipped)
149149
{

CODEOWNERS

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,4 @@ packages/replay @getsentry/replay-sdk-web
22
packages/replay-worker @getsentry/replay-sdk-web
33
packages/replay-canvas @getsentry/replay-sdk-web
44
packages/feedback @getsentry/replay-sdk-web
5-
packages/feedback-modal @getsentry/replay-sdk-web
6-
packages/feedback-screenshot @getsentry/replay-sdk-web
75
dev-packages/browser-integration-tests/suites/replay @getsentry/replay-sdk-web

dev-packages/browser-integration-tests/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
"test:bundle:replay:min": "PW_BUNDLE=bundle_replay_min yarn test",
2424
"test:bundle:tracing": "PW_BUNDLE=bundle_tracing yarn test",
2525
"test:bundle:tracing:min": "PW_BUNDLE=bundle_tracing_min yarn test",
26+
"test:bundle:full": "PW_BUNDLE=bundle_tracing_replay_feedback yarn test",
27+
"test:bundle:full:min": "PW_BUNDLE=bundle_tracing_replay_feedback_min yarn test",
2628
"test:cjs": "PW_BUNDLE=cjs yarn test",
2729
"test:esm": "PW_BUNDLE=esm yarn test",
2830
"test:loader": "npx playwright test -c playwright.loader.config.ts --project='chromium'",

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { expect } from '@playwright/test';
22

33
import { sentryTest } from '../../../utils/fixtures';
4-
import { envelopeRequestParser, getEnvelopeType } from '../../../utils/helpers';
4+
import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../utils/helpers';
55

6-
sentryTest('should capture feedback (@sentry-internal/feedback import)', async ({ getLocalTestPath, page }) => {
7-
if (process.env.PW_BUNDLE) {
6+
sentryTest('should capture feedback', async ({ getLocalTestPath, page }) => {
7+
if (shouldSkipFeedbackTest()) {
88
sentryTest.skip();
99
}
1010

@@ -39,7 +39,7 @@ sentryTest('should capture feedback (@sentry-internal/feedback import)', async (
3939
await page.locator('[name="name"]').fill('Jane Doe');
4040
await page.locator('[name="email"]').fill('[email protected]');
4141
await page.locator('[name="message"]').fill('my example feedback');
42-
await page.getByLabel('Send Bug Report').click();
42+
await page.locator('[data-sentry-feedback] .btn--primary').click();
4343

4444
const feedbackEvent = envelopeRequestParser((await feedbackRequestPromise).request());
4545
expect(feedbackEvent).toEqual({

dev-packages/browser-integration-tests/suites/feedback/captureFeedbackAndReplay/hasSampling/init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ window.Sentry = Sentry;
55
Sentry.init({
66
dsn: 'https://[email protected]/1337',
77
replaysOnErrorSampleRate: 1.0,
8-
replaysSessionSampleRate: 0,
8+
replaysSessionSampleRate: 1.0,
99
integrations: [
1010
Sentry.replayIntegration({
1111
flushMinDelay: 200,
Lines changed: 101 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -1,107 +1,111 @@
11
import { expect } from '@playwright/test';
22

33
import { sentryTest } from '../../../../utils/fixtures';
4-
import { envelopeRequestParser, getEnvelopeType } from '../../../../utils/helpers';
5-
import { getCustomRecordingEvents, getReplayEvent, waitForReplayRequest } from '../../../../utils/replayHelpers';
6-
7-
sentryTest(
8-
'should capture feedback (@sentry-internal/feedback import)',
9-
async ({ forceFlushReplay, getLocalTestPath, page }) => {
10-
if (process.env.PW_BUNDLE) {
11-
sentryTest.skip();
4+
import { envelopeRequestParser, getEnvelopeType, shouldSkipFeedbackTest } from '../../../../utils/helpers';
5+
import {
6+
collectReplayRequests,
7+
getReplayBreadcrumbs,
8+
shouldSkipReplayTest,
9+
waitForReplayRequest,
10+
} from '../../../../utils/replayHelpers';
11+
12+
sentryTest('should capture feedback', async ({ forceFlushReplay, getLocalTestPath, page }) => {
13+
if (shouldSkipFeedbackTest() || shouldSkipReplayTest()) {
14+
sentryTest.skip();
15+
}
16+
17+
const reqPromise0 = waitForReplayRequest(page, 0);
18+
19+
const feedbackRequestPromise = page.waitForResponse(res => {
20+
const req = res.request();
21+
22+
const postData = req.postData();
23+
if (!postData) {
24+
return false;
1225
}
1326

14-
const reqPromise0 = waitForReplayRequest(page, 0);
15-
const reqPromise1 = waitForReplayRequest(page, 1);
16-
const reqPromise2 = waitForReplayRequest(page, 2);
17-
const feedbackRequestPromise = page.waitForResponse(res => {
18-
const req = res.request();
19-
20-
const postData = req.postData();
21-
if (!postData) {
22-
return false;
23-
}
24-
25-
try {
26-
return getEnvelopeType(req) === 'feedback';
27-
} catch (err) {
28-
return false;
29-
}
30-
});
27+
try {
28+
return getEnvelopeType(req) === 'feedback';
29+
} catch (err) {
30+
return false;
31+
}
32+
});
3133

32-
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
33-
return route.fulfill({
34-
status: 200,
35-
contentType: 'application/json',
36-
body: JSON.stringify({ id: 'test-id' }),
37-
});
34+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
35+
return route.fulfill({
36+
status: 200,
37+
contentType: 'application/json',
38+
body: JSON.stringify({ id: 'test-id' }),
3839
});
40+
});
3941

40-
const url = await getLocalTestPath({ testDir: __dirname });
41-
42-
const [, , replayReq0] = await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
43-
44-
// Inputs are slow, these need to be serial
45-
await page.locator('[name="name"]').fill('Jane Doe');
46-
await page.locator('[name="email"]').fill('[email protected]');
47-
await page.locator('[name="message"]').fill('my example feedback');
48-
49-
// Force flush here, as inputs are slow and can cause click event to be in unpredictable segments
50-
await Promise.all([forceFlushReplay(), reqPromise1]);
51-
52-
const [, feedbackResp, replayReq2] = await Promise.all([
53-
page.getByLabel('Send Bug Report').click(),
54-
feedbackRequestPromise,
55-
reqPromise2,
56-
]);
57-
58-
const feedbackEvent = envelopeRequestParser(feedbackResp.request());
59-
const replayEvent = getReplayEvent(replayReq0);
60-
// Feedback breadcrumb is on second segment because we flush when "Report a Bug" is clicked
61-
// And then the breadcrumb is sent when feedback form is submitted
62-
const { breadcrumbs } = getCustomRecordingEvents(replayReq2);
63-
64-
expect(breadcrumbs).toEqual(
65-
expect.arrayContaining([
66-
expect.objectContaining({
67-
category: 'sentry.feedback',
68-
data: { feedbackId: expect.any(String) },
69-
timestamp: expect.any(Number),
70-
type: 'default',
71-
}),
72-
]),
73-
);
74-
75-
expect(feedbackEvent).toEqual({
76-
type: 'feedback',
77-
breadcrumbs: expect.any(Array),
78-
contexts: {
79-
feedback: {
80-
contact_email: '[email protected]',
81-
message: 'my example feedback',
82-
name: 'Jane Doe',
83-
replay_id: replayEvent.event_id,
84-
source: 'widget',
85-
url: expect.stringContaining('/dist/index.html'),
86-
},
87-
},
88-
level: 'info',
89-
timestamp: expect.any(Number),
90-
event_id: expect.stringMatching(/\w{32}/),
91-
environment: 'production',
92-
sdk: {
93-
integrations: expect.arrayContaining(['Feedback']),
94-
version: expect.any(String),
95-
name: 'sentry.javascript.browser',
96-
packages: expect.anything(),
97-
},
98-
request: {
42+
const url = await getLocalTestPath({ testDir: __dirname });
43+
44+
await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
45+
46+
const replayRequestPromise = collectReplayRequests(page, recordingEvents => {
47+
return getReplayBreadcrumbs(recordingEvents).some(breadcrumb => breadcrumb.category === 'sentry.feedback');
48+
});
49+
50+
// Inputs are slow, these need to be serial
51+
await page.locator('[name="name"]').fill('Jane Doe');
52+
await page.locator('[name="email"]').fill('[email protected]');
53+
await page.locator('[name="message"]').fill('my example feedback');
54+
55+
// Force flush here, as inputs are slow and can cause click event to be in unpredictable segments
56+
await Promise.all([forceFlushReplay()]);
57+
58+
const [, feedbackResp] = await Promise.all([
59+
page.locator('[data-sentry-feedback] .btn--primary').click(),
60+
feedbackRequestPromise,
61+
]);
62+
63+
const { replayEvents, replayRecordingSnapshots } = await replayRequestPromise;
64+
const breadcrumbs = getReplayBreadcrumbs(replayRecordingSnapshots);
65+
66+
const replayEvent = replayEvents[0];
67+
const feedbackEvent = envelopeRequestParser(feedbackResp.request());
68+
69+
expect(breadcrumbs).toEqual(
70+
expect.arrayContaining([
71+
expect.objectContaining({
72+
category: 'sentry.feedback',
73+
data: { feedbackId: expect.any(String) },
74+
timestamp: expect.any(Number),
75+
type: 'default',
76+
}),
77+
]),
78+
);
79+
80+
expect(feedbackEvent).toEqual({
81+
type: 'feedback',
82+
breadcrumbs: expect.any(Array),
83+
contexts: {
84+
feedback: {
85+
contact_email: '[email protected]',
86+
message: 'my example feedback',
87+
name: 'Jane Doe',
88+
replay_id: replayEvent.event_id,
89+
source: 'widget',
9990
url: expect.stringContaining('/dist/index.html'),
100-
headers: {
101-
'User-Agent': expect.stringContaining(''),
102-
},
10391
},
104-
platform: 'javascript',
105-
});
106-
},
107-
);
92+
},
93+
level: 'info',
94+
timestamp: expect.any(Number),
95+
event_id: expect.stringMatching(/\w{32}/),
96+
environment: 'production',
97+
sdk: {
98+
integrations: expect.arrayContaining(['Feedback']),
99+
version: expect.any(String),
100+
name: 'sentry.javascript.browser',
101+
packages: expect.anything(),
102+
},
103+
request: {
104+
url: expect.stringContaining('/dist/index.html'),
105+
headers: {
106+
'User-Agent': expect.stringContaining(''),
107+
},
108+
},
109+
platform: 'javascript',
110+
});
111+
});

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,14 @@ const useLoader = bundleKey.startsWith('loader');
2626
// In this case, if we encounter this import, we want to add this CDN bundle file instead
2727
const IMPORTED_INTEGRATION_CDN_BUNDLE_PATHS: Record<string, string> = {
2828
httpClientIntegration: 'httpclient',
29-
HttpClient: 'httpclient',
3029
captureConsoleIntegration: 'captureconsole',
3130
CaptureConsole: 'captureconsole',
3231
debugIntegration: 'debug',
33-
Debug: 'debug',
3432
rewriteFramesIntegration: 'rewriteframes',
35-
RewriteFrames: 'rewriteframes',
3633
contextLinesIntegration: 'contextlines',
37-
ContextLines: 'contextlines',
3834
extraErrorDataIntegration: 'extraerrordata',
39-
ExtraErrorData: 'extraerrordata',
4035
reportingObserverIntegration: 'reportingobserver',
41-
ReportingObserver: 'reportingobserver',
4236
sessionTimingIntegration: 'sessiontiming',
43-
SessionTiming: 'sessiontiming',
4437
};
4538

4639
const BUNDLE_PATHS: Record<string, Record<string, string>> = {
@@ -55,6 +48,8 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
5548
bundle_tracing_min: 'build/bundles/bundle.tracing.min.js',
5649
bundle_tracing_replay: 'build/bundles/bundle.tracing.replay.js',
5750
bundle_tracing_replay_min: 'build/bundles/bundle.tracing.replay.min.js',
51+
bundle_tracing_replay_feedback: 'build/bundles/bundle.tracing.replay.feedback.js',
52+
bundle_tracing_replay_feedback_min: 'build/bundles/bundle.tracing.replay.feedback.min.js',
5853
loader_base: 'build/bundles/bundle.min.js',
5954
loader_eager: 'build/bundles/bundle.min.js',
6055
loader_debug: 'build/bundles/bundle.debug.min.js',
@@ -227,7 +222,8 @@ class SentryScenarioGenerationPlugin {
227222
const integrationBundleKey = bundleKey
228223
.replace('loader_', 'bundle_')
229224
.replace('_replay', '')
230-
.replace('_tracing', '');
225+
.replace('_tracing', '')
226+
.replace('_feedback', '');
231227

232228
this.requiredIntegrations.forEach(integration => {
233229
const fileName = `${integration}.bundle.js`;

0 commit comments

Comments
 (0)