Skip to content

Commit c790133

Browse files
logaretmclaude
andcommitted
refactor: inject error texts via sendFeedback hint, preserve default messages
Addresses review feedback that sendFeedback (a public API) was rejecting with internal codes instead of human-readable strings, changing observable behavior for non-widget consumers. sendFeedback now accepts optional errorMessages overrides via its hint argument, defaulting to the original English strings. The widget wraps sendFeedback to inject its configured text options, so standalone consumers of sendFeedback see the same strings as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3faea27 commit c790133

8 files changed

Lines changed: 97 additions & 42 deletions

File tree

packages/core/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,8 @@ export type {
452452
ReplayStopReason,
453453
} from './types-hoist/replay';
454454
export type {
455+
FeedbackErrorCode,
456+
FeedbackErrorMessages,
455457
FeedbackEvent,
456458
FeedbackFormData,
457459
FeedbackInternalOptions,

packages/core/src/types-hoist/feedback/index.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,17 @@ import type {
66
FeedbackTextConfiguration,
77
FeedbackThemeConfiguration,
88
} from './config';
9-
import type { FeedbackEvent, SendFeedback, SendFeedbackParams, UserFeedback } from './sendFeedback';
9+
import type {
10+
FeedbackErrorCode,
11+
FeedbackErrorMessages,
12+
FeedbackEvent,
13+
SendFeedback,
14+
SendFeedbackParams,
15+
UserFeedback,
16+
} from './sendFeedback';
1017

1118
export type { FeedbackFormData } from './form';
12-
export type { FeedbackEvent, UserFeedback, SendFeedback, SendFeedbackParams };
19+
export type { FeedbackErrorCode, FeedbackErrorMessages, FeedbackEvent, SendFeedback, SendFeedbackParams, UserFeedback };
1320

1421
/**
1522
* The integration's internal `options` member where every value should be set

packages/core/src/types-hoist/feedback/sendFeedback.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,16 @@ export interface SendFeedbackParams {
4747
tags?: { [key: string]: Primitive };
4848
}
4949

50+
export type FeedbackErrorCode =
51+
| 'ERROR_EMPTY_MESSAGE'
52+
| 'ERROR_NO_CLIENT'
53+
| 'ERROR_TIMEOUT'
54+
| 'ERROR_FORBIDDEN'
55+
| 'ERROR_GENERIC';
56+
57+
export type FeedbackErrorMessages = Partial<Record<FeedbackErrorCode, string>>;
58+
5059
export type SendFeedback = (
5160
params: SendFeedbackParams,
52-
hint?: EventHint & { includeReplay?: boolean },
61+
hint?: EventHint & { includeReplay?: boolean; errorMessages?: FeedbackErrorMessages },
5362
) => Promise<string>;

packages/feedback/src/core/integration.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
/* eslint-disable complexity */
33

44
import type {
5+
FeedbackErrorMessages,
56
FeedbackInternalOptions,
67
FeedbackModalIntegration,
78
FeedbackScreenshotIntegration,
89
Integration,
910
IntegrationFn,
11+
SendFeedback,
1012
} from '@sentry/core';
1113
import { addIntegration, debug, isBrowser } from '@sentry/core';
1214
import {
@@ -246,6 +248,16 @@ export const buildFeedbackIntegration = ({
246248
debug.error('[Feedback] Missing feedback screenshot integration. Proceeding without screenshots.');
247249
}
248250

251+
const errorMessages: FeedbackErrorMessages = {
252+
ERROR_EMPTY_MESSAGE: options.errorEmptyMessageText,
253+
ERROR_NO_CLIENT: options.errorNoClientText,
254+
ERROR_TIMEOUT: options.errorTimeoutText,
255+
ERROR_FORBIDDEN: options.errorForbiddenText,
256+
ERROR_GENERIC: options.errorGenericText,
257+
};
258+
const wrappedSendFeedback: SendFeedback = (params, hint) =>
259+
sendFeedback(params, { includeReplay: true, ...hint, errorMessages });
260+
249261
const dialog = modalIntegration.createDialog({
250262
options: {
251263
...options,
@@ -259,7 +271,7 @@ export const buildFeedbackIntegration = ({
259271
},
260272
},
261273
screenshotIntegration,
262-
sendFeedback,
274+
sendFeedback: wrappedSendFeedback,
263275
shadow: _createShadow(options),
264276
});
265277

packages/feedback/src/core/sendFeedback.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,33 @@
1-
import type { Event, EventHint, SendFeedback, SendFeedbackParams, TransportMakeRequestResponse } from '@sentry/core';
1+
import type {
2+
Event,
3+
EventHint,
4+
FeedbackErrorMessages,
5+
SendFeedback,
6+
SendFeedbackParams,
7+
TransportMakeRequestResponse,
8+
} from '@sentry/core';
29
import { captureFeedback, getClient, getCurrentScope, getLocationHref } from '@sentry/core';
310
import { FEEDBACK_API_SOURCE } from '../constants';
4-
import type { FeedbackErrorCode } from '../util/createFeedbackError';
5-
import { createFeedbackError } from '../util/createFeedbackError';
11+
import { createFeedbackError, resolveFeedbackErrorMessage } from '../util/createFeedbackError';
612

713
/**
814
* Public API to send a Feedback item to Sentry
915
*/
1016
export const sendFeedback: SendFeedback = (
1117
params: SendFeedbackParams,
12-
hint: EventHint & { includeReplay?: boolean } = { includeReplay: true },
18+
hint: EventHint & { includeReplay?: boolean; errorMessages?: FeedbackErrorMessages } = { includeReplay: true },
1319
): Promise<string> => {
20+
const errorMessages = hint.errorMessages;
21+
1422
if (!params.message) {
15-
throw createFeedbackError('ERROR_EMPTY_MESSAGE');
23+
throw createFeedbackError('ERROR_EMPTY_MESSAGE', errorMessages);
1624
}
1725

1826
// We want to wait for the feedback to be sent (or not)
1927
const client = getClient();
2028

2129
if (!client) {
22-
throw createFeedbackError('ERROR_NO_CLIENT');
30+
throw createFeedbackError('ERROR_NO_CLIENT', errorMessages);
2331
}
2432

2533
if (params.tags && Object.keys(params.tags).length) {
@@ -39,7 +47,7 @@ export const sendFeedback: SendFeedback = (
3947
// After 30s, we want to clear anyhow
4048
const timeout = setTimeout(() => {
4149
cleanup();
42-
reject('ERROR_TIMEOUT' satisfies FeedbackErrorCode);
50+
reject(resolveFeedbackErrorMessage('ERROR_TIMEOUT', errorMessages));
4351
}, 30_000);
4452

4553
const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
@@ -56,10 +64,10 @@ export const sendFeedback: SendFeedback = (
5664
}
5765

5866
if (response?.statusCode === 403) {
59-
return reject('ERROR_FORBIDDEN' satisfies FeedbackErrorCode);
67+
return reject(resolveFeedbackErrorMessage('ERROR_FORBIDDEN', errorMessages));
6068
}
6169

62-
return reject('ERROR_GENERIC' satisfies FeedbackErrorCode);
70+
return reject(resolveFeedbackErrorMessage('ERROR_GENERIC', errorMessages));
6371
});
6472
});
6573
};

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import type { JSX, VNode } from 'preact';
99
import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars
1010
import { useCallback, useState } from 'preact/hooks';
1111
import { FEEDBACK_WIDGET_SOURCE } from '../../constants';
12-
import type { FeedbackErrorCode } from '../../util/createFeedbackError';
1312
import { DEBUG_BUILD } from '../../util/debug-build';
1413
import { getMissingFields } from '../../util/validate';
1514

@@ -60,20 +59,7 @@ export function Form({
6059
namePlaceholder,
6160
submitButtonLabel,
6261
isRequiredLabel,
63-
errorEmptyMessageText,
64-
errorNoClientText,
65-
errorTimeoutText,
66-
errorForbiddenText,
67-
errorGenericText,
6862
} = options;
69-
70-
const errorTextByCode: Record<FeedbackErrorCode, string> = {
71-
ERROR_EMPTY_MESSAGE: errorEmptyMessageText,
72-
ERROR_NO_CLIENT: errorNoClientText,
73-
ERROR_TIMEOUT: errorTimeoutText,
74-
ERROR_FORBIDDEN: errorForbiddenText,
75-
ERROR_GENERIC: errorGenericText,
76-
};
7763
const [isSubmitting, setIsSubmitting] = useState<boolean>(false);
7864
// TODO: set a ref on the form, and whenever an input changes call processForm() and setError()
7965
const [error, setError] = useState<null | string>(null);
@@ -146,7 +132,7 @@ export function Form({
146132
} catch (error) {
147133
DEBUG_BUILD && debug.error(error);
148134
const err = error instanceof Error ? error : new Error(String(error));
149-
setError(errorTextByCode[err.message as FeedbackErrorCode] ?? errorGenericText);
135+
setError(err.message);
150136
onSubmitError(err);
151137
}
152138
} finally {
Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,24 @@
1-
export type FeedbackErrorCode =
2-
| 'ERROR_EMPTY_MESSAGE'
3-
| 'ERROR_NO_CLIENT'
4-
| 'ERROR_TIMEOUT'
5-
| 'ERROR_FORBIDDEN'
6-
| 'ERROR_GENERIC';
1+
import type { FeedbackErrorCode, FeedbackErrorMessages } from '@sentry/core';
2+
import {
3+
ERROR_EMPTY_MESSAGE_TEXT,
4+
ERROR_FORBIDDEN_TEXT,
5+
ERROR_GENERIC_TEXT,
6+
ERROR_NO_CLIENT_TEXT,
7+
ERROR_TIMEOUT_TEXT,
8+
} from '../constants';
79

8-
export function createFeedbackError(reason: FeedbackErrorCode): Error {
9-
return new Error(reason);
10+
const DEFAULT_MESSAGES: Record<FeedbackErrorCode, string> = {
11+
ERROR_EMPTY_MESSAGE: ERROR_EMPTY_MESSAGE_TEXT,
12+
ERROR_NO_CLIENT: ERROR_NO_CLIENT_TEXT,
13+
ERROR_TIMEOUT: ERROR_TIMEOUT_TEXT,
14+
ERROR_FORBIDDEN: ERROR_FORBIDDEN_TEXT,
15+
ERROR_GENERIC: ERROR_GENERIC_TEXT,
16+
};
17+
18+
export function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
19+
return messages?.[code] ?? DEFAULT_MESSAGES[code];
20+
}
21+
22+
export function createFeedbackError(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): Error {
23+
return new Error(resolveFeedbackErrorMessage(code, messages));
1024
}

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ describe('sendFeedback', () => {
269269

270270
it('throws when message is empty', () => {
271271
mockSdk();
272-
expect(() => sendFeedback({ message: '' })).toThrow('ERROR_EMPTY_MESSAGE');
272+
expect(() => sendFeedback({ message: '' })).toThrow('Unable to submit feedback with an empty message');
273273
});
274274

275275
it('throws when no client is set up', async () => {
@@ -279,7 +279,18 @@ describe('sendFeedback', () => {
279279
getGlobalScope().setClient(undefined);
280280
getCurrentScope().setClient(undefined);
281281
getIsolationScope().setClient(undefined);
282-
expect(() => sendFeedback({ message: 'mi' })).toThrow('ERROR_NO_CLIENT');
282+
expect(() => sendFeedback({ message: 'mi' })).toThrow('No client setup, cannot send feedback.');
283+
});
284+
285+
it('uses provided errorMessages overrides', async () => {
286+
mockSdk();
287+
vi.spyOn(getClient()!.getTransport()!, 'send').mockImplementation(() => {
288+
return Promise.resolve({ statusCode: 403 });
289+
});
290+
291+
await expect(
292+
sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
293+
).rejects.toMatch('custom forbidden text');
283294
});
284295

285296
it('handles 400 transport error', async () => {
@@ -294,7 +305,9 @@ describe('sendFeedback', () => {
294305
email: 're@example.org',
295306
message: 'mi',
296307
}),
297-
).rejects.toMatch('ERROR_GENERIC');
308+
).rejects.toMatch(
309+
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
310+
);
298311
});
299312

300313
it('handles 0 transport error', async () => {
@@ -309,7 +322,9 @@ describe('sendFeedback', () => {
309322
email: 're@example.org',
310323
message: 'mi',
311324
}),
312-
).rejects.toMatch('ERROR_GENERIC');
325+
).rejects.toMatch(
326+
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
327+
);
313328
});
314329

315330
it('handles 403 transport error', async () => {
@@ -324,7 +339,9 @@ describe('sendFeedback', () => {
324339
email: 're@example.org',
325340
message: 'mi',
326341
}),
327-
).rejects.toMatch('ERROR_FORBIDDEN');
342+
).rejects.toMatch(
343+
'Unable to send feedback. This could be because this domain is not in your list of allowed domains.',
344+
);
328345
});
329346

330347
it('handles 200 transport response', async () => {
@@ -358,7 +375,7 @@ describe('sendFeedback', () => {
358375

359376
vi.advanceTimersByTime(30_000);
360377

361-
await expect(promise).rejects.toMatch('ERROR_TIMEOUT');
378+
await expect(promise).rejects.toMatch('Unable to determine if Feedback was correctly sent.');
362379

363380
vi.useRealTimers();
364381
});

0 commit comments

Comments
 (0)