From 6c56eb14a3bfef1d562f91f8ec6e2f32cf76e4a2 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Mon, 2 Dec 2024 12:46:47 +0200 Subject: [PATCH] feat: Add new captureFeedback API to RN SDK (#4320) --- CHANGELOG.md | 17 + packages/core/src/js/client.ts | 1 + packages/core/src/js/index.ts | 2 + packages/core/src/js/sdk.tsx | 13 +- packages/core/test/feedback.test.ts | 473 ++++++++++++++++++ .../src/components/UserFeedbackModal.tsx | 19 +- .../src/components/UserFeedbackModal.tsx | 19 +- 7 files changed, 539 insertions(+), 5 deletions(-) create mode 100644 packages/core/test/feedback.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a418f77fd..66be6a45c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,23 @@ ## Unreleased +### Features + +- Adds new `captureFeedback` and deprecates the `captureUserFeedback` API ([#4320](https://github.com/getsentry/sentry-react-native/pull/4320)) + + ```jsx + import * as Sentry from "@sentry/react-native"; + + const eventId = Sentry.lastEventId(); + + Sentry.captureFeedback({ + name: "John Doe", + email: "john@doe.com", + message: "Hello World!", + associatedEventId: eventId, // optional + }); + ``` + ### Fixes - Return `lastEventId` export from `@sentry/core` ([#4315](https://github.com/getsentry/sentry-react-native/pull/4315)) diff --git a/packages/core/src/js/client.ts b/packages/core/src/js/client.ts index b9fe2ad27d..1e2c858264 100644 --- a/packages/core/src/js/client.ts +++ b/packages/core/src/js/client.ts @@ -85,6 +85,7 @@ export class ReactNativeClient extends BaseClient { /** * Sends user feedback to Sentry. + * @deprecated Use `Sentry.captureFeedback` instead. */ public captureUserFeedback(feedback: UserFeedback): void { const envelope = createUserFeedbackEnvelope(feedback, { diff --git a/packages/core/src/js/index.ts b/packages/core/src/js/index.ts index f62a8624eb..4aa7e2fa21 100644 --- a/packages/core/src/js/index.ts +++ b/packages/core/src/js/index.ts @@ -4,6 +4,7 @@ export type { SdkInfo, Event, Exception, + SendFeedbackParams, SeverityLevel, StackFrame, Stacktrace, @@ -16,6 +17,7 @@ export { addBreadcrumb, captureException, captureEvent, + captureFeedback, captureMessage, Scope, setContext, diff --git a/packages/core/src/js/sdk.tsx b/packages/core/src/js/sdk.tsx index 039b44850d..448ce99148 100644 --- a/packages/core/src/js/sdk.tsx +++ b/packages/core/src/js/sdk.tsx @@ -1,10 +1,10 @@ /* eslint-disable complexity */ -import { getClient, getGlobalScope,getIntegrationsToSetup, getIsolationScope,initAndBind, withScope as coreWithScope } from '@sentry/core'; +import { captureFeedback, getClient, getGlobalScope,getIntegrationsToSetup, getIsolationScope,initAndBind, withScope as coreWithScope } from '@sentry/core'; import { defaultStackParser, makeFetchTransport, } from '@sentry/react'; -import type { Breadcrumb, BreadcrumbHint, Integration, Scope, UserFeedback } from '@sentry/types'; +import type { Breadcrumb, BreadcrumbHint, Integration, Scope, SendFeedbackParams, UserFeedback } from '@sentry/types'; import { logger, stackParserFromStackParserOptions } from '@sentry/utils'; import * as React from 'react'; @@ -219,9 +219,16 @@ export async function close(): Promise { /** * Captures user feedback and sends it to Sentry. + * @deprecated Use `Sentry.captureFeedback` instead. */ export function captureUserFeedback(feedback: UserFeedback): void { - getClient()?.captureUserFeedback(feedback); + const feedbackParams: SendFeedbackParams = { + name: feedback.name, + email: feedback.email, + message: feedback.comments, + associatedEventId: feedback.event_id, + }; + captureFeedback(feedbackParams); } /** diff --git a/packages/core/test/feedback.test.ts b/packages/core/test/feedback.test.ts new file mode 100644 index 0000000000..f36a697001 --- /dev/null +++ b/packages/core/test/feedback.test.ts @@ -0,0 +1,473 @@ +import { + addBreadcrumb, + captureFeedback, + getCurrentScope, + Scope, + setCurrentClient, + startSpan, + withIsolationScope, + withScope, +} from '@sentry/core'; +import type { Span } from '@sentry/types'; + +import { getDefaultTestClientOptions, TestClient } from './mocks/client'; + +describe('captureFeedback', () => { + beforeEach(() => { + getCurrentScope().setClient(undefined); + getCurrentScope().clear(); + }); + + test('it works without a client', () => { + const res = captureFeedback({ + message: 'test', + }); + + expect(typeof res).toBe('string'); + }); + + test('it works with minimal options', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + const eventId = captureFeedback({ + message: 'test', + }); + + await client.flush(); + + expect(typeof eventId).toBe('string'); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + feedback: { + message: 'test', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + tags: undefined, + type: 'feedback', + }, + ], + ], + ]); + }); + + test('it works with full options', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + const eventId = captureFeedback({ + name: 'doe', + email: 're@example.org', + message: 'mi', + url: 'http://example.com/', + source: 'custom-source', + associatedEventId: '1234', + }); + + await client.flush(); + + expect(typeof eventId).toBe('string'); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + feedback: { + name: 'doe', + contact_email: 're@example.org', + message: 'mi', + url: 'http://example.com/', + source: 'custom-source', + associated_event_id: '1234', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + tags: undefined, + type: 'feedback', + }, + ], + ], + ]); + }); + + test('it captures attachments', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + const attachment1 = new Uint8Array([1, 2, 3, 4, 5]); + const attachment2 = new Uint8Array([6, 7, 8, 9]); + + const eventId = captureFeedback( + { + message: 'test', + }, + { + attachments: [ + { + data: attachment1, + filename: 'test-file.txt', + }, + { + data: attachment2, + filename: 'test-file2.txt', + }, + ], + }, + ); + + await client.flush(); + + expect(typeof eventId).toBe('string'); + + expect(mockTransport).toHaveBeenCalledTimes(1); + + const [feedbackEnvelope] = mockTransport.mock.calls; + + expect(feedbackEnvelope).toHaveLength(1); + expect(feedbackEnvelope![0]).toEqual([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + feedback: { + message: 'test', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + tags: undefined, + type: 'feedback', + }, + ], + [ + { + type: 'attachment', + length: 5, + filename: 'test-file.txt', + }, + attachment1, + ], + [ + { + type: 'attachment', + length: 4, + filename: 'test-file2.txt', + }, + attachment2, + ], + ], + ]); + }); + + test('it captures DSC from scope', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + const traceId = '4C79F60C11214EB38604F4AE0781BFB2'; + const spanId = 'FA90FDEAD5F74052'; + const dsc = { + trace_id: traceId, + span_id: spanId, + sampled: 'true', + }; + + getCurrentScope().setPropagationContext({ + traceId, + spanId, + dsc, + }); + + const eventId = captureFeedback({ + message: 'test', + }); + + await client.flush(); + + expect(typeof eventId).toBe('string'); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + trace_id: traceId, + span_id: spanId, + }, + feedback: { + message: 'test', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + tags: undefined, + type: 'feedback', + }, + ], + ], + ]); + }); + + test('it captures data from active span', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + enableTracing: true, + // We don't care about transactions here... + beforeSendTransaction() { + return null; + }, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + let span: Span | undefined; + const eventId = startSpan({ name: 'test-span' }, _span => { + span = _span; + return captureFeedback({ + message: 'test', + }); + }); + + await client.flush(); + + expect(typeof eventId).toBe('string'); + expect(span).toBeDefined(); + + const { spanId, traceId } = span!.spanContext(); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + trace_id: traceId, + span_id: spanId, + }, + feedback: { + message: 'test', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + tags: undefined, + type: 'feedback', + }, + ], + ], + ]); + }); + + it('applies scope data to feedback', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + enableTracing: true, + // We don't care about transactions here... + beforeSendTransaction() { + return null; + }, + }), + ); + setCurrentClient(client); + client.init(); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + + withIsolationScope(isolationScope => { + isolationScope.setTag('test-1', 'tag'); + isolationScope.setExtra('test-1', 'extra'); + + return withScope(scope => { + scope.setTag('test-2', 'tag'); + scope.setExtra('test-2', 'extra'); + + addBreadcrumb({ message: 'test breadcrumb', timestamp: 12345 }); + + captureFeedback({ + name: 'doe', + email: 're@example.org', + message: 'mi', + }); + }); + }); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: expect.any(String), + sent_at: expect.any(String), + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: [{ message: 'test breadcrumb', timestamp: 12345 }], + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + feedback: { + contact_email: 're@example.org', + message: 'mi', + name: 'doe', + }, + }, + extra: { + 'test-1': 'extra', + 'test-2': 'extra', + }, + tags: { + 'test-1': 'tag', + 'test-2': 'tag', + }, + level: 'info', + environment: 'production', + event_id: expect.any(String), + timestamp: expect.any(Number), + type: 'feedback', + }, + ], + ], + ]); + }); + + test('it allows to pass a custom client', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + }), + ); + setCurrentClient(client); + client.init(); + + const client2 = new TestClient( + getDefaultTestClientOptions({ + dsn: 'https://dsn@ingest.f00.f00/1', + enableSend: true, + defaultIntegrations: false, + }), + ); + client2.init(); + const scope = new Scope(); + scope.setClient(client2); + + const mockTransport = jest.spyOn(client.getTransport()!, 'send'); + const mockTransport2 = jest.spyOn(client2.getTransport()!, 'send'); + + const eventId = captureFeedback( + { + message: 'test', + }, + {}, + scope, + ); + + await client.flush(); + await client2.flush(); + + expect(typeof eventId).toBe('string'); + + expect(mockTransport).not.toHaveBeenCalled(); + expect(mockTransport2).toHaveBeenCalledTimes(1); + }); +}); diff --git a/samples/react-native-macos/src/components/UserFeedbackModal.tsx b/samples/react-native-macos/src/components/UserFeedbackModal.tsx index 60e17af757..1f8dd6a4a3 100644 --- a/samples/react-native-macos/src/components/UserFeedbackModal.tsx +++ b/samples/react-native-macos/src/components/UserFeedbackModal.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { View, StyleSheet, Text, TextInput, Image, Button } from 'react-native'; import * as Sentry from '@sentry/react-native'; -import { UserFeedback } from '@sentry/react-native'; +import { SendFeedbackParams, UserFeedback } from '@sentry/react-native'; export const DEFAULT_COMMENTS = "It's broken again! Please fix it."; @@ -48,6 +48,23 @@ export function UserFeedbackModal(props: { onDismiss: () => void }) { }} /> +