Skip to content

Commit f6cab6a

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 3e2c8f4 commit f6cab6a

File tree

8 files changed

+124
-17
lines changed

8 files changed

+124
-17
lines changed

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

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

3-
import { sentryTest } from '../../../utils/fixtures';
4-
import { envelopeRequestParser, getEnvelopeType } from '../../../utils/helpers';
5-
import { getCustomRecordingEvents, getReplayEvent, waitForReplayRequest } from '../../../utils/replayHelpers';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { envelopeRequestParser, getEnvelopeType } from '../../../../utils/helpers';
5+
import { getCustomRecordingEvents, getReplayEvent, waitForReplayRequest } from '../../../../utils/replayHelpers';
66

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

1212
const reqPromise0 = waitForReplayRequest(page, 0);
13+
const reqPromise1 = waitForReplayRequest(page, 1);
1314
const feedbackRequestPromise = page.waitForResponse(res => {
1415
const req = res.request();
1516

@@ -35,18 +36,23 @@ sentryTest('should capture feedback (@sentry-internal/feedback import)', async (
3536

3637
const url = await getLocalTestPath({ testDir: __dirname });
3738

38-
await page.goto(url);
39-
await page.getByText('Report a Bug').click();
39+
await Promise.all([
40+
page.goto(url),
41+
page.getByText('Report a Bug').click(),
42+
]);
43+
4044
await page.locator('[name="name"]').fill('Jane Doe');
4145
await page.locator('[name="email"]').fill('[email protected]');
4246
await page.locator('[name="message"]').fill('my example feedback');
4347
await page.getByLabel('Send Bug Report').click();
4448

45-
const [feedbackResp, replayReq] = await Promise.all([feedbackRequestPromise, reqPromise0]);
49+
const [feedbackResp, replayReq1, replayReq2] = await Promise.all([feedbackRequestPromise, reqPromise0, reqPromise1]);
4650

4751
const feedbackEvent = envelopeRequestParser(feedbackResp.request());
48-
const replayEvent = getReplayEvent(replayReq);
49-
const { breadcrumbs } = getCustomRecordingEvents(replayReq);
52+
const replayEvent = getReplayEvent(replayReq1);
53+
// Feedback breadcrumb is on second segment because we flush when "Report a Bug" is clicked
54+
// And then the breadcrumb is sent when feedback form is submitted
55+
const { breadcrumbs } = getCustomRecordingEvents(replayReq2);
5056

5157
expect(breadcrumbs).toEqual(
5258
expect.arrayContaining([
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: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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('should capture feedback with no replay sampling when Form opens (@sentry-internal/feedback import)', async ({ getLocalTestPath, page }) => {
8+
if (process.env.PW_BUNDLE) {
9+
sentryTest.skip();
10+
}
11+
12+
const reqPromise0 = waitForReplayRequest(page, 0);
13+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
14+
return route.fulfill({
15+
status: 200,
16+
contentType: 'application/json',
17+
body: JSON.stringify({ id: 'test-id' }),
18+
});
19+
});
20+
21+
const url = await getLocalTestPath({ testDir: __dirname });
22+
23+
const [,,replayReq] = await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);
24+
25+
const replayEvent = getReplayEvent(replayReq);
26+
expect(replayEvent.segment_id).toBe(0);
27+
expect(replayEvent.replay_type).toBe('buffer');
28+
});

packages/feedback/src/integration.ts

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

44
import {
@@ -79,17 +79,18 @@ export class Feedback implements Integration {
7979
private _hasInsertedActorStyles: boolean;
8080

8181
public constructor({
82+
autoInject = true,
8283
id = 'sentry-feedback',
84+
includeReplay = true,
85+
isEmailRequired = false,
86+
isNameRequired = false,
8387
showBranding = true,
84-
autoInject = true,
8588
showEmail = true,
8689
showName = true,
8790
useSentryUser = {
8891
email: 'email',
8992
name: 'username',
9093
},
91-
isEmailRequired = false,
92-
isNameRequired = false,
9394

9495
themeDark,
9596
themeLight,
@@ -123,9 +124,10 @@ export class Feedback implements Integration {
123124
this._hasInsertedActorStyles = false;
124125

125126
this.options = {
126-
id,
127-
showBranding,
128127
autoInject,
128+
showBranding,
129+
id,
130+
includeReplay,
129131
isEmailRequired,
130132
isNameRequired,
131133
showEmail,
@@ -185,6 +187,31 @@ export class Feedback implements Integration {
185187
}
186188
}
187189

190+
/**
191+
* Hook that is called after all integrations is setup. This is used to
192+
* integrate with the Replay integration to save a replay when Feedback modal
193+
* is opened.
194+
*/
195+
public afterAllSetup(client: Client<ClientOptions<BaseTransportOptions>>): void {
196+
if (!this.options.includeReplay) {
197+
return;
198+
}
199+
200+
const replay = client.getIntegrationByName!('Replay');
201+
202+
if (!replay) {
203+
return;
204+
}
205+
206+
try {
207+
// @ts-expect-error Not sure how best to type this w/o
208+
// making @sentry/replay a dep
209+
replay.startBuffering();
210+
} catch (err) {
211+
DEBUG_BUILD && logger.error(err);
212+
}
213+
}
214+
188215
/**
189216
* Allows user to open the dialog box. Creates a new widget if
190217
* `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.

packages/feedback/src/widget/createWidget.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getCurrentScope } from '@sentry/core';
1+
import { getClient, getCurrentScope } from '@sentry/core';
22
import { logger } from '@sentry/utils';
33

44
import type { FeedbackFormData, FeedbackInternalOptions, FeedbackWidget } from '../types';
@@ -9,6 +9,8 @@ import type { DialogComponent } from './Dialog';
99
import { Dialog } from './Dialog';
1010
import { SuccessMessage } from './SuccessMessage';
1111

12+
import { DEBUG_BUILD } from '../debug-build';
13+
1214
interface CreateWidgetParams {
1315
/**
1416
* Shadow DOM to append to
@@ -124,6 +126,27 @@ export function createWidget({
124126
}
125127
}
126128

129+
/**
130+
* Internal handler when dialog is opened
131+
*/
132+
function handleOpenDialog({includeReplay}: {includeReplay?: boolean} = {}) {
133+
// Flush replay if it exists
134+
if (includeReplay) {
135+
const client = getClient();
136+
const replay = client && client.getIntegrationByName!('Replay');
137+
if (!replay) {
138+
return;
139+
}
140+
try {
141+
// @ts-expect-error Not sure how best to type this w/o
142+
// making @sentry/replay a dep
143+
void replay.flush();
144+
} catch (err) {
145+
DEBUG_BUILD && logger.error(err);
146+
}
147+
}
148+
}
149+
127150
/**
128151
* Displays the default actor
129152
*/
@@ -156,6 +179,7 @@ export function createWidget({
156179
if (options.onFormOpen) {
157180
options.onFormOpen();
158181
}
182+
handleOpenDialog({includeReplay: options.includeReplay});
159183
return;
160184
}
161185

@@ -208,6 +232,7 @@ export function createWidget({
208232
if (options.onFormOpen) {
209233
options.onFormOpen();
210234
}
235+
handleOpenDialog({includeReplay: options.includeReplay});
211236
} catch (err) {
212237
// TODO: Error handling?
213238
logger.error(err);

packages/replay/src/util/addGlobalListeners.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ export function addGlobalListeners(replay: ReplayContainer): void {
5959
const replayId = replay.getSessionId();
6060
if (options && options.includeReplay && replay.isEnabled() && replayId) {
6161
// This should never reject
62-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
63-
replay.flush();
6462
if (feedbackEvent.contexts && feedbackEvent.contexts.feedback) {
6563
feedbackEvent.contexts.feedback.replay_id = replayId;
6664
}

0 commit comments

Comments
 (0)