fix(core): Prevent duplicate feedback submissions and keep draft on error

CHANGELOG.md

@@ -20,6 +20,7 @@
 - Android Gradle plugin no longer writes generated `sentry.options.json` into your source tree during builds ([#6751](https://github.com/getsentry/sentry-react-native/pull/6751))
 - Android Gradle plugin no longer writes generated `modules.json` into your source tree during release builds ([#6753](https://github.com/getsentry/sentry-react-native/pull/6753))
 - Fix Mac Catalyst linking the wrong `Sentry.xcframework` slice ([#6758](https://github.com/getsentry/sentry-react-native/pull/6758))
+- `FeedbackForm` prevents duplicate submissions and keeps the draft on error ([#6769](https://github.com/getsentry/sentry-react-native/pull/6769))
 
 ### Internal
 

packages/core/etc/sentry-react-native.api.md

@@ -471,6 +471,8 @@ export interface FeedbackFormStyles {
     // (undocumented)
     submitButton?: ViewStyle;
     // (undocumented)
+    submitButtonDisabled?: ViewStyle;
+    // (undocumented)
     submitText?: TextStyle;
     // (undocumented)
     takeScreenshotButton?: ViewStyle;

packages/core/src/js/feedback/FeedbackForm.styles.ts

@@ -84,6 +84,9 @@ const defaultStyles = (theme: FeedbackFormTheme): FeedbackFormStyles => {
       alignItems: 'center',
       marginBottom: 10,
     },
+    submitButtonDisabled: {
+      opacity: 0.7,
+    },
     submitText: {
       color: theme.accentForeground,
       fontSize: 18,

packages/core/src/js/feedback/FeedbackForm.tsx

@@ -2,7 +2,15 @@
 import type { SendFeedbackParams, User } from '@sentry/core';
 import type { KeyboardTypeOptions, NativeEventSubscription } from 'react-native';
 
-import { captureFeedback, debug, getCurrentScope, getGlobalScope, getIsolationScope, lastEventId } from '@sentry/core';
+import {
+  captureFeedback,
+  debug,
+  getClient,
+  getCurrentScope,
+  getGlobalScope,
+  getIsolationScope,
+  lastEventId,
+} from '@sentry/core';
 import * as React from 'react';
 import {
   Appearance,
@@ -43,7 +51,7 @@ import { base64ToUint8Array, feedbackAlertDialog, isValidEmail } from './utils';
 export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFormState> {
   public static defaultProps = defaultConfiguration;
 
-  private static _savedState: Omit<FeedbackFormState, 'isVisible'> = {
+  private static _savedState: Omit<FeedbackFormState, 'isVisible' | 'isSubmitting'> = {
     name: '',
     email: '',
     description: '',
@@ -56,6 +64,11 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
 
   private _didSubmitForm: boolean = false;
 
+  // Synchronous guard against duplicate submissions. State can't be used for this because
+  // `setState` doesn't update `this.state` until the next render, so two taps in the same
+  // tick would both pass a state-based check.
+  private _isSubmitting: boolean = false;
+
   public constructor(props: FeedbackFormProps) {
     super(props);
 
@@ -68,6 +81,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
 
     this.state = {
       isVisible: true,
+      isSubmitting: false,
       name: FeedbackForm._savedState.name || currentUser.useSentryUser.name,
       email: FeedbackForm._savedState.email || currentUser.useSentryUser.email,
       description: FeedbackForm._savedState.description || '',
@@ -95,9 +109,15 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
 
   public handleFeedbackSubmit: () => void = () => {
     const { name, email, description } = this.state;
-    const { onSubmitSuccess, onSubmitError, onFormSubmitted } = this.props;
     const text = this.props;
 
+    // Ignore repeated taps: while a submission is running, and after a successful submit
+    // (a custom `onFormSubmitted` may keep the form mounted). Uses a synchronous flag so
+    // two taps in the same tick can't both get through.
+    if (this._isSubmitting) {
+      return;
+    }
+
     const trimmedName = name?.trim();
     const trimmedEmail = email?.trim();
     const trimmedDescription = description?.trim();
@@ -138,25 +158,75 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
       associatedEventId: eventId,
     };
 
+    this._isSubmitting = true;
+    this.setState({ isSubmitting: true });
+    this._submitFeedback(userFeedback, attachments);
+  };
+
+  /**
+   * Captures the feedback and reports the outcome.
+   *
+   * `captureFeedback` is synchronous and fire-and-forget — the React Native client hands the
+   * envelope to the (native or JS) transport without surfacing the delivery result, so the JS
+   * layer genuinely can't confirm the envelope reached Sentry. We therefore report success
+   * optimistically once the event has been captured, and only surface an error for the cases we
+   * can detect synchronously: no active client, or `captureFeedback` throwing. On those errors the
+   * form stays open with the draft intact so the user can retry.
+   */
+  private _submitFeedback = (
+    userFeedback: SendFeedbackParams,
+    attachments: Array<{ filename: string; data: string | Uint8Array }> | undefined,
+  ): void => {
+    const { onSubmitSuccess, onSubmitError, onFormSubmitted } = this.props;
+    const text = this.props;
+
     try {
+      if (!getClient()) {
+        throw new Error('No Sentry client is available to send the feedback.');
+      }
+
       if (!onFormSubmitted) {
         this.setState({ isVisible: false });
       }
+
       captureFeedback(userFeedback, attachments ? { attachments } : undefined);
-      onSubmitSuccess({
-        name: trimmedName,
-        email: trimmedEmail,
-        message: trimmedDescription,
-        attachments: attachments,
-      });
-      feedbackAlertDialog(text.successMessageText, '');
-      onFormSubmitted();
-      this._didSubmitForm = true;
     } catch (error) {
       const errorString = `Feedback form submission failed: ${error}`;
-      onSubmitError(new Error(errorString));
+      debug.error(errorString);
+      // Release the guard first so the user can retry even if `onSubmitError` throws.
+      this._isSubmitting = false;
+      this.setState({ isSubmitting: false });
+      this._runCallback(() => onSubmitError(new Error(errorString)));
       feedbackAlertDialog(text.errorTitle, text.genericError);
-      debug.error(`Feedback form submission failed: ${error}`);
+      return;
+    }
+
+    // The feedback was captured. Mark the form as submitted and keep `_isSubmitting` set so it
+    // can't be submitted again (even if a custom `onFormSubmitted` keeps it mounted). Each consumer
+    // callback is isolated so a throw in one can't undo the submission or skip the remaining success
+    // side-effects (the alert and closing the form).
+    this._didSubmitForm = true;
+    this._runCallback(() =>
+      onSubmitSuccess({
+        name: userFeedback.name ?? '',
+        email: userFeedback.email ?? '',
+        message: userFeedback.message,
+        attachments: attachments,
+      }),
+    );
+    feedbackAlertDialog(text.successMessageText, '');
+    this._runCallback(() => onFormSubmitted());
+  };
+
+  /**
+   * Runs a consumer-provided callback, isolating any thrown error so it can't disrupt the
+   * submission flow. The feedback has already been captured by the time these run.
+   */
+  private _runCallback = (callback: () => void): void => {
+    try {
+      callback();
+    } catch (error) {
+      debug.error(`Feedback form callback threw: ${error}`);
     }
   };
 
@@ -402,7 +472,11 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
               </Text>
             </TouchableOpacity>
           )}
-          <TouchableOpacity style={styles.submitButton} onPress={this.handleFeedbackSubmit}>
+          <TouchableOpacity
+            style={[styles.submitButton, this.state.isSubmitting && styles.submitButtonDisabled]}
+            onPress={this.handleFeedbackSubmit}
+            disabled={this.state.isSubmitting}
+          >
             <Text style={styles.submitText} testID="sentry-feedback-submit-button">
               {text.submitButtonLabel}
             </Text>

packages/core/src/js/feedback/FeedbackForm.types.ts

@@ -293,6 +293,7 @@ export interface FeedbackFormStyles {
   input?: TextStyle;
   textArea?: TextStyle;
   submitButton?: ViewStyle;
+  submitButtonDisabled?: ViewStyle;
   submitText?: TextStyle;
   cancelButton?: ViewStyle;
   cancelText?: TextStyle;
@@ -343,6 +344,11 @@ export interface ScreenshotButtonStyles {
  */
 export interface FeedbackFormState {
   isVisible: boolean;
+  /**
+   * Whether a submission is currently in flight (waiting for confirmation that the
+   * feedback was sent). Used to disable the submit button and prevent double submits.
+   */
+  isSubmitting: boolean;
   name: string;
   email: string;
   description: string;

packages/core/test/feedback/FeedbackForm.test.tsx

@@ -112,6 +112,10 @@ describe('FeedbackForm', () => {
   beforeEach(() => {
     mockIsolationScopeGetUser.mockReturnValue(undefined);
     mockGlobalScopeGetUser.mockReturnValue(undefined);
+    // An active client is required for a submission to be reported as successful.
+    const client = new TestClient(getDefaultTestClientOptions());
+    setCurrentClient(client);
+    client.init();
     FeedbackForm.reset();
   });
 
@@ -425,6 +429,125 @@ describe('FeedbackForm', () => {
     });
   });
 
+  it('reports an error and keeps the draft when there is no active Sentry client', async () => {
+    // @ts-expect-error - simulate the SDK not being initialized.
+    setCurrentClient(undefined);
+
+    const { getByPlaceholderText, getByText, unmount } = render(<FeedbackForm {...defaultProps} />);
+
+    fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), 'john.doe@example.com');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
+
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+
+    await waitFor(() => {
+      expect(mockOnSubmitError).toHaveBeenCalled();
+      expect(Alert.alert).toHaveBeenCalledWith(defaultProps.errorTitle, defaultProps.genericError);
+    });
+    expect(mockOnSubmitSuccess).not.toHaveBeenCalled();
+
+    // The draft is preserved so the user can retry.
+    unmount();
+    const { queryByPlaceholderText } = render(<FeedbackForm {...defaultProps} />);
+    expect(queryByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('John Doe');
+    expect(queryByPlaceholderText(defaultProps.messagePlaceholder).props.value).toBe('This is a feedback message.');
+  });
+
+  it('does not submit again after a successful submission', async () => {
+    const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);
+
+    fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), 'john.doe@example.com');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
+
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    // Repeated taps must not trigger additional submissions.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+
+    expect(captureFeedback).toHaveBeenCalledTimes(1);
+    expect(mockOnSubmitSuccess).toHaveBeenCalledTimes(1);
+  });
+
+  it('still shows success, closes the form, and blocks resubmit when onSubmitSuccess throws', async () => {
+    const throwingOnSubmitSuccess = jest.fn(() => {
+      throw new Error('callback error');
+    });
+    const { getByPlaceholderText, getByText } = render(
+      <FeedbackForm {...defaultProps} onSubmitSuccess={throwingOnSubmitSuccess} />,
+    );
+
+    fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), 'john.doe@example.com');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
+
+    // The feedback is captured, then the success callback throws.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    // A second tap must not capture again: the feedback was already submitted.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+
+    expect(captureFeedback).toHaveBeenCalledTimes(1);
+    expect(throwingOnSubmitSuccess).toHaveBeenCalledTimes(1);
+    // A throwing success callback must not be treated as a submission failure...
+    expect(mockOnSubmitError).not.toHaveBeenCalled();
+    // ...and must not skip the success alert or closing the form.
+    expect(Alert.alert).toHaveBeenCalledWith(defaultProps.successMessageText, '');
+    expect(mockOnFormSubmitted).toHaveBeenCalled();
+  });
+
+  it('allows retry when onSubmitError throws on a failed submission', async () => {
+    (captureFeedback as jest.Mock).mockImplementationOnce(() => {
+      throw new Error('capture error');
+    });
+    const throwingOnSubmitError = jest.fn(() => {
+      throw new Error('callback error');
+    });
+
+    const { getByPlaceholderText, getByText } = render(
+      <FeedbackForm {...defaultProps} onSubmitError={throwingOnSubmitError} />,
+    );
+
+    fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), 'john.doe@example.com');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
+
+    // First submission fails and its onSubmitError throws — the guard must still be released.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    expect(throwingOnSubmitError).toHaveBeenCalledTimes(1);
+
+    // Second submission goes through.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    await waitFor(() => {
+      expect(mockOnSubmitSuccess).toHaveBeenCalled();
+    });
+    expect(captureFeedback).toHaveBeenCalledTimes(2);
+  });
+
+  it('allows re-submitting after a failed submission', async () => {
+    (captureFeedback as jest.Mock).mockImplementationOnce(() => {
+      throw new Error('Test error');
+    });
+
+    const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);
+
+    fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), 'john.doe@example.com');
+    fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
+
+    // First submission fails and releases the in-flight guard.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    await waitFor(() => {
+      expect(mockOnSubmitError).toHaveBeenCalled();
+    });
+
+    // Second submission goes through.
+    fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    await waitFor(() => {
+      expect(mockOnSubmitSuccess).toHaveBeenCalled();
+    });
+    expect(captureFeedback).toHaveBeenCalledTimes(2);
+  });
+
   it('calls onAddScreenshot when the screenshot button is pressed and no image picker library is integrated', async () => {
     const { getByText } = render(<FeedbackForm {...defaultProps} enableScreenshot={true} />);
 
@@ -516,6 +639,10 @@ describe('FeedbackForm', () => {
     fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');
 
     fireEvent.press(getByText(defaultProps.submitButtonLabel));
+    await waitFor(() => {
+      expect(mockOnSubmitSuccess).toHaveBeenCalled();
+    });
+
     unmount();
     const { queryByPlaceholderText } = render(<FeedbackForm {...defaultProps} />);