Skip to content

ref(browser): Remove showReportDialog on browser client #4973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';

import { eventFromException, eventFromMessage } from './eventbuilder';
import { IS_DEBUG_BUILD } from './flags';
import { injectReportDialog, ReportDialogOptions } from './helpers';
import { Breadcrumbs } from './integrations';

export interface BaseBrowserOptions {
Expand Down Expand Up @@ -62,29 +59,6 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
super(options);
}

/**
* Show a report dialog to the user to send feedback to a specific event.
*
* @param options Set individual options for the dialog
*/
public showReportDialog(options: ReportDialogOptions = {}): void {
// doesn't work without a document (React Native)
const document = getGlobalObject<Window>().document;
if (!document) {
return;
}

if (!this._isEnabled()) {
IS_DEBUG_BUILD && logger.error('Trying to call showReportDialog with Sentry Client disabled');
return;
}

injectReportDialog({
...options,
dsn: options.dsn || this.getDsn(),
});
}

/**
* @inheritDoc
*/
Expand Down
1 change: 0 additions & 1 deletion packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,5 @@ export {
opera11StackParser,
winjsStackParser,
} from './stack-parsers';
export { injectReportDialog } from './helpers';
export { defaultIntegrations, forceLoad, init, lastEventId, onLoad, showReportDialog, flush, close, wrap } from './sdk';
export { SDK_NAME } from './version';
42 changes: 1 addition & 41 deletions packages/browser/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import { captureException, getReportDialogEndpoint, withScope } from '@sentry/core';
import { captureException, withScope } from '@sentry/core';
import { DsnLike, Event as SentryEvent, Mechanism, Scope, WrappedFunction } from '@sentry/types';
import {
addExceptionMechanism,
addExceptionTypeValue,
addNonEnumerableProperty,
getGlobalObject,
getOriginalFunction,
logger,
markFunctionWrapped,
} from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';

const global = getGlobalObject<Window>();
let ignoreOnError: number = 0;

/**
Expand Down Expand Up @@ -182,38 +177,3 @@ export interface ReportDialogOptions {
/** Callback after reportDialog showed up */
onLoad?(): void;
}

/**
* Injects the Report Dialog script
* @hidden
*/
export function injectReportDialog(options: ReportDialogOptions = {}): void {
if (!global.document) {
return;
}

if (!options.eventId) {
IS_DEBUG_BUILD && logger.error('Missing eventId option in showReportDialog call');
return;
}

if (!options.dsn) {
IS_DEBUG_BUILD && logger.error('Missing dsn option in showReportDialog call');
return;
}

const script = global.document.createElement('script');
script.async = true;
script.src = getReportDialogEndpoint(options.dsn, options);

if (options.onLoad) {
// eslint-disable-next-line @typescript-eslint/unbound-method
script.onload = options.onLoad;
}

const injectionPoint = global.document.head || global.document.body;

if (injectionPoint) {
injectionPoint.appendChild(script);
}
}
46 changes: 38 additions & 8 deletions packages/browser/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { getCurrentHub, getIntegrationsToSetup, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
import { Hub } from '@sentry/types';
import {
getCurrentHub,
getIntegrationsToSetup,
getReportDialogEndpoint,
Hub,
initAndBind,
Integrations as CoreIntegrations,
} from '@sentry/core';
import {
addInstrumentationHandler,
getGlobalObject,
Expand Down Expand Up @@ -121,9 +127,21 @@ export function init(options: BrowserOptions = {}): void {
*
* @param options Everything is optional, we try to fetch all info need from the global scope.
*/
export function showReportDialog(options: ReportDialogOptions = {}): void {
const hub = getCurrentHub();
const scope = hub.getScope();
export function showReportDialog(options: ReportDialogOptions = {}, hub: Hub = getCurrentHub()): void {
// doesn't work without a document (React Native)
const global = getGlobalObject<Window>();
if (!global.document) {
IS_DEBUG_BUILD && logger.error('Global document not defined in showReportDialog call');
return;
}

const { client, scope } = hub.getStackTop();
const dsn = options.dsn || (client && client.getDsn());
if (!dsn) {
IS_DEBUG_BUILD && logger.error('DSN not configured for showReportDialog call');
return;
}

if (scope) {
options.user = {
...scope.getUser(),
Expand All @@ -134,9 +152,21 @@ export function showReportDialog(options: ReportDialogOptions = {}): void {
if (!options.eventId) {
options.eventId = hub.lastEventId();
}
const client = hub.getClient<BrowserClient>();
if (client) {
client.showReportDialog(options);

const script = global.document.createElement('script');
script.async = true;
script.src = getReportDialogEndpoint(dsn, options);

if (options.onLoad) {
// eslint-disable-next-line @typescript-eslint/unbound-method
script.onload = options.onLoad;
}

const injectionPoint = global.document.head || global.document.body;
if (injectionPoint) {
injectionPoint.appendChild(script);
} else {
IS_DEBUG_BUILD && logger.error('Not injecting report dialog. No injection point found in HTML');
}
}

Expand Down
34 changes: 23 additions & 11 deletions packages/browser/test/unit/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SDK_VERSION } from '@sentry/core';
import { getReportDialogEndpoint, SDK_VERSION } from '@sentry/core';

import {
addBreadcrumb,
Expand All @@ -24,6 +24,14 @@ const dsn = 'https://[email protected]/4291';
// eslint-disable-next-line no-var
declare var global: any;

jest.mock('@sentry/core', () => {
const original = jest.requireActual('@sentry/core');
return {
...original,
getReportDialogEndpoint: jest.fn(),
};
});

describe('SentryBrowser', () => {
const beforeSend = jest.fn();

Expand Down Expand Up @@ -74,16 +82,14 @@ describe('SentryBrowser', () => {
});

describe('showReportDialog', () => {
beforeEach(() => {
(getReportDialogEndpoint as jest.Mock).mockReset();
});

describe('user', () => {
const EX_USER = { email: '[email protected]' };
const options = getDefaultBrowserClientOptions({ dsn });
const client = new BrowserClient(options);
const reportDialogSpy = jest.spyOn(client, 'showReportDialog');

beforeEach(() => {
reportDialogSpy.mockReset();
});

it('uses the user on the scope', () => {
configureScope(scope => {
scope.setUser(EX_USER);
Expand All @@ -92,8 +98,11 @@ describe('SentryBrowser', () => {

showReportDialog();

expect(reportDialogSpy).toBeCalled();
expect(reportDialogSpy.mock.calls[0][0]!.user!.email).toBe(EX_USER.email);
expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
expect(getReportDialogEndpoint).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({ user: { email: EX_USER.email } }),
);
});

it('prioritizes options user over scope user', () => {
Expand All @@ -105,8 +114,11 @@ describe('SentryBrowser', () => {
const DIALOG_OPTION_USER = { email: '[email protected]' };
showReportDialog({ user: DIALOG_OPTION_USER });

expect(reportDialogSpy).toBeCalled();
expect(reportDialogSpy.mock.calls[0][0]!.user!.email).toBe(DIALOG_OPTION_USER.email);
expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
expect(getReportDialogEndpoint).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({ user: { email: DIALOG_OPTION_USER.email } }),
);
});
});
});
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ export function getReportDialogEndpoint(
}

if (key === 'user') {
if (!dialogOptions.user) {
const user = dialogOptions.user;
if (!user) {
continue;
}
if (dialogOptions.user.name) {
encodedOptions += `&name=${encodeURIComponent(dialogOptions.user.name)}`;
if (user.name) {
encodedOptions += `&name=${encodeURIComponent(user.name)}`;
}
if (dialogOptions.user.email) {
encodedOptions += `&email=${encodeURIComponent(dialogOptions.user.email)}`;
if (user.email) {
encodedOptions += `&email=${encodeURIComponent(user.email)}`;
}
} else {
encodedOptions += `&${encodeURIComponent(key)}=${encodeURIComponent(dialogOptions[key] as string)}`;
Expand Down