Skip to content

Commit b94062f

Browse files
authored
fix(feedback): Prevent removeFromDom() from throwing (#16030)
An internal user reported seeing "NotFoundError: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node." and the replay of their session confirms it: https://sentry.sentry.io/explore/replays/127444034ae84099a84b524458b6dd90/ However, there is no matching JS error to give more clues. What I think happened is that after interacting with the Feedback SDK the widget got into a state where it was no longer mounted into the page, but we called `removeFromDom()` anyway. Looking at the replay i saw the moment when the feedback dom was aded to the page, but it wasn't visible, only this got added at 18:51: <img width="265" alt="SCR-20250411-lsav" src="https://github.com/user-attachments/assets/1f65a174-1a8a-405c-8ce4-be0a10d09a64" /> So something prevented it from opening all the way up. Fixes getsentry/sentry#89424
1 parent de2f4b5 commit b94062f

File tree

3 files changed

+24
-4
lines changed

3 files changed

+24
-4
lines changed

packages/feedback/src/core/components/Actor.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ export function Actor({ triggerLabel, triggerAriaLabel, shadow, styleNonce }: Ac
4646
shadow.appendChild(el);
4747
},
4848
removeFromDom(): void {
49-
shadow.removeChild(el);
50-
shadow.removeChild(style);
49+
el.remove();
50+
style.remove();
5151
},
5252
show(): void {
5353
el.ariaHidden = 'false';

packages/feedback/src/modal/integration.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ export const feedbackModalIntegration = ((): FeedbackModalIntegration => {
4444
}
4545
},
4646
removeFromDom(): void {
47-
shadowRoot.removeChild(el);
48-
shadowRoot.removeChild(style);
47+
el.remove();
48+
style.remove();
4949
DOCUMENT.body.style.overflow = originalOverflow;
5050
},
5151
open() {

packages/feedback/test/core/components/Actor.test.ts

+20
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,24 @@ describe('Actor', () => {
6262
expect(actorAria.el.textContent).toBe('Button');
6363
expect(actorAria.el.ariaLabel).toBe('Aria');
6464
});
65+
66+
it('does not throw if removeFromDom() is called when it is not mounted', () => {
67+
const feedbackIntegration = buildFeedbackIntegration({
68+
lazyLoadIntegration: vi.fn(),
69+
});
70+
71+
const configuredIntegration = feedbackIntegration({});
72+
mockSdk({
73+
sentryOptions: {
74+
integrations: [configuredIntegration],
75+
},
76+
});
77+
78+
const feedback = getFeedback();
79+
80+
const actorComponent = feedback!.createWidget();
81+
82+
expect(() => actorComponent.removeFromDom()).not.toThrowError();
83+
expect(() => actorComponent.removeFromDom()).not.toThrowError();
84+
});
6585
});

0 commit comments

Comments
 (0)