Skip to content

Commit

Permalink
feat(feedback): Auto start buffering replays if enabled and flush on …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
billyvg committed Feb 21, 2024
1 parent 15216cd commit c45d03f
Show file tree
Hide file tree
Showing 11 changed files with 221 additions and 100 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',
});
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { Feedback } from '@sentry-internal/feedback';
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
replaysOnErrorSampleRate: 0,
replaysSessionSampleRate: 0,
integrations: [
new Sentry.Replay({
flushMinDelay: 200,
flushMaxDelay: 200,
minReplayDuration: 0,
}),
new Feedback(),
],
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
'should capture feedback with no replay sampling when Form opens (@sentry-internal/feedback import)',
async ({ getLocalTestPath, page }) => {
if (process.env.PW_BUNDLE) {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);
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 [, , replayReq] = await Promise.all([page.goto(url), page.getByText('Report a Bug').click(), reqPromise0]);

const replayEvent = getReplayEvent(replayReq);
expect(replayEvent.segment_id).toBe(0);
expect(replayEvent.replay_type).toBe('buffer');
},
);

This file was deleted.

3 changes: 3 additions & 0 deletions packages/feedback/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
"@sentry/types": "7.100.0",
"@sentry/utils": "7.100.0"
},
"devDependencies": {
"@sentry/replay": "7.100.0"
},
"scripts": {
"build": "run-p build:transpile build:types build:bundle",
"build:transpile": "rollup -c rollup.npm.config.mjs",
Expand Down
38 changes: 32 additions & 6 deletions packages/feedback/src/integration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Integration, IntegrationFn } from '@sentry/types';
import { type replayIntegration } from '@sentry/replay';
import type { BaseTransportOptions, Client, ClientOptions, Integration, IntegrationFn } from '@sentry/types';
import { isBrowser, logger } from '@sentry/utils';

import {
Expand Down Expand Up @@ -79,17 +80,18 @@ export class Feedback implements Integration {
private _hasInsertedActorStyles: boolean;

public constructor({
autoInject = true,
id = 'sentry-feedback',
includeReplay = true,
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 +125,10 @@ export class Feedback implements Integration {
this._hasInsertedActorStyles = false;

this.options = {
id,
showBranding,
autoInject,
showBranding,
id,
includeReplay,
isEmailRequired,
isNameRequired,
showEmail,
Expand Down Expand Up @@ -185,6 +188,29 @@ export class Feedback implements Integration {
}
}

/**
* Hook that is called after all integrations are setup. This is used to
* integrate with the Replay integration to save a replay when Feedback modal
* is opened.
*/
public afterAllSetup(client: Client<ClientOptions<BaseTransportOptions>>): void {
if (!this.options.includeReplay) {
return;
}

const replay = client.getIntegrationByName<ReturnType<typeof replayIntegration>>('Replay');

if (!replay) {
return;
}

try {
replay.startBuffering();
} catch (err) {
DEBUG_BUILD && logger.error(err);
}
}

/**
* Allows user to open the dialog box. Creates a new widget if
* `autoInject` was false, otherwise re-uses the default widget that was
Expand Down
5 changes: 5 additions & 0 deletions packages/feedback/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ export interface FeedbackGeneralConfiguration {
*/
showName: boolean;

/**
* Should Feedback attempt to include a Session Replay if the integration is installed?
*/
includeReplay: boolean;

/**
* Fill in email/name input fields with Sentry user context if it exists.
* The value of the email/name keys represent the properties of your user context.
Expand Down

0 comments on commit c45d03f

Please sign in to comment.