diff --git a/CHANGELOG.md b/CHANGELOG.md index 6830378a55..050c78d344 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ ### Fixes +- Attach the on-error Session Replay buffer to feedback submitted from the feedback form ([#6870](https://github.com/getsentry/sentry-react-native/pull/6870)) - Do not mark `sendDefaultPii` as deprecated in the React Native options ([#6864](https://github.com/getsentry/sentry-react-native/pull/6864)) ### Dependencies diff --git a/packages/core/src/js/feedback/FeedbackForm.tsx b/packages/core/src/js/feedback/FeedbackForm.tsx index a7e2c2a23a..0b3a11f093 100644 --- a/packages/core/src/js/feedback/FeedbackForm.tsx +++ b/packages/core/src/js/feedback/FeedbackForm.tsx @@ -156,6 +156,7 @@ export class FeedbackForm extends React.Component { this.forceUpdate(); }); + + // Signals the Session Replay integration to upload the buffered replay so it's ready to attach + // when the form is submitted. + getClient()?.emit('openFeedbackWidget'); } /** diff --git a/packages/core/src/js/replay/mobilereplay.ts b/packages/core/src/js/replay/mobilereplay.ts index a2b79a6edc..b9b73f1520 100644 --- a/packages/core/src/js/replay/mobilereplay.ts +++ b/packages/core/src/js/replay/mobilereplay.ts @@ -1,3 +1,4 @@ +/* oxlint-disable eslint(max-lines) */ import type { Breadcrumb, BreadcrumbHint, @@ -6,6 +7,7 @@ import type { ErrorEvent, Event, EventHint, + FeedbackEvent, Metric, } from '@sentry/core'; @@ -381,6 +383,13 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau // This will be updated when we know the replay ID changes (e.g., after captureReplay) let cachedReplayId: string | null = null; + // The replay upload started when the feedback widget opened. `openId` is the buffer id read + // before the flush (what it uploads, and the fallback for a submit before it resolves); `id` is + // the uploaded replay id once resolved (`null` on a sampling miss). A fresh object per open means + // a superseded or already-consumed flush can't affect a later feedback — its late resolution + // mutates an object no one reads any more. + let feedbackReplayFlush: { openId: string | null; id: string | null; resolved: boolean } | null = null; + function updateCachedReplayId(replayId: string | null): void { cachedReplayId = replayId; } @@ -514,6 +523,95 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau } } + // Start uploading the buffer when the widget opens. Read the buffer id first (before capture may + // restart buffering) so the id we link matches what this flush uploads. Each open starts a fresh + // flush, except that a remount while an earlier open's upload is still in flight (e.g. the + // screenshot flow closes and reopens the form) reuses it rather than discarding the pending + // upload. A new open never inherits a previous open's completed id: that id belongs to the form + // that opened it, so reusing it could link a replay from a form the user already dismissed. + // Wrapped: a synchronous bridge throw must not propagate out of the widget's `componentDidMount`. + function flushReplayForFeedbackWidget(): void { + try { + if (feedbackReplayFlush && !feedbackReplayFlush.resolved) { + return; + } + const flush: { openId: string | null; id: string | null; resolved: boolean } = { + openId: NATIVE.getCurrentReplayId(), + id: null, + resolved: false, + }; + feedbackReplayFlush = flush; + // On settle, mark resolved and drop the cached id: capture may change the native replay + // identity, so the next DSC/metric re-reads the current id (as the runtime controls do) rather + // than caching the uploaded one. + const settle = (): void => { + flush.resolved = true; + invalidateCachedReplayId(); + }; + NATIVE.captureReplay(false) + .then(uploadedId => { + flush.id = uploadedId; + settle(); + }) + .then(undefined, (error: unknown) => { + settle(); + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to flush replay for feedback widget`, error); + }); + } catch (error) { + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to start replay flush for feedback widget`, error); + } + } + + // Emitted synchronously before the event is sent, so resolve the id synchronously. Widget + // feedback links the id the open flush uploaded — `null` on a sampling miss means no replay was + // sent, so nothing is linked; before it resolves, fall back to the id read at open. Other + // feedback (direct `captureFeedback`) ignores the widget flush and links the current recording, + // flushing it now. Wrapped: a synchronous bridge throw must not abort feedback capture. + function linkReplayToFeedback(feedback: FeedbackEvent, options?: { includeReplay?: boolean }): void { + if (!options?.includeReplay) { + return; + } + + let replayId: string | null = null; + try { + if (feedback.contexts?.feedback?.source === 'widget') { + const flush = feedbackReplayFlush; + feedbackReplayFlush = null; + replayId = flush?.resolved ? flush.id : (flush?.openId ?? null); + } else { + replayId = NATIVE.getCurrentReplayId(); + if (replayId) { + // Flush the current buffer and, like the widget flush, invalidate the cached id on settle: + // capture may change the native replay identity, so the next DSC/metric re-reads it. + NATIVE.captureReplay(false).then( + () => { + invalidateCachedReplayId(); + }, + (error: unknown) => { + invalidateCachedReplayId(); + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to flush replay for feedback`, error); + }, + ); + } + } + } catch (error) { + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to resolve replay for feedback`, error); + return; + } + + if (!replayId) { + debug.log(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} no active recording to link to feedback.`); + return; + } + + feedback.contexts = feedback.contexts || {}; + feedback.contexts.feedback = { + ...feedback.contexts.feedback, + replay_id: replayId, + }; + debug.log(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} linked replay ${replayId} to feedback ${feedback.event_id}.`); + } + function setup(client: Client): void { if (!hasHooks(client)) { return; @@ -626,6 +724,10 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau // errors are logged inside flushReplayForSentEvent flushReplayForSentEvent(event).then(undefined, () => {}); }); + + // Attach the Session Replay to user feedback: upload on open, link on submit. + client.on('openFeedbackWidget', flushReplayForFeedbackWidget); + client.on('beforeSendFeedback', linkReplayToFeedback); } function getReplayId(): string | null { diff --git a/packages/core/test/feedback/FeedbackForm.test.tsx b/packages/core/test/feedback/FeedbackForm.test.tsx index 8cbf7ceaf3..d502f15792 100644 --- a/packages/core/test/feedback/FeedbackForm.test.tsx +++ b/packages/core/test/feedback/FeedbackForm.test.tsx @@ -345,12 +345,22 @@ describe('FeedbackForm', () => { message: 'This is a feedback message.', name: 'John Doe', email: 'john.doe@example.com', + source: 'widget', }, - undefined, + { includeReplay: true }, ); }); }); + it('emits openFeedbackWidget on mount so the replay buffer can be primed', () => { + const client = getClient()!; + const emitSpy = jest.spyOn(client, 'emit'); + + render(); + + expect(emitSpy).toHaveBeenCalledWith('openFeedbackWidget'); + }); + it('shows success message when the form is submitted successfully', async () => { const { getByPlaceholderText, getByText } = render(); diff --git a/packages/core/test/replay/mobilereplay.test.ts b/packages/core/test/replay/mobilereplay.test.ts index 60423b9fbf..04c388fd1a 100644 --- a/packages/core/test/replay/mobilereplay.test.ts +++ b/packages/core/test/replay/mobilereplay.test.ts @@ -6,6 +6,7 @@ import type { ErrorEvent, Event, EventHint, + FeedbackEvent, } from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; @@ -1307,6 +1308,304 @@ describe('Mobile Replay Integration', () => { expect(id3).toBe(cachedReplayId); }); }); + + describe('feedback replay', () => { + const getOpenFeedbackWidget = (): (() => void) => { + const call = mockOn.mock.calls.find(c => c[0] === 'openFeedbackWidget'); + return call![1] as () => void; + }; + + const getBeforeSendFeedback = (): ((feedback: FeedbackEvent, options?: { includeReplay?: boolean }) => void) => { + const call = mockOn.mock.calls.find(c => c[0] === 'beforeSendFeedback'); + return call![1] as (feedback: FeedbackEvent, options?: { includeReplay?: boolean }) => void; + }; + + const makeFeedbackEvent = (source: string = 'widget'): FeedbackEvent => + ({ + event_id: 'test-feedback-id', + type: 'feedback', + contexts: { feedback: { message: 'hi', source } }, + }) as unknown as FeedbackEvent; + + it('registers the openFeedbackWidget and beforeSendFeedback hooks', () => { + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + expect(mockOn.mock.calls.some(c => c[0] === 'openFeedbackWidget')).toBe(true); + expect(mockOn.mock.calls.some(c => c[0] === 'beforeSendFeedback')).toBe(true); + }); + + it('flushes the on-error buffer (not a hard crash) when the widget opens', async () => { + mockCaptureReplay.mockResolvedValue('feedback-replay-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); + await flushAsync(); + + expect(mockCaptureReplay).toHaveBeenCalledTimes(1); + expect(mockCaptureReplay).toHaveBeenCalledWith(false); + }); + + it('links the uploaded replay to widget feedback after the open flush resolves', async () => { + mockCaptureReplay.mockResolvedValue('uploaded-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); + await flushAsync(); + mockCaptureReplay.mockClear(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBe('uploaded-id'); + // The widget already flushed on open; a widget submit must not flush again. + expect(mockCaptureReplay).not.toHaveBeenCalled(); + }); + + it('does not link a replay when includeReplay is not set', async () => { + mockCaptureReplay.mockResolvedValue('uploaded-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); + await flushAsync(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback); + + expect(feedback.contexts.feedback.replay_id).toBeUndefined(); + }); + + it('flushes at submit time for API feedback without a widget open', () => { + mockGetCurrentReplayId.mockReturnValue('buffer-replay-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + mockCaptureReplay.mockClear(); + + const feedback = makeFeedbackEvent('api'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + // Current recording id linked synchronously ... + expect(feedback.contexts.feedback.replay_id).toBe('buffer-replay-id'); + // ... and the upload is triggered fire-and-forget. + expect(mockCaptureReplay).toHaveBeenCalledWith(false); + }); + + it('does not link a replay when there is no active recording', () => { + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + mockCaptureReplay.mockClear(); + + const feedback = makeFeedbackEvent('api'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBeUndefined(); + expect(mockCaptureReplay).not.toHaveBeenCalled(); + }); + + it('does not link a replay to widget feedback when the open flush is a sampling miss', async () => { + mockGetCurrentReplayId.mockReturnValue('buffer-id'); + mockCaptureReplay.mockResolvedValue(null); // on-error sampling miss: nothing uploaded + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); + await flushAsync(); + mockCaptureReplay.mockClear(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBeUndefined(); + expect(mockCaptureReplay).not.toHaveBeenCalled(); + }); + + it('uses the latest open flush when the form remounts (e.g. screenshot flow)', async () => { + mockCaptureReplay.mockResolvedValueOnce('first-id').mockResolvedValueOnce('second-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // first open + await flushAsync(); + getOpenFeedbackWidget()(); // remount supersedes the first flush + await flushAsync(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBe('second-id'); + }); + + it('does not leak a cancelled widget flush onto later API feedback', async () => { + mockCaptureReplay.mockResolvedValue('widget-upload-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // widget opens, starts a flush + await flushAsync(); + + // Widget cancelled (no submit). A later direct captureFeedback should use the CURRENT + // recording and flush it, not the widget flush. + mockGetCurrentReplayId.mockReturnValue('current-buffer-id'); + mockCaptureReplay.mockClear(); + const api = makeFeedbackEvent('api'); + getBeforeSendFeedback()(api, { includeReplay: true }); + + expect(api.contexts.feedback.replay_id).toBe('current-buffer-id'); + expect(mockCaptureReplay).toHaveBeenCalledWith(false); + }); + + it('uses the id read at open (not at submit) when the flush is still pending', () => { + mockGetCurrentReplayId.mockReturnValue('open-buffer'); + mockCaptureReplay.mockReturnValueOnce(new Promise(() => {})); // never resolves + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // openId = 'open-buffer' + + // The native buffer rolls over before the user submits. + mockGetCurrentReplayId.mockReturnValue('new-buffer'); + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBe('open-buffer'); + }); + + it('does not revive a consumed widget flush after it resolves late', async () => { + mockGetCurrentReplayId.mockReturnValue('open-buffer'); + let resolveOpenFlush!: (v: string | null) => void; + mockCaptureReplay.mockReturnValueOnce( + new Promise(resolve => { + resolveOpenFlush = resolve; + }), + ); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // open flush pending (unresolved) + + // Submit before the flush resolves: falls back to the id read at open. + const first = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(first, { includeReplay: true }); + expect(first.contexts.feedback.replay_id).toBe('open-buffer'); + + // The open flush resolves late; it must not revive the consumed flush for a later feedback. + resolveOpenFlush('late-uploaded-id'); + await flushAsync(); + + const second = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(second, { includeReplay: true }); + expect(second.contexts.feedback.replay_id).toBeUndefined(); + }); + + it('invalidates the cached replay id after a widget flush instead of caching the uploaded id', async () => { + mockGetCurrentReplayId.mockReturnValue('id-before'); + mockCaptureReplay.mockResolvedValue('uploaded-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); // cache initialized to 'id-before' + + getOpenFeedbackWidget()(); + // The native replay identity changes during the capture. + mockGetCurrentReplayId.mockReturnValue('id-after'); + await flushAsync(); + + // The flush invalidated the cache, so the next read reflects the current native id — neither + // the stale pre-flush id nor the uploaded id. + expect(integration.getReplayId()).toBe('id-after'); + }); + + it('invalidates the cached replay id after an API feedback flush', async () => { + mockGetCurrentReplayId.mockReturnValue('id-before'); + mockCaptureReplay.mockResolvedValue('uploaded-id'); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); // cache initialized to 'id-before' + + const feedback = makeFeedbackEvent('api'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + expect(feedback.contexts.feedback.replay_id).toBe('id-before'); + + // The native replay identity changes during the capture. + mockGetCurrentReplayId.mockReturnValue('id-after'); + await flushAsync(); + + expect(integration.getReplayId()).toBe('id-after'); + }); + + it('does not link a replay from a dismissed form when a later open uploads nothing', async () => { + // First open (a form the user dismisses) uploads a replay; a later open's flush uploads + // nothing (e.g. a sampling miss). The later submission must not inherit the dismissed form's + // replay id - it belongs to a form the user already closed. + mockCaptureReplay.mockResolvedValueOnce('dismissed-form-id').mockResolvedValueOnce(null); + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // first open uploads 'dismissed-form-id' + await flushAsync(); + getOpenFeedbackWidget()(); // later open, its own flush uploads nothing + await flushAsync(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBeUndefined(); + }); + + it('keeps the in-flight open flush when the form remounts before it settles', async () => { + // The first open's capture is still pending when the form remounts. + let resolveFirst!: (v: string | null) => void; + mockCaptureReplay.mockReturnValueOnce( + new Promise(resolve => { + resolveFirst = resolve; + }), + ); + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // first open, flush in flight + getOpenFeedbackWidget()(); // remount before the first settles + + // The remount reuses the in-flight flush instead of starting another. + expect(mockCaptureReplay).toHaveBeenCalledTimes(1); + + resolveFirst('in-flight-id'); + await flushAsync(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBe('in-flight-id'); + }); + + it('does not propagate a synchronous bridge error when the widget opens', () => { + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + // The bridge starts throwing only after setup (which reads the id once). + mockGetCurrentReplayId.mockImplementation(() => { + throw new Error('bridge boom'); + }); + + expect(() => getOpenFeedbackWidget()()).not.toThrow(); + }); + + it('does not abort feedback capture when a synchronous bridge error occurs', () => { + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + mockGetCurrentReplayId.mockImplementation(() => { + throw new Error('bridge boom'); + }); + + const feedback = makeFeedbackEvent('api'); + expect(() => getBeforeSendFeedback()(feedback, { includeReplay: true })).not.toThrow(); + expect(feedback.contexts.feedback.replay_id).toBeUndefined(); + }); + }); }); describe('serializeNetworkDetailUrlsForNative', () => {