Skip to content

Commit 3cb5108

Browse files
authored
feat(feedback): Flush replays when feedback form opens (#10567)
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 bd37568 commit 3cb5108

File tree

6 files changed

+132
-99
lines changed

6 files changed

+132
-99
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+
);

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

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

packages/feedback/src/integration.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,17 @@ export class Feedback implements Integration {
7979
private _hasInsertedActorStyles: boolean;
8080

8181
public constructor({
82+
autoInject = true,
8283
id = 'sentry-feedback',
84+
isEmailRequired = false,
85+
isNameRequired = false,
8386
showBranding = true,
84-
autoInject = true,
8587
showEmail = true,
8688
showName = true,
8789
useSentryUser = {
8890
email: 'email',
8991
name: 'username',
9092
},
91-
isEmailRequired = false,
92-
isNameRequired = false,
9393

9494
themeDark,
9595
themeLight,
@@ -123,9 +123,9 @@ export class Feedback implements Integration {
123123
this._hasInsertedActorStyles = false;
124124

125125
this.options = {
126-
id,
127-
showBranding,
128126
autoInject,
127+
showBranding,
128+
id,
129129
isEmailRequired,
130130
isNameRequired,
131131
showEmail,

packages/feedback/src/widget/createWidget.ts

Lines changed: 20 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,21 @@ export function createWidget({
124126
}
125127
}
126128

129+
/**
130+
* Internal handler when dialog is opened
131+
*/
132+
function handleOpenDialog(): void {
133+
// Flush replay if integration exists
134+
const client = getClient();
135+
const replay = client && client.getIntegrationByName<{ name: string; flush: () => Promise<void> }>('Replay');
136+
if (!replay) {
137+
return;
138+
}
139+
replay.flush().catch(err => {
140+
DEBUG_BUILD && logger.error(err);
141+
});
142+
}
143+
127144
/**
128145
* Displays the default actor
129146
*/
@@ -156,6 +173,7 @@ export function createWidget({
156173
if (options.onFormOpen) {
157174
options.onFormOpen();
158175
}
176+
handleOpenDialog();
159177
return;
160178
}
161179

@@ -208,6 +226,7 @@ export function createWidget({
208226
if (options.onFormOpen) {
209227
options.onFormOpen();
210228
}
229+
handleOpenDialog();
211230
} catch (err) {
212231
// TODO: Error handling?
213232
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)