fix(feedback): Prevent removeFromDom() from throwing
#16030
+24
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
❌ We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format. |
chargome
reviewed
Apr 11, 2025
| logger.error( | ||
| '[Feedback] Error when trying to remove Actor from the DOM. It is not appended to the DOM yet!', | ||
| ); | ||
| throw new Error('[Feedback] Actor is not appended to DOM, nothing to remove.'); |
There was a problem hiding this comment.
Why do you want to throw this one here?
There was a problem hiding this comment.
turns out el.remove() doesn't even throw if there's no parent element. unlike parent.removeChild(child).
So now if people call removeFromDom() we'll basically guarantee that the elements will be removed, and there'll be no errors thrown.
Similar to the appendToDom() call, SDK users can't really tell what state things are currently in, but they can call methods to put things into the state you want to be in.
ryan953
force-pushed
the
ryan953/fix-feedback-throws
branch
from
7a904c7 to
18c9875
Compare
ryan953
force-pushed
the
ryan953/fix-feedback-throws
branch
from
96483aa to
0463304
Compare
ryan953
force-pushed
the
ryan953/fix-feedback-throws
branch
from
0463304 to
b85449b
Compare
size-limit report 📦
|
ryan953
changed the title
removeFromDom() from throwing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
So something prevented it from opening all the way up.
Fixes getsentry/sentry#89424