From ac9261bf2ee05f47ce829c76f2784e52540aa2ed Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 20 May 2025 11:37:46 +0200 Subject: [PATCH 1/2] ref(browser): Streamline `showReportDialog` code Just a small refactor, noticed this while working on other browser sdk stuff. Moving this into a dedicated file decouples this from the core SDK functionality, which this IMHO is not (anymore). Also, we can streamline the code slightly for efficiency. --- packages/browser/src/exports.ts | 3 +- packages/browser/src/report-dialog.ts | 64 ++++++++++++++++++++++++ packages/browser/src/sdk.ts | 71 +-------------------------- 3 files changed, 67 insertions(+), 71 deletions(-) create mode 100644 packages/browser/src/report-dialog.ts diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 8745e34106f4..b454f29556c2 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -84,7 +84,8 @@ export { } from './stack-parsers'; export { eventFromException, eventFromMessage, exceptionFromError } from './eventbuilder'; export { createUserFeedbackEnvelope } from './userfeedback'; -export { getDefaultIntegrations, forceLoad, init, onLoad, showReportDialog } from './sdk'; +export { getDefaultIntegrations, forceLoad, init, onLoad } from './sdk'; +export { showReportDialog } from './report-dialog'; export { breadcrumbsIntegration } from './integrations/breadcrumbs'; export { globalHandlersIntegration } from './integrations/globalhandlers'; diff --git a/packages/browser/src/report-dialog.ts b/packages/browser/src/report-dialog.ts new file mode 100644 index 000000000000..99b9cbd7733b --- /dev/null +++ b/packages/browser/src/report-dialog.ts @@ -0,0 +1,64 @@ +import type { ReportDialogOptions } from '@sentry/core'; +import { getClient, getCurrentScope, getReportDialogEndpoint, lastEventId, logger } from '@sentry/core'; +import { DEBUG_BUILD } from './debug-build'; +import { WINDOW } from './helpers'; + +/** + * Present the user with a report dialog. + * + * @param options Everything is optional, we try to fetch all info need from the current scope. + */ +export function showReportDialog(options: ReportDialogOptions = {}): void { + const optionalDocument = WINDOW.document as Document | undefined; + const injectionPoint = optionalDocument?.head || optionalDocument?.body; + + // doesn't work without a document (React Native) + if (!injectionPoint) { + DEBUG_BUILD && logger.error('[showReportDialog] Global document not defined'); + return; + } + + const scope = getCurrentScope(); + const client = getClient(); + const dsn = client?.getDsn(); + + if (!dsn) { + DEBUG_BUILD && logger.error('[showReportDialog] DSN not configured'); + return; + } + + const mergedOptions = { + ...options, + user: { + ...scope.getUser(), + ...options.user, + }, + eventId: options.eventId || lastEventId(), + }; + + const script = WINDOW.document.createElement('script'); + script.async = true; + script.crossOrigin = 'anonymous'; + script.src = getReportDialogEndpoint(dsn, mergedOptions); + + const { onLoad, onClose } = mergedOptions; + + if (onLoad) { + script.onload = onLoad; + } + + if (onClose) { + const reportDialogClosedMessageHandler = (event: MessageEvent): void => { + if (event.data === '__sentry_reportdialog_closed__') { + try { + onClose(); + } finally { + WINDOW.removeEventListener('message', reportDialogClosedMessageHandler); + } + } + }; + WINDOW.addEventListener('message', reportDialogClosedMessageHandler); + } + + injectionPoint.appendChild(script); +} diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 1c7e6fbe95ad..f3c8be4b3f40 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -1,15 +1,12 @@ -import type { Client, Integration, Options, ReportDialogOptions } from '@sentry/core'; +import type { Client, Integration, Options } from '@sentry/core'; import { consoleSandbox, dedupeIntegration, functionToStringIntegration, - getCurrentScope, getIntegrationsToSetup, getLocationHref, - getReportDialogEndpoint, inboundFiltersIntegration, initAndBind, - lastEventId, logger, stackParserFromStackParserOptions, supportsFetch, @@ -201,72 +198,6 @@ export function init(browserOptions: BrowserOptions = {}): Client | undefined { return initAndBind(BrowserClient, clientOptions); } -/** - * Present the user with a report dialog. - * - * @param options Everything is optional, we try to fetch all info need from the global scope. - */ -export function showReportDialog(options: ReportDialogOptions = {}): void { - // doesn't work without a document (React Native) - if (!WINDOW.document) { - DEBUG_BUILD && logger.error('Global document not defined in showReportDialog call'); - return; - } - - const scope = getCurrentScope(); - const client = scope.getClient(); - const dsn = client?.getDsn(); - - if (!dsn) { - DEBUG_BUILD && logger.error('DSN not configured for showReportDialog call'); - return; - } - - if (scope) { - options.user = { - ...scope.getUser(), - ...options.user, - }; - } - - if (!options.eventId) { - const eventId = lastEventId(); - if (eventId) { - options.eventId = eventId; - } - } - - const script = WINDOW.document.createElement('script'); - script.async = true; - script.crossOrigin = 'anonymous'; - script.src = getReportDialogEndpoint(dsn, options); - - if (options.onLoad) { - script.onload = options.onLoad; - } - - const { onClose } = options; - if (onClose) { - const reportDialogClosedMessageHandler = (event: MessageEvent): void => { - if (event.data === '__sentry_reportdialog_closed__') { - try { - onClose(); - } finally { - WINDOW.removeEventListener('message', reportDialogClosedMessageHandler); - } - } - }; - WINDOW.addEventListener('message', reportDialogClosedMessageHandler); - } - - const injectionPoint = WINDOW.document.head || WINDOW.document.body; - if (injectionPoint) { - injectionPoint.appendChild(script); - } else { - DEBUG_BUILD && logger.error('Not injecting report dialog. No injection point found in HTML'); - } -} - /** * This function is here to be API compatible with the loader. * @hidden From 3cb4910075409807089d16e305172f60bc0970b6 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 20 May 2025 11:47:55 +0200 Subject: [PATCH 2/2] suppress warnings in test --- .../browser/test/tracing/browserTracingIntegration.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index b3686272d12e..0b71fcc01383 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -71,6 +71,9 @@ describe('browserTracingIntegration', () => { getIsolationScope().clear(); getCurrentScope().setClient(undefined); document.head.innerHTML = ''; + + // We want to suppress the "Multiple browserTracingIntegration instances are not supported." warnings + vi.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => {