ref(feedback): Make eventId optional and use lastEventId in repor…

…t dialog (#12029)
getsentry · May 14, 2024 · d5568d8 · d5568d8
1 parent 6f81262
commit d5568d8
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 9 deletions.
diff --git a/packages/angular/src/errorhandler.ts b/packages/angular/src/errorhandler.ts
@@ -14,7 +14,7 @@ import { runOutsideAngular } from './zone';
 export interface ErrorHandlerOptions {
   logErrors?: boolean;
   showDialog?: boolean;
-  dialogOptions?: Omit<ReportDialogOptions, 'eventId'>;
+  dialogOptions?: ReportDialogOptions;
   /**
    * Custom implementation of error extraction from the raw value captured by the Angular.
    * @param error Value captured by Angular's ErrorHandler provider

diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
@@ -6,6 +6,7 @@ import {
   getIntegrationsToSetup,
   getReportDialogEndpoint,
   initAndBind,
+  lastEventId,
   startSession,
 } from '@sentry/core';
 import type { DsnLike, Integration, Options, UserFeedback } from '@sentry/types';
@@ -168,7 +169,7 @@ export function init(browserOptions: BrowserOptions = {}): void {
 export interface ReportDialogOptions {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   [key: string]: any;
-  eventId: string;
+  eventId?: string;
   dsn?: DsnLike;
   user?: {
     email?: string;
@@ -197,7 +198,7 @@ export interface ReportDialogOptions {
  *
  * @param options Everything is optional, we try to fetch all info need from the global scope.
  */
-export function showReportDialog(options: ReportDialogOptions): void {
+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');
@@ -220,6 +221,13 @@ export function showReportDialog(options: ReportDialogOptions): void {
     };
   }
 
+  if (!options.eventId) {
+    const eventId = lastEventId();
+    if (eventId) {
+      options.eventId = eventId;
+    }
+  }
+
   const script = WINDOW.document.createElement('script');
   script.async = true;
   script.crossOrigin = 'anonymous';

diff --git a/packages/browser/test/unit/index.test.ts b/packages/browser/test/unit/index.test.ts
@@ -4,6 +4,7 @@ import {
   getIsolationScope,
   getReportDialogEndpoint,
   inboundFiltersIntegration,
+  lastEventId,
 } from '@sentry/core';
 import * as utils from '@sentry/utils';
 
@@ -94,7 +95,7 @@ describe('SentryBrowser', () => {
         getCurrentScope().setUser(EX_USER);
         setCurrentClient(client);
 
-        showReportDialog({ eventId: 'foobar' });
+        showReportDialog();
 
         expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
         expect(getReportDialogEndpoint).toHaveBeenCalledWith(
@@ -103,12 +104,37 @@ describe('SentryBrowser', () => {
         );
       });
 
+      it('uses `lastEventId` from isolation scope', async () => {
+        setCurrentClient(client);
+        const eventId = captureException(new Error('Some error'));
+        await flush(2000);
+
+        showReportDialog();
+
+        expect(eventId).toEqual(lastEventId());
+        expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
+        expect(getReportDialogEndpoint).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({ eventId }));
+      });
+
+      it('uses the passed in `eventId` over `lastEventId`', async () => {
+        setCurrentClient(client);
+        captureException(new Error('Some error'));
+        await flush(2000);
+
+        showReportDialog({ eventId: 'foobar' });
+        expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
+        expect(getReportDialogEndpoint).toHaveBeenCalledWith(
+          expect.any(Object),
+          expect.objectContaining({ eventId: 'foobar' }),
+        );
+      });
+
       it('prioritizes options user over scope user', () => {
         getCurrentScope().setUser(EX_USER);
         setCurrentClient(client);
 
         const DIALOG_OPTION_USER = { email: 'option@example.com' };
-        showReportDialog({ eventId: 'foobar', user: DIALOG_OPTION_USER });
+        showReportDialog({ user: DIALOG_OPTION_USER });
 
         expect(getReportDialogEndpoint).toHaveBeenCalledTimes(1);
         expect(getReportDialogEndpoint).toHaveBeenCalledWith(
@@ -142,7 +168,7 @@ describe('SentryBrowser', () => {
       it('should call `onClose` when receiving `__sentry_reportdialog_closed__` MessageEvent', async () => {
         const onClose = jest.fn();
 
-        showReportDialog({ eventId: 'foobar', onClose });
+        showReportDialog({ onClose });
 
         await waitForPostMessage('__sentry_reportdialog_closed__');
         expect(onClose).toHaveBeenCalledTimes(1);
@@ -157,7 +183,7 @@ describe('SentryBrowser', () => {
           throw new Error();
         });
 
-        showReportDialog({ eventId: 'foobar', onClose });
+        showReportDialog({ onClose });
 
         await waitForPostMessage('__sentry_reportdialog_closed__');
         expect(onClose).toHaveBeenCalledTimes(1);
@@ -170,7 +196,7 @@ describe('SentryBrowser', () => {
       it('should not call `onClose` for other MessageEvents', async () => {
         const onClose = jest.fn();
 
-        showReportDialog({ eventId: 'foobar', onClose });
+        showReportDialog({ onClose });
 
         await waitForPostMessage('some_message');
         expect(onClose).not.toHaveBeenCalled();

diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx
@@ -29,7 +29,7 @@ export type ErrorBoundaryProps = {
    * Options to be passed into the Sentry report dialog.
    * No-op if {@link showDialog} is false.
    */
-  dialogOptions?: Omit<ReportDialogOptions, 'eventId'> | undefined;
+  dialogOptions?: ReportDialogOptions | undefined;
   /**
    * A fallback component that gets rendered when the error boundary encounters an error.
    *