From 30ad95170d0c9c82dbf20e52948259a603e83a34 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Tue, 17 Dec 2024 12:43:20 +0200 Subject: [PATCH 1/2] Use sendFeedback from JS to get error messages --- packages/core/package.json | 1 + .../core/src/js/feedback/FeedbackForm.tsx | 29 ++++++++++++++----- .../core/test/feedback/FeedbackForm.test.tsx | 12 ++++---- yarn.lock | 1 + 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 12d2da1b7..6bbc66d95 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -65,6 +65,7 @@ "react-native": ">=0.65.0" }, "dependencies": { + "@sentry-internal/feedback": "8.45.1", "@sentry/babel-plugin-component-annotate": "2.20.1", "@sentry/browser": "8.45.1", "@sentry/cli": "2.39.1", diff --git a/packages/core/src/js/feedback/FeedbackForm.tsx b/packages/core/src/js/feedback/FeedbackForm.tsx index 6f2ed9187..25dabb054 100644 --- a/packages/core/src/js/feedback/FeedbackForm.tsx +++ b/packages/core/src/js/feedback/FeedbackForm.tsx @@ -1,5 +1,6 @@ -import { captureFeedback, getCurrentScope, lastEventId, logger } from '@sentry/core'; -import type { SendFeedbackParams } from '@sentry/types'; +import { sendFeedback } from '@sentry-internal/feedback'; +import type { EventHint, SendFeedbackParams } from '@sentry/core'; +import { getCurrentScope, lastEventId, logger } from '@sentry/core'; import * as React from 'react'; import type { KeyboardTypeOptions } from 'react-native'; import { @@ -20,6 +21,15 @@ import defaultStyles from './FeedbackForm.styles'; import type { FeedbackFormProps, FeedbackFormState, FeedbackFormStyles,FeedbackGeneralConfiguration, FeedbackTextConfiguration } from './FeedbackForm.types'; import { checkInternetConnection, isValidEmail } from './utils'; +const submitFeedback = async (feedbackParams: SendFeedbackParams, hint: EventHint, success: () => void, error: (e: string) => void): Promise => { + try { + await sendFeedback(feedbackParams, hint); + success(); + } catch (e) { + error(e.toString()); + } +}; + /** * @beta * Implements a feedback form screen that sends feedback to Sentry using Sentry.captureFeedback. @@ -75,11 +85,16 @@ export class FeedbackForm extends React.Component { // onConnected - this.setState({ isVisible: false }); - captureFeedback(userFeedback); - onSubmitSuccess({ name: trimmedName, email: trimmedEmail, message: trimmedDescription, attachments: undefined }); - Alert.alert(text.successMessageText); - onFormSubmitted(); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + submitFeedback(userFeedback, undefined, () => { // success + this.setState({ isVisible: false }); + onSubmitSuccess({ name: trimmedName, email: trimmedEmail, message: trimmedDescription, attachments: undefined }); + Alert.alert(text.successMessageText); + onFormSubmitted(); + }, (error: string) => { // error + Alert.alert(text.errorTitle, error); + logger.error(`Feedback form submission failed: ${error}`); + }); }, () => { // onDisconnected Alert.alert(text.errorTitle, text.networkError); logger.error(`Feedback form submission failed: ${text.networkError}`); diff --git a/packages/core/test/feedback/FeedbackForm.test.tsx b/packages/core/test/feedback/FeedbackForm.test.tsx index 2d19f5402..07e9ee54f 100644 --- a/packages/core/test/feedback/FeedbackForm.test.tsx +++ b/packages/core/test/feedback/FeedbackForm.test.tsx @@ -1,4 +1,4 @@ -import { captureFeedback } from '@sentry/core'; +import { sendFeedback } from '@sentry-internal/feedback'; import { fireEvent, render, waitFor } from '@testing-library/react-native'; import * as React from 'react'; import { Alert } from 'react-native'; @@ -16,7 +16,6 @@ jest.spyOn(Alert, 'alert'); jest.mock('@sentry/core', () => ({ ...jest.requireActual('@sentry/core'), - captureFeedback: jest.fn(), getCurrentScope: jest.fn(() => ({ getUser: jest.fn(() => ({ email: 'test@example.com', @@ -29,6 +28,9 @@ jest.mock('../../src/js/feedback/utils', () => ({ ...jest.requireActual('../../src/js/feedback/utils'), checkInternetConnection: jest.fn(), })); +jest.mock('@sentry-internal/feedback', () => ({ + sendFeedback: jest.fn(), +})); const defaultProps: FeedbackFormProps = { onFormClose: mockOnFormClose, @@ -112,7 +114,7 @@ describe('FeedbackForm', () => { }); }); - it('calls captureFeedback when the form is submitted successfully', async () => { + it('calls sendFeedback when the form is submitted successfully', async () => { const { getByPlaceholderText, getByText } = render(); fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe'); @@ -122,11 +124,11 @@ describe('FeedbackForm', () => { fireEvent.press(getByText(defaultProps.submitButtonLabel)); await waitFor(() => { - expect(captureFeedback).toHaveBeenCalledWith({ + expect(sendFeedback).toHaveBeenCalledWith({ message: 'This is a feedback message.', name: 'John Doe', email: 'john.doe@example.com', - }); + }, undefined); }); }); diff --git a/yarn.lock b/yarn.lock index 1ab773092..f77509c43 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7821,6 +7821,7 @@ __metadata: "@react-native/babel-preset": 0.76.3 "@sentry-internal/eslint-config-sdk": 8.45.1 "@sentry-internal/eslint-plugin-sdk": 8.45.1 + "@sentry-internal/feedback": 8.45.1 "@sentry-internal/typescript": 8.45.1 "@sentry/babel-plugin-component-annotate": 2.20.1 "@sentry/browser": 8.45.1 From eeaa319a839cedbb108015116446f7babaf7aa4b Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Fri, 20 Dec 2024 13:20:38 +0200 Subject: [PATCH 2/2] Move sendFeedback implementation in the SDK --- packages/core/package.json | 1 - .../core/src/js/feedback/FeedbackForm.tsx | 2 +- packages/core/src/js/feedback/sendFeedback.ts | 52 +++++++++++++++++++ .../core/test/feedback/FeedbackForm.test.tsx | 4 +- yarn.lock | 17 ------ 5 files changed, 55 insertions(+), 21 deletions(-) create mode 100644 packages/core/src/js/feedback/sendFeedback.ts diff --git a/packages/core/package.json b/packages/core/package.json index 45420473b..fd9efa782 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -65,7 +65,6 @@ "react-native": ">=0.65.0" }, "dependencies": { - "@sentry-internal/feedback": "8.45.1", "@sentry/babel-plugin-component-annotate": "2.20.1", "@sentry/browser": "8.47.0", "@sentry/cli": "2.39.1", diff --git a/packages/core/src/js/feedback/FeedbackForm.tsx b/packages/core/src/js/feedback/FeedbackForm.tsx index dc6cd4f69..7f9a91375 100644 --- a/packages/core/src/js/feedback/FeedbackForm.tsx +++ b/packages/core/src/js/feedback/FeedbackForm.tsx @@ -1,4 +1,3 @@ -import { sendFeedback } from '@sentry-internal/feedback'; import type { EventHint, SendFeedbackParams } from '@sentry/core'; import { getCurrentScope, lastEventId, logger } from '@sentry/core'; import * as React from 'react'; @@ -21,6 +20,7 @@ import { sentryLogo } from './branding'; import { defaultConfiguration } from './defaults'; import defaultStyles from './FeedbackForm.styles'; import type { FeedbackFormProps, FeedbackFormState, FeedbackFormStyles,FeedbackGeneralConfiguration, FeedbackTextConfiguration } from './FeedbackForm.types'; +import { sendFeedback } from './sendFeedback'; import { checkInternetConnection, isValidEmail } from './utils'; const submitFeedback = async (feedbackParams: SendFeedbackParams, hint: EventHint, success: () => void, error: (e: string) => void): Promise => { diff --git a/packages/core/src/js/feedback/sendFeedback.ts b/packages/core/src/js/feedback/sendFeedback.ts new file mode 100644 index 000000000..d55e2e563 --- /dev/null +++ b/packages/core/src/js/feedback/sendFeedback.ts @@ -0,0 +1,52 @@ +import type { Event, EventHint, SendFeedback, SendFeedbackParams, TransportMakeRequestResponse } from '@sentry/core'; +import { captureFeedback, getClient, logger } from '@sentry/core'; + +const FEEDBACK_WIDGET_SOURCE = 'widget'; +const FEEDBACK_RESPONSE_TIMEOUT = 5_000; + +/** + * Sends feedback to Sentry using Sentry.captureFeedback and waits for the response. + */ +export const sendFeedback: SendFeedback = ( + params: SendFeedbackParams, + hint: EventHint & { includeReplay?: boolean } = { includeReplay: true }, +): Promise => { + const client = getClient(); + if (!client) { + throw new Error('No client setup, cannot send feedback.'); + } + + const eventId = captureFeedback( + { + source: FEEDBACK_WIDGET_SOURCE, + ...params, + }, + hint, + ); + + logger.debug('Feedback captured with Event ID:', eventId); + + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + logger.error('Feedback submission timed out.'); + reject('Unable to determine if Feedback was correctly sent.'); + }, FEEDBACK_RESPONSE_TIMEOUT); + + const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => { + if (event.event_id !== eventId) { + return; + } + + clearTimeout(timeout); + cleanup(); + + if (event.type === 'feedback') { + logger.debug('Feedback detected as sent with eventId:', eventId); + resolve(eventId); + } else { + logger.error('Unexpected response status:', response?.statusCode); + reject('Unable to send Feedback due to unexpected issues.'); + } + }); + }); +}; diff --git a/packages/core/test/feedback/FeedbackForm.test.tsx b/packages/core/test/feedback/FeedbackForm.test.tsx index 52602d539..812005a80 100644 --- a/packages/core/test/feedback/FeedbackForm.test.tsx +++ b/packages/core/test/feedback/FeedbackForm.test.tsx @@ -1,10 +1,10 @@ -import { sendFeedback } from '@sentry-internal/feedback'; import { fireEvent, render, waitFor } from '@testing-library/react-native'; import * as React from 'react'; import { Alert } from 'react-native'; import { FeedbackForm } from '../../src/js/feedback/FeedbackForm'; import type { FeedbackFormProps } from '../../src/js/feedback/FeedbackForm.types'; +import { sendFeedback } from '../../src/js/feedback/sendFeedback'; import { checkInternetConnection } from '../../src/js/feedback/utils'; const mockOnFormClose = jest.fn(); @@ -29,7 +29,7 @@ jest.mock('../../src/js/feedback/utils', () => ({ ...jest.requireActual('../../src/js/feedback/utils'), checkInternetConnection: jest.fn(), })); -jest.mock('@sentry-internal/feedback', () => ({ +jest.mock('../../src/js/feedback/sendFeedback', () => ({ sendFeedback: jest.fn(), })); diff --git a/yarn.lock b/yarn.lock index 5ee1c9cdc..b9c883a93 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7591,15 +7591,6 @@ __metadata: languageName: node linkType: hard -"@sentry-internal/feedback@npm:8.45.1": - version: 8.45.1 - resolution: "@sentry-internal/feedback@npm:8.45.1" - dependencies: - "@sentry/core": 8.45.1 - checksum: aa0d50078350dbfd3ebdfed448a0d4279324d624fb464bc411d3ec286d6895e52753bc9217bcdfe75a35e184d479588227d1e503bf766ea39bd5392103431b25 - languageName: node - linkType: hard - "@sentry-internal/feedback@npm:8.47.0": version: 8.47.0 resolution: "@sentry-internal/feedback@npm:8.47.0" @@ -7788,13 +7779,6 @@ __metadata: languageName: node linkType: hard -"@sentry/core@npm:8.45.1": - version: 8.45.1 - resolution: "@sentry/core@npm:8.45.1" - checksum: 81ed72485685d7baf246ed35c01428664a97eece445ab7b801b908e93dcb58652eaca588c2f9f80baa95419590eb78251305f9e5258ba8a1efee839cb40b15c8 - languageName: node - linkType: hard - "@sentry/core@npm:8.47.0": version: 8.47.0 resolution: "@sentry/core@npm:8.47.0" @@ -7837,7 +7821,6 @@ __metadata: "@react-native/babel-preset": 0.76.3 "@sentry-internal/eslint-config-sdk": 8.47.0 "@sentry-internal/eslint-plugin-sdk": 8.47.0 - "@sentry-internal/feedback": 8.45.1 "@sentry-internal/typescript": 8.47.0 "@sentry/babel-plugin-component-annotate": 2.20.1 "@sentry/browser": 8.47.0