Skip to content

Commit c45d03f

Browse files
committed
feat(feedback): Auto start buffering replays if enabled and flush on form open
* By default (can be disabled), if replay integration exists, start buffering * Flush replay when the feedback form is first opened instead of at submit time We are making this change because we have noticed a lot of feedback replays only consist of the user submitting the feedback and not what they did prior to submitting feedback. This may result in false positives if users open but do not submit feedback, but this should make replays from feedback more useful.
1 parent 15216cd commit c45d03f

File tree

11 files changed

+221
-100
lines changed

11 files changed

+221
-100
lines changed
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { expect } from '@playwright/test';
2+
3+
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();
12+
}
13+
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+
});
31+
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+
});
38+
});
39+
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: {
99+
url: expect.stringContaining('/dist/index.html'),
100+
headers: {
101+
'User-Agent': expect.stringContaining(''),
102+
},
103+
},
104+
platform: 'javascript',
105+
});
106+
},
107+
);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { Feedback } from '@sentry-internal/feedback';
2+
import * as Sentry from '@sentry/browser';
3+
4+
window.Sentry = Sentry;
5+
6+
Sentry.init({
7+
dsn: 'https://[email protected]/1337',
8+
replaysOnErrorSampleRate: 0,
9+
replaysSessionSampleRate: 0,
10+
integrations: [
11+
new Sentry.Replay({
12+
flushMinDelay: 200,
13+
flushMaxDelay: 200,
14+
minReplayDuration: 0,
15+
}),
16+
new Feedback(),
17+
],
18+
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { getReplayEvent, waitForReplayRequest } from '../../../../utils/replayHelpers';
5+
6+
sentryTest(
7+
'should capture feedback with no replay sampling when Form opens (@sentry-internal/feedback import)',
8+
async ({ getLocalTestPath, page }) => {
9+
if (process.env.PW_BUNDLE) {
10+
sentryTest.skip();
11+
}
12+
13+
const reqPromise0 = waitForReplayRequest(page, 0);
14+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
15+
return route.fulfill({
16+
status: 200,
17+
contentType: 'application/json',
18+
body: JSON.stringify({ id: 'test-id' }),
19+
});
20+
});
21+
22+
const url = await getLocalTestPath({ testDir: __dirname });
23+
24+
const [, , replayReq] = await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
25+
26+
const replayEvent = getReplayEvent(replayReq);
27+
expect(replayEvent.segment_id).toBe(0);
28+
expect(replayEvent.replay_type).toBe('buffer');
29+
},
30+
);

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

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

packages/feedback/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
"@sentry/types": "7.100.0",
3434
"@sentry/utils": "7.100.0"
3535
},
36+
"devDependencies": {
37+
"@sentry/replay": "7.100.0"
38+
},
3639
"scripts": {
3740
"build": "run-p build:transpile build:types build:bundle",
3841
"build:transpile": "rollup -c rollup.npm.config.mjs",

packages/feedback/src/integration.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { Integration, IntegrationFn } from '@sentry/types';
1+
import { type replayIntegration } from '@sentry/replay';
2+
import type { BaseTransportOptions, Client, ClientOptions, Integration, IntegrationFn } from '@sentry/types';
23
import { isBrowser, logger } from '@sentry/utils';
34

45
import {
@@ -79,17 +80,18 @@ export class Feedback implements Integration {
7980
private _hasInsertedActorStyles: boolean;
8081

8182
public constructor({
83+
autoInject = true,
8284
id = 'sentry-feedback',
85+
includeReplay = true,
86+
isEmailRequired = false,
87+
isNameRequired = false,
8388
showBranding = true,
84-
autoInject = true,
8589
showEmail = true,
8690
showName = true,
8791
useSentryUser = {
8892
email: 'email',
8993
name: 'username',
9094
},
91-
isEmailRequired = false,
92-
isNameRequired = false,
9395

9496
themeDark,
9597
themeLight,
@@ -123,9 +125,10 @@ export class Feedback implements Integration {
123125
this._hasInsertedActorStyles = false;
124126

125127
this.options = {
126-
id,
127-
showBranding,
128128
autoInject,
129+
showBranding,
130+
id,
131+
includeReplay,
129132
isEmailRequired,
130133
isNameRequired,
131134
showEmail,
@@ -185,6 +188,29 @@ export class Feedback implements Integration {
185188
}
186189
}
187190

191+
/**
192+
* Hook that is called after all integrations are setup. This is used to
193+
* integrate with the Replay integration to save a replay when Feedback modal
194+
* is opened.
195+
*/
196+
public afterAllSetup(client: Client<ClientOptions<BaseTransportOptions>>): void {
197+
if (!this.options.includeReplay) {
198+
return;
199+
}
200+
201+
const replay = client.getIntegrationByName<ReturnType<typeof replayIntegration>>('Replay');
202+
203+
if (!replay) {
204+
return;
205+
}
206+
207+
try {
208+
replay.startBuffering();
209+
} catch (err) {
210+
DEBUG_BUILD && logger.error(err);
211+
}
212+
}
213+
188214
/**
189215
* Allows user to open the dialog box. Creates a new widget if
190216
* `autoInject` was false, otherwise re-uses the default widget that was

packages/feedback/src/types/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ export interface FeedbackGeneralConfiguration {
7272
*/
7373
showName: boolean;
7474

75+
/**
76+
* Should Feedback attempt to include a Session Replay if the integration is installed?
77+
*/
78+
includeReplay: boolean;
79+
7580
/**
7681
* Fill in email/name input fields with Sentry user context if it exists.
7782
* The value of the email/name keys represent the properties of your user context.

0 commit comments

Comments
 (0)