Skip to content

Commit

Permalink
feat(feedback): Flush replays when feedback form opens (#10567)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
billyvg committed Feb 29, 2024
1 parent bd37568 commit 3cb5108
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 99 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, getEnvelopeType } from '../../../../utils/helpers';
import { getCustomRecordingEvents, getReplayEvent, waitForReplayRequest } from '../../../../utils/replayHelpers';

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

const reqPromise0 = waitForReplayRequest(page, 0);
const reqPromise1 = waitForReplayRequest(page, 1);
const reqPromise2 = waitForReplayRequest(page, 2);
const feedbackRequestPromise = page.waitForResponse(res => {
const req = res.request();

const postData = req.postData();
if (!postData) {
return false;
}

try {
return getEnvelopeType(req) === 'feedback';
} catch (err) {
return false;
}
});

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

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

const [, , replayReq0] = await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);

// Inputs are slow, these need to be serial
await page.locator('[name="name"]').fill('Jane Doe');
await page.locator('[name="email"]').fill('janedoe@example.org');
await page.locator('[name="message"]').fill('my example feedback');

// Force flush here, as inputs are slow and can cause click event to be in unpredictable segments
await Promise.all([forceFlushReplay(), reqPromise1]);

const [, feedbackResp, replayReq2] = await Promise.all([
page.getByLabel('Send Bug Report').click(),
feedbackRequestPromise,
reqPromise2,
]);

const feedbackEvent = envelopeRequestParser(feedbackResp.request());
const replayEvent = getReplayEvent(replayReq0);
// Feedback breadcrumb is on second segment because we flush when "Report a Bug" is clicked
// And then the breadcrumb is sent when feedback form is submitted
const { breadcrumbs } = getCustomRecordingEvents(replayReq2);

expect(breadcrumbs).toEqual(
expect.arrayContaining([
expect.objectContaining({
category: 'sentry.feedback',
data: { feedbackId: expect.any(String) },
timestamp: expect.any(Number),
type: 'default',
}),
]),
);

expect(feedbackEvent).toEqual({
type: 'feedback',
breadcrumbs: expect.any(Array),
contexts: {
feedback: {
contact_email: 'janedoe@example.org',
message: 'my example feedback',
name: 'Jane Doe',
replay_id: replayEvent.event_id,
source: 'widget',
url: expect.stringContaining('/dist/index.html'),
},
},
level: 'info',
timestamp: expect.any(Number),
event_id: expect.stringMatching(/\w{32}/),
environment: 'production',
sdk: {
integrations: expect.arrayContaining(['Feedback']),
version: expect.any(String),
name: 'sentry.javascript.browser',
packages: expect.anything(),
},
request: {
url: expect.stringContaining('/dist/index.html'),
headers: {
'User-Agent': expect.stringContaining(''),
},
},
platform: 'javascript',
});
},
);

This file was deleted.

10 changes: 5 additions & 5 deletions packages/feedback/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,17 @@ export class Feedback implements Integration {
private _hasInsertedActorStyles: boolean;

public constructor({
autoInject = true,
id = 'sentry-feedback',
isEmailRequired = false,
isNameRequired = false,
showBranding = true,
autoInject = true,
showEmail = true,
showName = true,
useSentryUser = {
email: 'email',
name: 'username',
},
isEmailRequired = false,
isNameRequired = false,

themeDark,
themeLight,
Expand Down Expand Up @@ -123,9 +123,9 @@ export class Feedback implements Integration {
this._hasInsertedActorStyles = false;

this.options = {
id,
showBranding,
autoInject,
showBranding,
id,
isEmailRequired,
isNameRequired,
showEmail,
Expand Down
21 changes: 20 additions & 1 deletion packages/feedback/src/widget/createWidget.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getCurrentScope } from '@sentry/core';
import { getClient, getCurrentScope } from '@sentry/core';
import { logger } from '@sentry/utils';

import type { FeedbackFormData, FeedbackInternalOptions, FeedbackWidget } from '../types';
Expand All @@ -9,6 +9,8 @@ import type { DialogComponent } from './Dialog';
import { Dialog } from './Dialog';
import { SuccessMessage } from './SuccessMessage';

import { DEBUG_BUILD } from '../debug-build';

interface CreateWidgetParams {
/**
* Shadow DOM to append to
Expand Down Expand Up @@ -124,6 +126,21 @@ export function createWidget({
}
}

/**
* Internal handler when dialog is opened
*/
function handleOpenDialog(): void {
// Flush replay if integration exists
const client = getClient();
const replay = client && client.getIntegrationByName<{ name: string; flush: () => Promise<void> }>('Replay');
if (!replay) {
return;
}
replay.flush().catch(err => {
DEBUG_BUILD && logger.error(err);
});
}

/**
* Displays the default actor
*/
Expand Down Expand Up @@ -156,6 +173,7 @@ export function createWidget({
if (options.onFormOpen) {
options.onFormOpen();
}
handleOpenDialog();
return;
}

Expand Down Expand Up @@ -208,6 +226,7 @@ export function createWidget({
if (options.onFormOpen) {
options.onFormOpen();
}
handleOpenDialog();
} catch (err) {
// TODO: Error handling?
logger.error(err);
Expand Down
2 changes: 0 additions & 2 deletions packages/replay/src/util/addGlobalListeners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ export function addGlobalListeners(replay: ReplayContainer): void {
const replayId = replay.getSessionId();
if (options && options.includeReplay && replay.isEnabled() && replayId) {
// This should never reject
// eslint-disable-next-line @typescript-eslint/no-floating-promises
replay.flush();
if (feedbackEvent.contexts && feedbackEvent.contexts.feedback) {
feedbackEvent.contexts.feedback.replay_id = replayId;
}
Expand Down

0 comments on commit 3cb5108

Please sign in to comment.