diff --git a/packages/core/src/feedback.ts b/packages/core/src/feedback.ts index adedf9b4d1da..ac78efeaa319 100644 --- a/packages/core/src/feedback.ts +++ b/packages/core/src/feedback.ts @@ -1,23 +1,13 @@ -import type { Attachment, EventHint, FeedbackEvent } from '@sentry/types'; +import type { EventHint, FeedbackEvent, SendFeedbackParams } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { getClient, getCurrentScope } from './currentScopes'; import { createAttachmentEnvelope } from './envelope'; -interface FeedbackParams { - message: string; - name?: string; - email?: string; - attachments?: Attachment[]; - url?: string; - source?: string; - associatedEventId?: string; -} - /** * Send user feedback to Sentry. */ export function captureFeedback( - feedbackParams: FeedbackParams, + feedbackParams: SendFeedbackParams, hint?: EventHint & { includeReplay?: boolean }, ): string { const { message, name, email, url, source, attachments, associatedEventId } = feedbackParams; @@ -26,10 +16,6 @@ export function captureFeedback( const transport = client && client.getTransport(); const dsn = client && client.getDsn(); - if (!client || !transport || !dsn) { - throw new Error('Invalid Sentry client'); - } - const feedbackEvent: FeedbackEvent = { contexts: { feedback: dropUndefinedKeys({ @@ -54,7 +40,7 @@ export function captureFeedback( // For now, we have to send attachments manually in a separate envelope // Because we do not support attachments in the feedback envelope // Once the Sentry API properly supports this, we can get rid of this and send it through the event envelope - if (client && attachments && attachments.length) { + if (client && transport && dsn && attachments && attachments.length) { // TODO: https://docs.sentry.io/platforms/javascript/enriching-events/attachments/ // eslint-disable-next-line @typescript-eslint/no-floating-promises void transport.send( diff --git a/packages/core/test/lib/feedback.test.ts b/packages/core/test/lib/feedback.test.ts new file mode 100644 index 000000000000..563fc812a138 --- /dev/null +++ b/packages/core/test/lib/feedback.test.ts @@ -0,0 +1,383 @@ +import { Span } from '@sentry/types'; +import { getCurrentScope, setCurrentClient, startSpan } from '../../src'; +import { captureFeedback } from '../../src/feedback'; +import { TestClient, getDefaultTestClientOptions } 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), + trace: { + environment: 'production', + public_key: 'dsn', + trace_id: 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), + 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), + trace: { + environment: 'production', + public_key: 'dsn', + trace_id: 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), + 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(2); + + const [feedbackEnvelope, attachmentEnvelope] = mockTransport.mock.calls; + + // Feedback event is sent normally in one envelope + expect(feedbackEnvelope[0]).toEqual([ + { + event_id: eventId, + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'dsn', + trace_id: 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), + type: 'feedback', + }, + ], + ], + ]); + + // Attachments are sent in separate envelope + expect(attachmentEnvelope[0]).toEqual([ + { + event_id: eventId, + sent_at: expect.any(String), + }, + [ + [ + { + 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), + trace: { + trace_id: traceId, + span_id: spanId, + sampled: 'true', + }, + }, + [ + [ + { 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), + 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), + trace: { + trace_id: traceId, + environment: 'production', + public_key: 'dsn', + sampled: 'true', + sample_rate: '1', + transaction: 'test-span', + }, + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + trace_id: traceId, + span_id: spanId, + data: { + 'sentry.origin': 'manual', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + }, + origin: 'manual', + }, + feedback: { + message: 'test', + }, + }, + level: 'info', + environment: 'production', + event_id: eventId, + timestamp: expect.any(Number), + type: 'feedback', + }, + ], + ], + ]); + }); +}); diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index 473028ea4b12..8063dab46592 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -77,13 +77,14 @@ export class TestClient extends BaseClient { public sendEvent(event: Event, hint?: EventHint): void { this.event = event; - // In real life, this will get deleted as part of envelope creation. - delete event.sdkProcessingMetadata; - if (this._options.enableSend) { super.sendEvent(event, hint); return; } + + // In real life, this will get deleted as part of envelope creation. + delete event.sdkProcessingMetadata; + TestClient.sendEventCalled && TestClient.sendEventCalled(event); } diff --git a/packages/feedback/src/core/sendFeedback.test.ts b/packages/feedback/src/core/sendFeedback.test.ts index 2b139874783f..a59054f278a3 100644 --- a/packages/feedback/src/core/sendFeedback.test.ts +++ b/packages/feedback/src/core/sendFeedback.test.ts @@ -30,7 +30,58 @@ describe('sendFeedback', () => { patchedDecoder && delete global.window.TextDecoder; }); - it('sends feedback', async () => { + it('sends feedback with minimal options', async () => { + mockSdk(); + const mockTransport = jest.spyOn(getClient()!.getTransport()!, 'send'); + + const promise = sendFeedback({ + message: 'mi', + }); + + expect(promise).toBeInstanceOf(Promise); + + const eventId = await promise; + + expect(typeof eventId).toEqual('string'); + + expect(mockTransport).toHaveBeenCalledWith([ + { + event_id: expect.any(String), + sent_at: expect.any(String), + trace: { + trace_id: expect.any(String), + environment: 'production', + public_key: 'dsn', + }, + }, + [ + [ + { type: 'feedback' }, + { + breadcrumbs: undefined, + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + feedback: { + message: 'mi', + source: 'api', + url: 'http://localhost/', + }, + }, + level: 'info', + environment: 'production', + event_id: expect.any(String), + timestamp: expect.any(Number), + type: 'feedback', + }, + ], + ], + ]); + }); + + it('sends feedback with full options', async () => { mockSdk(); const mockTransport = jest.spyOn(getClient()!.getTransport()!, 'send'); @@ -38,6 +89,9 @@ describe('sendFeedback', () => { name: 'doe', email: 're@example.org', message: 'mi', + url: 'http://example.com/', + source: 'custom-source', + associatedEventId: '1234', }); expect(promise).toBeInstanceOf(Promise); @@ -67,11 +121,12 @@ describe('sendFeedback', () => { trace_id: expect.any(String), }, feedback: { + name: 'doe', contact_email: 're@example.org', message: 'mi', - name: 'doe', - source: 'api', - url: 'http://localhost/', + url: 'http://example.com/', + source: 'custom-source', + associated_event_id: '1234', }, }, level: 'info', diff --git a/packages/feedback/src/core/sendFeedback.ts b/packages/feedback/src/core/sendFeedback.ts index 3d99abfb5a70..7feff8b98e1e 100644 --- a/packages/feedback/src/core/sendFeedback.ts +++ b/packages/feedback/src/core/sendFeedback.ts @@ -9,10 +9,10 @@ import { FEEDBACK_API_SOURCE } from '../constants'; * Public API to send a Feedback item to Sentry */ export const sendFeedback: SendFeedback = ( - { name, email, message, attachments, source = FEEDBACK_API_SOURCE, url = getLocationHref() }: SendFeedbackParams, + options: SendFeedbackParams, { includeReplay = true } = {}, ): Promise => { - if (!message) { + if (!options.message) { throw new Error('Unable to submit feedback with empty message'); } @@ -23,7 +23,14 @@ export const sendFeedback: SendFeedback = ( throw new Error('No client setup, cannot send feedback.'); } - const eventId = captureFeedback({ name, email, message, attachments, source, url }, { includeReplay }); + const eventId = captureFeedback( + { + source: FEEDBACK_API_SOURCE, + url: getLocationHref(), + ...options, + }, + { includeReplay }, + ); // We want to wait for the feedback to be sent (or not) return new Promise((resolve, reject) => { diff --git a/packages/types/src/feedback/sendFeedback.ts b/packages/types/src/feedback/sendFeedback.ts index 1dc0d6fa4369..3ea126eff693 100644 --- a/packages/types/src/feedback/sendFeedback.ts +++ b/packages/types/src/feedback/sendFeedback.ts @@ -39,6 +39,7 @@ export interface SendFeedbackParams { attachments?: Attachment[]; url?: string; source?: string; + associatedEventId?: string; } interface SendFeedbackOptions {