diff --git a/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/subject.js b/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/subject.js new file mode 100644 index 000000000000..d547113b8f5e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/subject.js @@ -0,0 +1,6 @@ +Sentry.showReportDialog({ + eventId: 'test_id', + onError: error => { + window._reportDialogError = error.message; + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/test.ts b/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/test.ts new file mode 100644 index 000000000000..d0d94225c375 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/showReportDialog/on-error/test.ts @@ -0,0 +1,15 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; + +sentryTest('calls `onError` when the dialog script is blocked', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + // Registered after `getLocalTestUrl` so it takes precedence over the fixture's ingest route + await page.route(/\/api\/embed\/error-page\//, route => route.abort('blockedbyclient')); + + await page.goto(url); + + const errorMessage = await page.waitForFunction(() => (window as any)._reportDialogError); + + expect(await errorMessage.jsonValue()).toBe('Failed to load the report dialog script'); +}); diff --git a/packages/browser/src/report-dialog.ts b/packages/browser/src/report-dialog.ts index 03255a7db91d..7abf68f5ea18 100644 --- a/packages/browser/src/report-dialog.ts +++ b/packages/browser/src/report-dialog.ts @@ -36,17 +36,30 @@ export function showReportDialog(options: ReportDialogOptions = {}): void { eventId: options.eventId || lastEventId(), }; + const { onLoad, onClose, onError } = mergedOptions; + + // The endpoint rejects requests without an event ID, and a failed script load hides the reason + if (!mergedOptions.eventId) { + DEBUG_BUILD && debug.error('[showReportDialog] No event ID'); + onError?.(new Error('No event ID to show the report dialog for')); + return; + } + 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 (onError) { + script.onerror = () => { + onError(new Error('Failed to load the report dialog script')); + }; + } + if (onClose) { const reportDialogClosedMessageHandler = (event: MessageEvent): void => { if (event.data === '__sentry_reportdialog_closed__') { @@ -58,6 +71,10 @@ export function showReportDialog(options: ReportDialogOptions = {}): void { } }; WINDOW.addEventListener('message', reportDialogClosedMessageHandler); + // A dialog that never loads never closes, so the listener would stay forever + script.addEventListener('error', () => { + WINDOW.removeEventListener('message', reportDialogClosedMessageHandler); + }); } injectionPoint.appendChild(script); diff --git a/packages/browser/test/index.test.ts b/packages/browser/test/index.test.ts index 4a0986048cf1..e9836c90308b 100644 --- a/packages/browser/test/index.test.ts +++ b/packages/browser/test/index.test.ts @@ -97,7 +97,7 @@ describe('SentryBrowser', () => { getCurrentScope().setUser(EX_USER); setCurrentClient(client); - showReportDialog(); + showReportDialog({ eventId: 'foobar' }); expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1); expect(getReportDialogEndpoint).toHaveBeenCalledWith( @@ -136,7 +136,7 @@ describe('SentryBrowser', () => { setCurrentClient(client); const DIALOG_OPTION_USER = { email: 'option@example.com' }; - showReportDialog({ user: DIALOG_OPTION_USER }); + showReportDialog({ eventId: 'foobar', user: DIALOG_OPTION_USER }); expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1); expect(getReportDialogEndpoint).toHaveBeenCalledWith( @@ -170,7 +170,7 @@ describe('SentryBrowser', () => { it('should call `onClose` when receiving `__sentry_reportdialog_closed__` MessageEvent', async () => { const onClose = vi.fn(); - showReportDialog({ onClose }); + showReportDialog({ eventId: 'foobar', onClose }); await waitForPostMessage('__sentry_reportdialog_closed__'); expect(onClose).toHaveBeenCalledTimes(1); @@ -185,7 +185,7 @@ describe('SentryBrowser', () => { throw new Error(); }); - showReportDialog({ onClose }); + showReportDialog({ eventId: 'foobar', onClose }); await waitForPostMessage('__sentry_reportdialog_closed__'); expect(onClose).toHaveBeenCalledTimes(1); @@ -198,7 +198,7 @@ describe('SentryBrowser', () => { it('should not call `onClose` for other MessageEvents', async () => { const onClose = vi.fn(); - showReportDialog({ onClose }); + showReportDialog({ eventId: 'foobar', onClose }); await waitForPostMessage('some_message'); expect(onClose).not.toHaveBeenCalled(); @@ -206,6 +206,45 @@ describe('SentryBrowser', () => { await waitForPostMessage('__sentry_reportdialog_closed__'); expect(onClose).toHaveBeenCalledTimes(1); }); + + it('should remove the `onClose` listener when the script fails to load', async () => { + const onClose = vi.fn(); + + showReportDialog({ eventId: 'foobar', onClose }); + + const script = WINDOW.document.head.lastElementChild as HTMLScriptElement; + script.dispatchEvent(new Event('error')); + + await waitForPostMessage('__sentry_reportdialog_closed__'); + expect(onClose).not.toHaveBeenCalled(); + }); + }); + + describe('onError', () => { + it('should call `onError` when the script fails to load', () => { + const onError = vi.fn(); + + showReportDialog({ eventId: 'foobar', onError }); + + const script = WINDOW.document.head.lastElementChild as HTMLScriptElement; + script.dispatchEvent(new Event('error')); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith(new Error('Failed to load the report dialog script')); + }); + + it('should call `onError` and not inject the script without an event ID', () => { + const onError = vi.fn(); + const appendChildSpy = vi.spyOn(WINDOW.document.head, 'appendChild'); + + showReportDialog({ onError }); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith(new Error('No event ID to show the report dialog for')); + expect(appendChildSpy).not.toHaveBeenCalled(); + + appendChildSpy.mockRestore(); + }); }); }); diff --git a/packages/core/src/api.ts b/packages/core/src/api.ts index 842e6f35be2d..a6610a9c73ba 100644 --- a/packages/core/src/api.ts +++ b/packages/core/src/api.ts @@ -60,7 +60,7 @@ export function getReportDialogEndpoint(dsnLike: DsnLike, dialogOptions: ReportD continue; } - if (key === 'onClose') { + if (key === 'onClose' || key === 'onError') { continue; } diff --git a/packages/core/src/report-dialog.ts b/packages/core/src/report-dialog.ts index a679604e5d0b..30c330baeef6 100644 --- a/packages/core/src/report-dialog.ts +++ b/packages/core/src/report-dialog.ts @@ -26,4 +26,10 @@ export interface ReportDialogOptions extends Record { onLoad?(this: void): void; /** Callback after reportDialog closed */ onClose?(this: void): void; + /** + * Callback if the reportDialog cannot be shown. This happens when: + * - there is no event ID (no `eventId` option and no event captured yet) + * - the dialog script fails to load (e.g. blocked by an ad blocker or a network error) + */ + onError?(this: void, error: Error): void; } diff --git a/packages/core/test/lib/api.test.ts b/packages/core/test/lib/api.test.ts index 879257eb20c1..866b50d3de30 100644 --- a/packages/core/test/lib/api.test.ts +++ b/packages/core/test/lib/api.test.ts @@ -123,6 +123,12 @@ describe('API', () => { { onClose: () => {} }, 'https://sentry.io:1234/subpath/api/embed/error-page/?dsn=https://abc@sentry.io:1234/subpath/123', ], + [ + 'with Public DSN and onError callback', + dsnPublic, + { onError: () => {} }, + 'https://sentry.io:1234/subpath/api/embed/error-page/?dsn=https://abc@sentry.io:1234/subpath/123', + ], ])( '%s', (