Skip to content

Commit 31d6ad3

Browse files
committed
feat(feedback)!: always reject sendFeedback with an Error
Previously, sendFeedback's async rejection paths (timeout, 403, generic transport error) rejected with a plain string, while the sync-throw paths (empty message, no client) threw an Error. Unify on Error for all paths so consumers can rely on a consistent rejection shape. BREAKING CHANGE: sendFeedback now rejects with an Error instead of a string. The message text is unchanged and is still customizable via the `errorMessages` hint, so consumers reading it should switch from the rejection value itself to `error.message`.
1 parent 1dca402 commit 31d6ad3

5 files changed

Lines changed: 37 additions & 17 deletions

File tree

‎MIGRATION.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,22 @@ Two consequences to be aware of when upgrading:
520520
- **Issue grouping:** Grouping in Sentry differs for events with and without stack traces, so you may see new issue groups after upgrading.
521521
- **Release health:** Events with a stack trace are counted as errors, so a `captureMessage` call (including messages emitted by `captureConsoleIntegration`) now marks the current session as _errored_. This affects errored-session counts but does **not** mark sessions as crashed, so crash-free session rate is unaffected. If you use `captureMessage` for purely informational output, consider using Sentry Logs instead, which is better suited and does not affect release health.
522522

523+
### `sendFeedback` rejects with an `Error`
524+
525+
Affected SDKs: All SDKs running in the browser.
526+
527+
`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`:
528+
529+
```js
530+
try {
531+
await Sentry.sendFeedback({ message: 'Hello' });
532+
} catch (error) {
533+
// v10: a string on send failures, an Error on validation failures
534+
// v11: always an Error
535+
console.log(error.message);
536+
}
537+
```
538+
523539
### Span attribute changes
524540

525541
Affected SDKs: All SDKs.

‎packages/feedback/src/core/sendFeedback.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type {
88
} from '@sentry/core';
99
import { captureFeedback, getClient, getCurrentScope, getLocationHref } from '@sentry/core';
1010
import { FEEDBACK_API_SOURCE } from '../constants';
11-
import { createFeedbackError, resolveFeedbackErrorMessage } from '../util/createFeedbackError';
11+
import { createFeedbackError } from '../util/createFeedbackError';
1212

1313
/**
1414
* Public API to send a Feedback item to Sentry
@@ -47,7 +47,7 @@ export const sendFeedback: SendFeedback = (
4747
// After 30s, we want to clear anyhow
4848
const timeout = setTimeout(() => {
4949
cleanup();
50-
reject(resolveFeedbackErrorMessage('ERROR_TIMEOUT', errorMessages));
50+
reject(createFeedbackError('ERROR_TIMEOUT', errorMessages));
5151
}, 30_000);
5252

5353
const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
@@ -64,10 +64,10 @@ export const sendFeedback: SendFeedback = (
6464
}
6565

6666
if (response?.statusCode === 403) {
67-
return reject(resolveFeedbackErrorMessage('ERROR_FORBIDDEN', errorMessages));
67+
return reject(createFeedbackError('ERROR_FORBIDDEN', errorMessages));
6868
}
6969

70-
return reject(resolveFeedbackErrorMessage('ERROR_GENERIC', errorMessages));
70+
return reject(createFeedbackError('ERROR_GENERIC', errorMessages));
7171
});
7272
});
7373
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ export function Form({
131131
onSubmitSuccess(data, eventId);
132132
} catch (error) {
133133
DEBUG_BUILD && debug.error(error);
134-
const err = error instanceof Error ? error : new Error(String(error));
134+
const err = error as Error;
135135
setError(err.message);
136136
onSubmitError(err);
137137
}

‎packages/feedback/src/util/createFeedbackError.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const DEFAULT_MESSAGES: Record<FeedbackErrorCode, string> = {
1515
ERROR_GENERIC: ERROR_GENERIC_TEXT,
1616
};
1717

18-
export function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
18+
function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
1919
return messages?.[code] ?? DEFAULT_MESSAGES[code];
2020
}
2121

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

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ describe('sendFeedback', () => {
3333
patchedDecoder && delete global.window.TextDecoder;
3434
});
3535

36+
// The rejection value must be an `Error`, not a bare string: `rejects.toThrow(text)` alone
37+
// also passes for a string rejection, so assert the shape explicitly too.
38+
async function expectRejectsWithError(promise: Promise<unknown>, message: string): Promise<void> {
39+
await expect(promise).rejects.toBeInstanceOf(Error);
40+
await expect(promise).rejects.toThrow(message);
41+
}
42+
3643
it('sends feedback with minimal options', async () => {
3744
mockSdk();
3845
const mockTransport = vi.spyOn(getClient()!.getTransport()!, 'send');
@@ -288,9 +295,10 @@ describe('sendFeedback', () => {
288295
return Promise.resolve({ statusCode: 403 });
289296
});
290297

291-
await expect(
298+
await expectRejectsWithError(
292299
sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
293-
).rejects.toMatch('custom forbidden text');
300+
'custom forbidden text',
301+
);
294302
});
295303

296304
it('falls back to default messages for codes not in errorMessages', async () => {
@@ -300,9 +308,8 @@ describe('sendFeedback', () => {
300308
});
301309

302310
// Only override ERROR_FORBIDDEN — a 400 should still use the default generic message.
303-
await expect(
311+
await expectRejectsWithError(
304312
sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
305-
).rejects.toMatch(
306313
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
307314
);
308315
});
@@ -313,13 +320,12 @@ describe('sendFeedback', () => {
313320
return Promise.resolve({ statusCode: 400 });
314321
});
315322

316-
await expect(
323+
await expectRejectsWithError(
317324
sendFeedback({
318325
name: 'doe',
319326
email: 're@example.org',
320327
message: 'mi',
321328
}),
322-
).rejects.toMatch(
323329
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
324330
);
325331
});
@@ -330,13 +336,12 @@ describe('sendFeedback', () => {
330336
return Promise.resolve({ statusCode: 0 });
331337
});
332338

333-
await expect(
339+
await expectRejectsWithError(
334340
sendFeedback({
335341
name: 'doe',
336342
email: 're@example.org',
337343
message: 'mi',
338344
}),
339-
).rejects.toMatch(
340345
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
341346
);
342347
});
@@ -347,13 +352,12 @@ describe('sendFeedback', () => {
347352
return Promise.resolve({ statusCode: 403 });
348353
});
349354

350-
await expect(
355+
await expectRejectsWithError(
351356
sendFeedback({
352357
name: 'doe',
353358
email: 're@example.org',
354359
message: 'mi',
355360
}),
356-
).rejects.toMatch(
357361
'Unable to send feedback. This could be because this domain is not in your list of allowed domains.',
358362
);
359363
});
@@ -389,7 +393,7 @@ describe('sendFeedback', () => {
389393

390394
vi.advanceTimersByTime(30_000);
391395

392-
await expect(promise).rejects.toMatch('Unable to determine if Feedback was correctly sent.');
396+
await expectRejectsWithError(promise, 'Unable to determine if Feedback was correctly sent.');
393397

394398
vi.useRealTimers();
395399
});

0 commit comments

Comments
 (0)