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..0275957452 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,104 @@ 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. A reopen is the same draft + // resumed (the screenshot flow closes and reopens the form, restoring the draft; only a submit + // discards it), so it continues the earlier flush: while its upload is still in flight the reopen + // reuses it, and once it has settled the reopen carries its uploaded id forward (upgrading to its + // own if that capture uploads a fresher replay). That keeps the replay already sent for this draft + // instead of dropping it on a reopen that uploads nothing (e.g. an on-error sampling miss). The + // flush is cleared on submit, so an unrelated later feedback never inherits it. Wrapped: a + // synchronous bridge throw must not propagate out of the widget's `componentDidMount`. + function flushReplayForFeedbackWidget(): void { + try { + if (feedbackReplayFlush && !feedbackReplayFlush.resolved) { + return; + } + const carried = feedbackReplayFlush?.resolved ? feedbackReplayFlush.id : null; + const flush: { openId: string | null; id: string | null; resolved: boolean } = { + // Prefer a carried id as the pre-resolve fallback: it is a replay already sent for this + // draft, so a submit while this reopen's capture is still in flight must not fall back to + // the current (unconfirmed, maybe-never-sent) buffer id instead. + openId: carried ?? NATIVE.getCurrentReplayId(), + id: carried, + 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 => { + // Keep the carried id when this reopen uploaded nothing, so it isn't dropped. + if (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 +733,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..0c73a0f0ac 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,354 @@ 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('keeps the replay a draft already uploaded when a reopen flush uploads nothing', async () => { + // The form reopens for the same draft (e.g. the screenshot flow). The first open uploaded a + // replay; the reopen's flush uploads nothing (e.g. a sampling miss). The submission must still + // link the replay already sent for this draft rather than drop it. + mockCaptureReplay.mockResolvedValueOnce('first-id').mockResolvedValueOnce(null); + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // first open uploads 'first-id' + await flushAsync(); + getOpenFeedbackWidget()(); // reopen (same draft), its own flush uploads nothing + await flushAsync(); + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); + + expect(feedback.contexts.feedback.replay_id).toBe('first-id'); + }); + + it('links the carried id, not the current buffer, when submitting during a reopen flush', async () => { + // First open uploads 'first-id'. The form reopens and its capture is still in flight when the + // user submits, while a current buffer id exists. The submission must link the replay already + // sent for this draft ('first-id'), not the unconfirmed current buffer. + let resolveReopen!: (v: string | null) => void; + mockCaptureReplay.mockResolvedValueOnce('first-id').mockReturnValueOnce( + new Promise(resolve => { + resolveReopen = resolve; + }), + ); + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); // first open uploads 'first-id' + await flushAsync(); + + mockGetCurrentReplayId.mockReturnValue('current-buffer'); // a buffer exists at reopen + getOpenFeedbackWidget()(); // reopen, capture still pending + + const feedback = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(feedback, { includeReplay: true }); // submit before the reopen settles + expect(feedback.contexts.feedback.replay_id).toBe('first-id'); + + // The reopen's capture then uploads nothing; nothing to revive, nothing changes. + resolveReopen(null); + await flushAsync(); + }); + + it('does not revive a consumed flush for an unrelated later feedback', async () => { + // A widget submit consumes (clears) the flush. A later widget feedback that uploads nothing + // must not inherit the previous submission's replay id. + mockCaptureReplay.mockResolvedValueOnce('first-id').mockResolvedValueOnce(null); + mockGetCurrentReplayId.mockReturnValue(null); + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + getOpenFeedbackWidget()(); + await flushAsync(); + const first = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(first, { includeReplay: true }); // submit consumes the flush + expect(first.contexts.feedback.replay_id).toBe('first-id'); + + getOpenFeedbackWidget()(); // a fresh, unrelated open whose flush uploads nothing + await flushAsync(); + const second = makeFeedbackEvent('widget'); + getBeforeSendFeedback()(second, { includeReplay: true }); + expect(second.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', () => {