Skip to content

Commit ffe2d6c

Browse files
antonisclaudealwx
authored
fix(feedback): Guard show/hide against uninitialized _setVisibility (#6435)
* fix(feedback): Guard show/hide against uninitialized _setVisibility When Sentry.showFeedbackForm() (or the equivalent Button/Screenshot show/hide) is called before FeedbackFormProvider has mounted and run initialize(), the static _setVisibility was undefined. The guard `_setVisibility !== NOOP_SET_VISIBILITY` evaluated true and the code called undefined(true), throwing TypeError. Default the static field to NOOP_SET_VISIBILITY at declaration so the guard correctly falls through to the intended console.warn fallback. Fixes SDK-CRASHES-REACT-NATIVE-62C Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: Add CHANGELOG entry for #6435 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Alexander <alex@apantiukhov.com>
1 parent 208bf09 commit ffe2d6c

3 files changed

Lines changed: 27 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
### Fixes
1616

17+
- Fix `TypeError` when `showFeedbackForm`/`showFeedbackButton`/`showScreenshotButton` is called before `FeedbackFormProvider` mounts ([#6435](https://github.com/getsentry/sentry-react-native/pull/6435))
1718
- Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
1819

1920
## 8.18.0

packages/core/src/js/feedback/FeedbackFormManager.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ const NOOP_SET_VISIBILITY = (): void => {
1919

2020
abstract class FeedbackManager {
2121
protected static _isVisible = false;
22-
protected static _setVisibility: (visible: boolean) => void;
22+
// Default to NOOP so `show()`/`hide()` don't call `undefined(...)` when
23+
// invoked before `FeedbackFormProvider` mounts and calls `initialize()`.
24+
protected static _setVisibility: (visible: boolean) => void = NOOP_SET_VISIBILITY;
2325

2426
protected static get _feedbackComponentName(): string {
2527
throw new Error('Subclasses must override feedbackComponentName');

packages/core/test/feedback/FeedbackFormManager.test.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,29 @@ describe('FeedbackFormManager', () => {
8787
}).not.toThrow();
8888
});
8989

90+
it('showFeedbackForm does not throw before any provider or reset has run', () => {
91+
// Regression for SDK-CRASHES-REACT-NATIVE-62C: `_setVisibility` used to be
92+
// uninitialized, so calling show() before FeedbackFormProvider mounted threw
93+
// `TypeError: this._setVisibility is not a function`. Load the module fresh
94+
// so the class statics have never been touched by other tests.
95+
jest.isolateModules(() => {
96+
// eslint-disable-next-line @typescript-eslint/no-require-imports
97+
const freshManager = require('../../src/js/feedback/FeedbackFormManager');
98+
expect(() => freshManager.showFeedbackForm()).not.toThrow();
99+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("requires 'Sentry.wrap(RootComponent)'"));
100+
});
101+
});
102+
103+
it('hideFeedbackForm does not throw before any provider or reset has run', () => {
104+
// Symmetric to show(): hide() reads the same uninitialized static field.
105+
jest.isolateModules(() => {
106+
// eslint-disable-next-line @typescript-eslint/no-require-imports
107+
const freshManager = require('../../src/js/feedback/FeedbackFormManager');
108+
expect(() => freshManager.FeedbackFormManager.hide()).not.toThrow();
109+
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("requires 'Sentry.wrap(RootComponent)'"));
110+
});
111+
});
112+
90113
it('showFeedbackForm displays the form with the feedbackIntegration options', async () => {
91114
mockedIsModalSupported.mockReturnValue(true);
92115
const { getByPlaceholderText, getByText } = render(

0 commit comments

Comments
 (0)