fix(feedback)!: always reject sendFeedback with an Error (#20475)

`sendFeedback` currently rejects with an `Error` in some paths and a raw
string in others. This normalizes all paths to reject with an `Error`,
which is the expected shape and easier to handle for consumers.

Breaking: intended for the next major.

Author: logaretm

MIGRATION.md

@@ -613,6 +613,22 @@ on casing, or use `tracePropagationTargets` in combination with a more specific
 As part of this, the `g` and `y` flags are ignored on `tracePropagationTargets` regular expressions. These flags made
 matching stateful via `lastIndex`, so a target like `/myApi\.com/g` previously matched only every other request.
 
+### `sendFeedback` rejects with an `Error`
+
+Affected SDKs: All SDKs running in the browser.
+
+`Sentry.sendFeedback()` now rejects with an `Error` in all cases. Previously it rejected with a plain string when the request timed out, was rejected with a 403, or otherwise failed to send, while the synchronous validation paths (empty message, no client configured) already threw an `Error`. The message text itself is unchanged, and is still customizable through the `errorMessages` hint, so read it off `error.message`:
+
+```js
+try {
+  await Sentry.sendFeedback({ message: 'Hello' });
+} catch (error) {
+  // v10: a string on send failures, an Error on validation failures
+  // v11: always an Error
+  console.log(error.message);
+}
+```
+
 ### Span attribute changes
 
 Affected SDKs: All SDKs.

packages/feedback/src/core/sendFeedback.ts

@@ -8,7 +8,7 @@ import type {
 } from '@sentry/core';
 import { captureFeedback, getClient, getCurrentScope, getLocationHref } from '@sentry/core';
 import { FEEDBACK_API_SOURCE } from '../constants';
-import { createFeedbackError, resolveFeedbackErrorMessage } from '../util/createFeedbackError';
+import { createFeedbackError } from '../util/createFeedbackError';
 
 /**
  * Public API to send a Feedback item to Sentry
@@ -47,7 +47,7 @@ export const sendFeedback: SendFeedback = (
     // After 30s, we want to clear anyhow
     const timeout = setTimeout(() => {
       cleanup();
-      reject(resolveFeedbackErrorMessage('ERROR_TIMEOUT', errorMessages));
+      reject(createFeedbackError('ERROR_TIMEOUT', errorMessages));
     }, 30_000);
 
     const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
@@ -64,10 +64,10 @@ export const sendFeedback: SendFeedback = (
       }
 
       if (response?.statusCode === 403) {
-        return reject(resolveFeedbackErrorMessage('ERROR_FORBIDDEN', errorMessages));
+        return reject(createFeedbackError('ERROR_FORBIDDEN', errorMessages));
       }
 
-      return reject(resolveFeedbackErrorMessage('ERROR_GENERIC', errorMessages));
+      return reject(createFeedbackError('ERROR_GENERIC', errorMessages));
     });
   });
 };

packages/feedback/src/modal/components/Form.tsx

@@ -131,7 +131,7 @@ export function Form({
           onSubmitSuccess(data, eventId);
         } catch (error) {
           DEBUG_BUILD && debug.error(error);
-          const err = error instanceof Error ? error : new Error(String(error));
+          const err = error as Error;
           setError(err.message);
           onSubmitError(err);
         }

packages/feedback/src/util/createFeedbackError.ts

@@ -15,7 +15,7 @@ const DEFAULT_MESSAGES: Record<FeedbackErrorCode, string> = {
   ERROR_GENERIC: ERROR_GENERIC_TEXT,
 };
 
-export function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
+function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
   return messages?.[code] ?? DEFAULT_MESSAGES[code];
 }
 

packages/feedback/test/core/sendFeedback.test.ts

@@ -33,6 +33,18 @@ describe('sendFeedback', () => {
     patchedDecoder && delete global.window.TextDecoder;
   });
 
+  // `sendFeedback` always signals failure with an `Error`, never a bare string. A `toThrow(text)`
+  // assertion alone also passes for a thrown/rejected string, so assert the shape explicitly too.
+  async function expectRejectsWithError(promise: Promise<unknown>, message: string): Promise<void> {
+    await expect(promise).rejects.toBeInstanceOf(Error);
+    await expect(promise).rejects.toThrow(message);
+  }
+
+  function expectThrowsWithError(fn: () => unknown, message: string): void {
+    expect(fn).toThrow(Error);
+    expect(fn).toThrow(message);
+  }
+
   it('sends feedback with minimal options', async () => {
     mockSdk();
     const mockTransport = vi.spyOn(getClient()!.getTransport()!, 'send');
@@ -269,7 +281,7 @@ describe('sendFeedback', () => {
 
   it('throws when message is empty', () => {
     mockSdk();
-    expect(() => sendFeedback({ message: '' })).toThrow('Unable to submit feedback with empty message');
+    expectThrowsWithError(() => sendFeedback({ message: '' }), 'Unable to submit feedback with empty message');
   });
 
   it('throws when no client is set up', async () => {
@@ -279,7 +291,7 @@ describe('sendFeedback', () => {
     getGlobalScope().setClient(undefined);
     getCurrentScope().setClient(undefined);
     getIsolationScope().setClient(undefined);
-    expect(() => sendFeedback({ message: 'mi' })).toThrow('No client setup, cannot send feedback.');
+    expectThrowsWithError(() => sendFeedback({ message: 'mi' }), 'No client setup, cannot send feedback.');
   });
 
   it('uses provided errorMessages overrides', async () => {
@@ -288,9 +300,10 @@ describe('sendFeedback', () => {
       return Promise.resolve({ statusCode: 403 });
     });
 
-    await expect(
+    await expectRejectsWithError(
       sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
-    ).rejects.toMatch('custom forbidden text');
+      'custom forbidden text',
+    );
   });
 
   it('falls back to default messages for codes not in errorMessages', async () => {
@@ -300,9 +313,8 @@ describe('sendFeedback', () => {
     });
 
     // Only override ERROR_FORBIDDEN — a 400 should still use the default generic message.
-    await expect(
+    await expectRejectsWithError(
       sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
-    ).rejects.toMatch(
       'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
     );
   });
@@ -313,13 +325,12 @@ describe('sendFeedback', () => {
       return Promise.resolve({ statusCode: 400 });
     });
 
-    await expect(
+    await expectRejectsWithError(
       sendFeedback({
         name: 'doe',
         email: 're@example.org',
         message: 'mi',
       }),
-    ).rejects.toMatch(
       'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
     );
   });
@@ -330,13 +341,12 @@ describe('sendFeedback', () => {
       return Promise.resolve({ statusCode: 0 });
     });
 
-    await expect(
+    await expectRejectsWithError(
       sendFeedback({
         name: 'doe',
         email: 're@example.org',
         message: 'mi',
       }),
-    ).rejects.toMatch(
       'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
     );
   });
@@ -347,13 +357,12 @@ describe('sendFeedback', () => {
       return Promise.resolve({ statusCode: 403 });
     });
 
-    await expect(
+    await expectRejectsWithError(
       sendFeedback({
         name: 'doe',
         email: 're@example.org',
         message: 'mi',
       }),
-    ).rejects.toMatch(
       'Unable to send feedback. This could be because this domain is not in your list of allowed domains.',
     );
   });
@@ -389,7 +398,7 @@ describe('sendFeedback', () => {
 
     vi.advanceTimersByTime(30_000);
 
-    await expect(promise).rejects.toMatch('Unable to determine if Feedback was correctly sent.');
+    await expectRejectsWithError(promise, 'Unable to determine if Feedback was correctly sent.');
 
     vi.useRealTimers();
   });