Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ module.exports = [
path: 'packages/browser/build/npm/esm/prod/index.js',
import: createImport('init', 'feedbackIntegration'),
gzip: true,
limit: '43 KB',
limit: '44 KB',
},
{
name: '@sentry/browser (incl. sendFeedback)',
Expand Down Expand Up @@ -233,7 +233,7 @@ module.exports = [
name: 'CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)',
path: createCDNPath('bundle.tracing.replay.feedback.logs.metrics.min.js'),
gzip: true,
limit: '90 KB',
limit: '91 KB',
},
// browser CDN bundles (non-gzipped)
{
Expand Down Expand Up @@ -297,7 +297,7 @@ module.exports = [
path: createCDNPath('bundle.tracing.replay.feedback.logs.metrics.min.js'),
gzip: false,
brotli: false,
limit: '272 KB',
limit: '273 KB',
},
// Next.js SDK (ESM)
{
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ export type {
ReplayStopReason,
} from './types-hoist/replay';
export type {
FeedbackErrorCode,
FeedbackErrorMessages,
FeedbackEvent,
FeedbackFormData,
FeedbackInternalOptions,
Expand Down
25 changes: 25 additions & 0 deletions packages/core/src/types-hoist/feedback/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,31 @@ export interface FeedbackTextConfiguration {
* The label for the button that removed a highlight/hidden section of the screenshot.
*/
removeHighlightText: string;

/**
* Error text shown when feedback submission is attempted with an empty message
*/
errorEmptyMessageText: string;

/**
* Error text shown when the Sentry client is not set up
*/
errorNoClientText: string;

/**
* Error text shown when the feedback submission times out (after 30s)
*/
errorTimeoutText: string;

/**
* Error text shown when the feedback submission is blocked because the domain is not allowed (HTTP 403)
*/
errorForbiddenText: string;

/**
* Error text shown when the feedback submission fails for any other reason (e.g. network error, ad-blocker)
*/
errorGenericText: string;
}

/**
Expand Down
11 changes: 9 additions & 2 deletions packages/core/src/types-hoist/feedback/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ import type {
FeedbackTextConfiguration,
FeedbackThemeConfiguration,
} from './config';
import type { FeedbackEvent, SendFeedback, SendFeedbackParams, UserFeedback } from './sendFeedback';
import type {
FeedbackErrorCode,
FeedbackErrorMessages,
FeedbackEvent,
SendFeedback,
SendFeedbackParams,
UserFeedback,
} from './sendFeedback';

export type { FeedbackFormData } from './form';
export type { FeedbackEvent, UserFeedback, SendFeedback, SendFeedbackParams };
export type { FeedbackErrorCode, FeedbackErrorMessages, FeedbackEvent, SendFeedback, SendFeedbackParams, UserFeedback };

/**
* The integration's internal `options` member where every value should be set
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/types-hoist/feedback/sendFeedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ export interface SendFeedbackParams {
tags?: { [key: string]: Primitive };
}

export type FeedbackErrorCode =
| 'ERROR_EMPTY_MESSAGE'
| 'ERROR_NO_CLIENT'
| 'ERROR_TIMEOUT'
| 'ERROR_FORBIDDEN'
| 'ERROR_GENERIC';

export type FeedbackErrorMessages = Partial<Record<FeedbackErrorCode, string>>;

export type SendFeedback = (
params: SendFeedbackParams,
hint?: EventHint & { includeReplay?: boolean },
hint?: EventHint & { includeReplay?: boolean; errorMessages?: FeedbackErrorMessages },
) => Promise<string>;
8 changes: 8 additions & 0 deletions packages/feedback/src/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ export const HIGHLIGHT_TOOL_TEXT = 'Highlight';
export const HIDE_TOOL_TEXT = 'Hide';
export const REMOVE_HIGHLIGHT_TEXT = 'Remove';

export const ERROR_EMPTY_MESSAGE_TEXT = 'Unable to submit feedback with empty message';
export const ERROR_NO_CLIENT_TEXT = 'No client setup, cannot send feedback.';
export const ERROR_TIMEOUT_TEXT = 'Unable to determine if Feedback was correctly sent.';
export const ERROR_FORBIDDEN_TEXT =
'Unable to send feedback. This could be because this domain is not in your list of allowed domains.';
export const ERROR_GENERIC_TEXT =
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.';

export const FEEDBACK_WIDGET_SOURCE = 'widget';
export const FEEDBACK_API_SOURCE = 'api';

Expand Down
30 changes: 29 additions & 1 deletion packages/feedback/src/core/integration.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/* eslint-disable max-lines */
/* eslint-disable complexity */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Could this be avoided by outsourcing wrappedSendFeedback ? Would still be nice to keep as many eslint oxlint rules as possible

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complexity is coming from the massive destructions, extracting the inner function doesn't make the rule pass 😕


import type {
FeedbackErrorMessages,
FeedbackInternalOptions,
FeedbackModalIntegration,
FeedbackScreenshotIntegration,
Integration,
IntegrationFn,
SendFeedback,
} from '@sentry/core';
import { addIntegration, debug, isBrowser } from '@sentry/core';
import {
Expand All @@ -15,6 +18,11 @@ import {
DOCUMENT,
EMAIL_LABEL,
EMAIL_PLACEHOLDER,
ERROR_EMPTY_MESSAGE_TEXT,
ERROR_FORBIDDEN_TEXT,
ERROR_GENERIC_TEXT,
ERROR_NO_CLIENT_TEXT,
ERROR_TIMEOUT_TEXT,
FORM_TITLE,
HIDE_TOOL_TEXT,
HIGHLIGHT_TOOL_TEXT,
Expand Down Expand Up @@ -119,6 +127,11 @@ export const buildFeedbackIntegration = ({
highlightToolText = HIGHLIGHT_TOOL_TEXT,
hideToolText = HIDE_TOOL_TEXT,
removeHighlightText = REMOVE_HIGHLIGHT_TEXT,
errorEmptyMessageText = ERROR_EMPTY_MESSAGE_TEXT,
errorNoClientText = ERROR_NO_CLIENT_TEXT,
errorTimeoutText = ERROR_TIMEOUT_TEXT,
errorForbiddenText = ERROR_FORBIDDEN_TEXT,
errorGenericText = ERROR_GENERIC_TEXT,

// FeedbackCallbacks
onFormOpen,
Expand Down Expand Up @@ -164,6 +177,11 @@ export const buildFeedbackIntegration = ({
highlightToolText,
hideToolText,
removeHighlightText,
errorEmptyMessageText,
errorNoClientText,
errorTimeoutText,
errorForbiddenText,
errorGenericText,

onFormClose,
onFormOpen,
Expand Down Expand Up @@ -230,6 +248,16 @@ export const buildFeedbackIntegration = ({
debug.error('[Feedback] Missing feedback screenshot integration. Proceeding without screenshots.');
}

const errorMessages: FeedbackErrorMessages = {
ERROR_EMPTY_MESSAGE: options.errorEmptyMessageText,
ERROR_NO_CLIENT: options.errorNoClientText,
ERROR_TIMEOUT: options.errorTimeoutText,
ERROR_FORBIDDEN: options.errorForbiddenText,
ERROR_GENERIC: options.errorGenericText,
};
const wrappedSendFeedback: SendFeedback = (params, hint) =>
sendFeedback(params, { includeReplay: true, ...hint, errorMessages });

const dialog = modalIntegration.createDialog({
options: {
...options,
Expand All @@ -243,7 +271,7 @@ export const buildFeedbackIntegration = ({
},
},
screenshotIntegration,
sendFeedback,
sendFeedback: wrappedSendFeedback,
shadow: _createShadow(options),
});

Expand Down
31 changes: 20 additions & 11 deletions packages/feedback/src/core/sendFeedback.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,33 @@
import type { Event, EventHint, SendFeedback, SendFeedbackParams, TransportMakeRequestResponse } from '@sentry/core';
import type {
Event,
EventHint,
FeedbackErrorMessages,
SendFeedback,
SendFeedbackParams,
TransportMakeRequestResponse,
} from '@sentry/core';
import { captureFeedback, getClient, getCurrentScope, getLocationHref } from '@sentry/core';
import { FEEDBACK_API_SOURCE } from '../constants';
import { createFeedbackError, resolveFeedbackErrorMessage } from '../util/createFeedbackError';

/**
* Public API to send a Feedback item to Sentry
*/
export const sendFeedback: SendFeedback = (
params: SendFeedbackParams,
hint: EventHint & { includeReplay?: boolean } = { includeReplay: true },
hint: EventHint & { includeReplay?: boolean; errorMessages?: FeedbackErrorMessages } = { includeReplay: true },
): Promise<string> => {
const errorMessages = hint.errorMessages;

if (!params.message) {
throw new Error('Unable to submit feedback with empty message');
throw createFeedbackError('ERROR_EMPTY_MESSAGE', errorMessages);
}

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

if (!client) {
throw new Error('No client setup, cannot send feedback.');
throw createFeedbackError('ERROR_NO_CLIENT', errorMessages);
}
Comment thread
logaretm marked this conversation as resolved.

if (params.tags && Object.keys(params.tags).length) {
Expand All @@ -35,7 +45,10 @@ export const sendFeedback: SendFeedback = (
// We want to wait for the feedback to be sent (or not)
return new Promise<string>((resolve, reject) => {
// After 30s, we want to clear anyhow
const timeout = setTimeout(() => reject('Unable to determine if Feedback was correctly sent.'), 30_000);
const timeout = setTimeout(() => {
cleanup();
reject(resolveFeedbackErrorMessage('ERROR_TIMEOUT', errorMessages));
}, 30_000);

const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
Comment thread
logaretm marked this conversation as resolved.
if (event.event_id !== eventId) {
Expand All @@ -51,14 +64,10 @@ export const sendFeedback: SendFeedback = (
}

if (response?.statusCode === 403) {
return reject(
'Unable to send feedback. This could be because this domain is not in your list of allowed domains.',
);
return reject(resolveFeedbackErrorMessage('ERROR_FORBIDDEN', errorMessages));
}

return reject(
'Unable to send feedback. This could be because of network issues, or because you are using an ad-blocker.',
);
return reject(resolveFeedbackErrorMessage('ERROR_GENERIC', errorMessages));
});
Comment thread
logaretm marked this conversation as resolved.
});
};
Expand Down
5 changes: 3 additions & 2 deletions packages/feedback/src/modal/components/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,9 @@ export function Form({
onSubmitSuccess(data, eventId);
} catch (error) {
DEBUG_BUILD && debug.error(error);
setError(error as string);
onSubmitError(error as Error);
const err = error instanceof Error ? error : new Error(String(error));
setError(err.message);
onSubmitError(err);
}
} finally {
setIsSubmitting(false);
Expand Down
24 changes: 24 additions & 0 deletions packages/feedback/src/util/createFeedbackError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { FeedbackErrorCode, FeedbackErrorMessages } from '@sentry/core';
import {
ERROR_EMPTY_MESSAGE_TEXT,
ERROR_FORBIDDEN_TEXT,
ERROR_GENERIC_TEXT,
ERROR_NO_CLIENT_TEXT,
ERROR_TIMEOUT_TEXT,
} from '../constants';

const DEFAULT_MESSAGES: Record<FeedbackErrorCode, string> = {
ERROR_EMPTY_MESSAGE: ERROR_EMPTY_MESSAGE_TEXT,
ERROR_NO_CLIENT: ERROR_NO_CLIENT_TEXT,
ERROR_TIMEOUT: ERROR_TIMEOUT_TEXT,
ERROR_FORBIDDEN: ERROR_FORBIDDEN_TEXT,
ERROR_GENERIC: ERROR_GENERIC_TEXT,
};

export function resolveFeedbackErrorMessage(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): string {
return messages?.[code] ?? DEFAULT_MESSAGES[code];
}

export function createFeedbackError(code: FeedbackErrorCode, messages?: FeedbackErrorMessages): Error {
return new Error(resolveFeedbackErrorMessage(code, messages));
}
40 changes: 40 additions & 0 deletions packages/feedback/test/core/sendFeedback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,46 @@ describe('sendFeedback', () => {
]);
});

it('throws when message is empty', () => {
mockSdk();
expect(() => sendFeedback({ message: '' })).toThrow('Unable to submit feedback with empty message');
});

it('throws when no client is set up', async () => {
// Isolate in its own scope so the client set up by other tests doesn't bleed in.
// `getClient` reads from the current scope; resetting it here leaves no client.
const { getGlobalScope } = await import('@sentry/core');
getGlobalScope().setClient(undefined);
getCurrentScope().setClient(undefined);
getIsolationScope().setClient(undefined);
expect(() => sendFeedback({ message: 'mi' })).toThrow('No client setup, cannot send feedback.');
});

it('uses provided errorMessages overrides', async () => {
mockSdk();
vi.spyOn(getClient()!.getTransport()!, 'send').mockImplementation(() => {
return Promise.resolve({ statusCode: 403 });
});

await expect(
sendFeedback({ message: 'mi' }, { errorMessages: { ERROR_FORBIDDEN: 'custom forbidden text' } }),
).rejects.toMatch('custom forbidden text');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: maybe it could be worth adding a case where we check that non-overriden messages keep the default?

});

it('falls back to default messages for codes not in errorMessages', async () => {
mockSdk();
vi.spyOn(getClient()!.getTransport()!, 'send').mockImplementation(() => {
return Promise.resolve({ statusCode: 400 });
});

// Only override ERROR_FORBIDDEN — a 400 should still use the default generic message.
await expect(
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.',
);
});

it('handles 400 transport error', async () => {
mockSdk();
vi.spyOn(getClient()!.getTransport()!, 'send').mockImplementation(() => {
Expand Down
Loading