Skip to content

Commit 378cccb

Browse files
andreiborzaclaude
andauthored
feat(browser): Add onError callback to showReportDialog (#24780)
## What `showReportDialog` accepts a new `onError` callback. The SDK calls it when the dialog cannot be shown: the dialog script fails to load, or there is no event ID. ## Why Ad blockers or network problems can block the dialog script, and there was no way to detect it. Apps can now tell users why the dialog did not open. Closes: #24765 --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent e417276 commit 378cccb

7 files changed

Lines changed: 97 additions & 8 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Sentry.showReportDialog({
2+
eventId: 'test_id',
3+
onError: error => {
4+
window._reportDialogError = error.message;
5+
},
6+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
4+
sentryTest('calls `onError` when the dialog script is blocked', async ({ getLocalTestUrl, page }) => {
5+
const url = await getLocalTestUrl({ testDir: __dirname });
6+
7+
// Registered after `getLocalTestUrl` so it takes precedence over the fixture's ingest route
8+
await page.route(/\/api\/embed\/error-page\//, route => route.abort('blockedbyclient'));
9+
10+
await page.goto(url);
11+
12+
const errorMessage = await page.waitForFunction(() => (window as any)._reportDialogError);
13+
14+
expect(await errorMessage.jsonValue()).toBe('Failed to load the report dialog script');
15+
});

‎packages/browser/src/report-dialog.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,30 @@ export function showReportDialog(options: ReportDialogOptions = {}): void {
3636
eventId: options.eventId || lastEventId(),
3737
};
3838

39+
const { onLoad, onClose, onError } = mergedOptions;
40+
41+
// The endpoint rejects requests without an event ID, and a failed script load hides the reason
42+
if (!mergedOptions.eventId) {
43+
DEBUG_BUILD && debug.error('[showReportDialog] No event ID');
44+
onError?.(new Error('No event ID to show the report dialog for'));
45+
return;
46+
}
47+
3948
const script = WINDOW.document.createElement('script');
4049
script.async = true;
4150
script.crossOrigin = 'anonymous';
4251
script.src = getReportDialogEndpoint(dsn, mergedOptions);
4352

44-
const { onLoad, onClose } = mergedOptions;
45-
4653
if (onLoad) {
4754
script.onload = onLoad;
4855
}
4956

57+
if (onError) {
58+
script.onerror = () => {
59+
onError(new Error('Failed to load the report dialog script'));
60+
};
61+
}
62+
5063
if (onClose) {
5164
const reportDialogClosedMessageHandler = (event: MessageEvent): void => {
5265
if (event.data === '__sentry_reportdialog_closed__') {
@@ -58,6 +71,10 @@ export function showReportDialog(options: ReportDialogOptions = {}): void {
5871
}
5972
};
6073
WINDOW.addEventListener('message', reportDialogClosedMessageHandler);
74+
// A dialog that never loads never closes, so the listener would stay forever
75+
script.addEventListener('error', () => {
76+
WINDOW.removeEventListener('message', reportDialogClosedMessageHandler);
77+
});
6178
}
6279

6380
injectionPoint.appendChild(script);

‎packages/browser/test/index.test.ts‎

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe('SentryBrowser', () => {
9797
getCurrentScope().setUser(EX_USER);
9898
setCurrentClient(client);
9999

100-
showReportDialog();
100+
showReportDialog({ eventId: 'foobar' });
101101

102102
expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
103103
expect(getReportDialogEndpoint).toHaveBeenCalledWith(
@@ -136,7 +136,7 @@ describe('SentryBrowser', () => {
136136
setCurrentClient(client);
137137

138138
const DIALOG_OPTION_USER = { email: 'option@example.com' };
139-
showReportDialog({ user: DIALOG_OPTION_USER });
139+
showReportDialog({ eventId: 'foobar', user: DIALOG_OPTION_USER });
140140

141141
expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
142142
expect(getReportDialogEndpoint).toHaveBeenCalledWith(
@@ -170,7 +170,7 @@ describe('SentryBrowser', () => {
170170
it('should call `onClose` when receiving `__sentry_reportdialog_closed__` MessageEvent', async () => {
171171
const onClose = vi.fn();
172172

173-
showReportDialog({ onClose });
173+
showReportDialog({ eventId: 'foobar', onClose });
174174

175175
await waitForPostMessage('__sentry_reportdialog_closed__');
176176
expect(onClose).toHaveBeenCalledTimes(1);
@@ -185,7 +185,7 @@ describe('SentryBrowser', () => {
185185
throw new Error();
186186
});
187187

188-
showReportDialog({ onClose });
188+
showReportDialog({ eventId: 'foobar', onClose });
189189

190190
await waitForPostMessage('__sentry_reportdialog_closed__');
191191
expect(onClose).toHaveBeenCalledTimes(1);
@@ -198,14 +198,53 @@ describe('SentryBrowser', () => {
198198
it('should not call `onClose` for other MessageEvents', async () => {
199199
const onClose = vi.fn();
200200

201-
showReportDialog({ onClose });
201+
showReportDialog({ eventId: 'foobar', onClose });
202202

203203
await waitForPostMessage('some_message');
204204
expect(onClose).not.toHaveBeenCalled();
205205

206206
await waitForPostMessage('__sentry_reportdialog_closed__');
207207
expect(onClose).toHaveBeenCalledTimes(1);
208208
});
209+
210+
it('should remove the `onClose` listener when the script fails to load', async () => {
211+
const onClose = vi.fn();
212+
213+
showReportDialog({ eventId: 'foobar', onClose });
214+
215+
const script = WINDOW.document.head.lastElementChild as HTMLScriptElement;
216+
script.dispatchEvent(new Event('error'));
217+
218+
await waitForPostMessage('__sentry_reportdialog_closed__');
219+
expect(onClose).not.toHaveBeenCalled();
220+
});
221+
});
222+
223+
describe('onError', () => {
224+
it('should call `onError` when the script fails to load', () => {
225+
const onError = vi.fn();
226+
227+
showReportDialog({ eventId: 'foobar', onError });
228+
229+
const script = WINDOW.document.head.lastElementChild as HTMLScriptElement;
230+
script.dispatchEvent(new Event('error'));
231+
232+
expect(onError).toHaveBeenCalledTimes(1);
233+
expect(onError).toHaveBeenCalledWith(new Error('Failed to load the report dialog script'));
234+
});
235+
236+
it('should call `onError` and not inject the script without an event ID', () => {
237+
const onError = vi.fn();
238+
const appendChildSpy = vi.spyOn(WINDOW.document.head, 'appendChild');
239+
240+
showReportDialog({ onError });
241+
242+
expect(onError).toHaveBeenCalledTimes(1);
243+
expect(onError).toHaveBeenCalledWith(new Error('No event ID to show the report dialog for'));
244+
expect(appendChildSpy).not.toHaveBeenCalled();
245+
246+
appendChildSpy.mockRestore();
247+
});
209248
});
210249
});
211250

‎packages/core/src/api.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export function getReportDialogEndpoint(dsnLike: DsnLike, dialogOptions: ReportD
6060
continue;
6161
}
6262

63-
if (key === 'onClose') {
63+
if (key === 'onClose' || key === 'onError') {
6464
continue;
6565
}
6666

‎packages/core/src/report-dialog.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,10 @@ export interface ReportDialogOptions extends Record<string, unknown> {
2626
onLoad?(this: void): void;
2727
/** Callback after reportDialog closed */
2828
onClose?(this: void): void;
29+
/**
30+
* Callback if the reportDialog cannot be shown. This happens when:
31+
* - there is no event ID (no `eventId` option and no event captured yet)
32+
* - the dialog script fails to load (e.g. blocked by an ad blocker or a network error)
33+
*/
34+
onError?(this: void, error: Error): void;
2935
}

‎packages/core/test/lib/api.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@ describe('API', () => {
123123
{ onClose: () => {} },
124124
'https://sentry.io:1234/subpath/api/embed/error-page/?dsn=https://abc@sentry.io:1234/subpath/123',
125125
],
126+
[
127+
'with Public DSN and onError callback',
128+
dsnPublic,
129+
{ onError: () => {} },
130+
'https://sentry.io:1234/subpath/api/embed/error-page/?dsn=https://abc@sentry.io:1234/subpath/123',
131+
],
126132
])(
127133
'%s',
128134
(

0 commit comments

Comments
 (0)