-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow users to supress hydration warning related errors at the Sentry level #6295
Comments
Thank you for capturing this! 🙇🏼♂️ |
While I can see how these errors are annoying/flooding, usually they are valid and I advise against ignoring them and I am therefore on the fence about whether such an option is a good idea - best practice wise and also maintenance wise. The code snipped is not telling enough about what exactly causes the mismatch between the SSR and the render on the client but I can only assume that somewhere in the render tree something like This is an antipattern that should be avoided. Instead of ignoring these errors, I recommend fixing them by storing them in a static state that is the same on the client and the server and that is then updated on mount (ie with |
So is @lforst saying that the OP's snippet is incomplete? The dynamic portion of this code which causes the warning is actually a dynamic Because my application is massive and I have no idea where my hydration errors are coming from. Been burning through my Sentry quote for a few hundred dollars already with no bandwidth to spend days on this. Would love to ignore all in Sentry like @souredoutlook proposed 🙏 |
So my application is deployed on Vercel and in the code snippet I take a date like |
@dawsbot I guess if your app is too large this ask makes sense. If you really want to ignore the errors via the SDK you can use the |
@lforst would love a copy-pasteable snippet for |
Hey folks, my Sentry dashboard is unusable with the volume of these errors flowing in: I followed the advice from @souredoutlook with no improvement: |
|
@dawsbot Ok just figured out why
We might add an option for this in the future since this is a bit obscure but we will monitor the communitiy's need for this first to avoid API bloat. |
Uploaded! Will know if this helps soon @lforst 🙏 |
did it work @dawsbot ? |
@geryit I cannot tell. My frontend error logging is swamped with these so since there's no hydration warning perhaps the original issue is fixed but it falls through to here? |
@dawsbot I missed that one. Can you try the following if you find the time?:
In general, if you want to ignore particular react errors, you can look them up here: https://github.com/facebook/react/blob/main/scripts/error-codes/codes.json |
I think this worked! Thanks @lforst 🙏 |
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This has literally been driving me crazy for the past 2 months! 😬 Can we reopen this issue, please? At the very least, can we please have some documentation around the It would've saved me days—no kidding! 😅 |
@Merott If you find a place in the docs where you would have found the information the most useful, feel free to open a PR and tag me and I'll review it! |
@lforst I think it would be useful to know how to check for the original error message, which can then be used in the filter. When I looked in the JSON of the error events, the original error messages were nowhere to be found. It seems that they'd been entirely obscured by Sentry's backend processing pipeline. If you'd point me to where/how to find the original message, I'd be happy to open a PR and save the next person's sanity! 😅 |
I am going to re-open this. We will figure out how to improve our product experience around this, stay tuned. |
I've had similar issues for the past 4 days now, that I couldn't get What annoys me most is that the SDK event payload is different from the event payload stored on Sentry's servers. Too much "magic" is happening under the hood, making things confusing. E.g. That leads me to the question, on what field does |
@larsqa Our code for this isn't too complicated if you wanna read up on it:
In this particular case, we have to do the mapping/transformation serverside because we can't have a humungous list of react error code mappings stored in the SDK for bundle size reasons. But as @HazAT already said we will try to find an in-product solution that will (at-least) make these errors way less noisy. Maybe we will also display a hint on how to ignore a particular react error server-side or smth like that. |
Thanks for linking to the source code of This makes the usage of
While this sure helps, there are still plenty of edge cases that your "magic handling" won't cover, i.e. leading us back to above question to be more transparent, less "magic" one has to figure out themselves, and give the developer more control. |
Something that hints of this is definitely needed. I have now spent a significant number of hours and deploys just for React errors to keep popping up the dashboard. While I'm just iterating and confirming what has been said already, particularly misleading was that the JSON blob Sentry gives doesn't match the one sent by the client. What complicated matters even more was that the property path was the exact same (exception > values > value), so its content overwritten by Sentry, which led me in the completely wrong direction. And maybe a hovercard explaining what "originally" means here. |
@lforst @Lms24 @AbhiPrasad Multiple users, some with paid plans, have already raised their concerns (& frustration) that the current solution is just too ambiguous. In some of our integrations, we've wasted more than 20% of our Error Quota within 2 days just because of this issue - Error Quota we pay for! |
@larsqa We are aware of this issue and that it is using quota. Fortunately, you already have the tool in your hands to ignore these errors. See #6295 (comment)
Our stance here is the following:
|
@lforst I understand the concerns and am not arguing for keeping the data. I apologize if I was not clear in this. With that comment and the path, I tried to illustrate what added to the confusion, here it actually overwriting certain values found differently on the client. |
We're going to be looking at making this easier to understand via the UI - task tracking this is here: getsentry/sentry#44877 In essence we're going to tell you exactly how/what to filter directly on the issue details page. |
While this solves the problem of this issue, it doesn't solve the general issue which is raised in here. The |
I agree with @larsqa and am happy to see you are addressing it!
That sounds like a perfect solution, thank you. |
@larsqa As far as I am aware, none. |
Hey, we had more internal conversations around this and decided to, for now, filter out hydration errors by default to not use up quota. Those errors are often not actionable (yet). We might find a way in the future to get more value out of them, but until then, we are filtering them out by default so that you can opt-out. |
I also created an issue in the react repo to see if we can make these errors better/more actionable: facebook/react#26224 Let's see what happens! Feel free to throw your thoughts in there as well. |
Hi everyone! We just released an update on SaaS Sentry that filters out hydration errors by default without you having to change your SDK settings. We decided to filter them because they are super unactionable and very noisy at the moment. Unfortunately react doesn't yet provide a way to determine the location of a hydration error in production mode. We opened an upstream issue for this over at facebook/react#26224. So if you want Sentry to tell you where your hydration errors happen, show that issue some love. As I said, hydration errors should now be filtered by default. If you want to disable this behaviour to have hydration errors show up in your issue stream, you can flip the "Filter out hydration errors" switch on the "Inbound Filters" Project Settings: |
Does it work at Replay? It looks fine at Issue but not working replay. |
@jiwon-mun this solution is an inbound filter, which means the error still might be tagged by the replay when being ingested. @bruno-garcia how does replay work with inbound filtered errors? |
Could we consider filtering these out as well? There are lots of false positives, for example: reduxjs/react-redux#1962. |
@OliverJAsh My immediate instinct is not to filter these out by default. I also don't think the issue you linked is a false positive, at least the way I understood the problem. I recommend you add |
Perhaps "false positive" was a poor choice of words. My understanding is that these errors can indicate performance issues but otherwise they're not errors that affect users. Furthermore, the error contains no information about the source of the issue, meaning the error isn't actionable. As an aside, I spotted this PR which seems to remove these errors (?), although I'm not sure if it's been released yet.
Thanks, we'll give that a go. |
That is actually a very valid reason to ignore the errors which I haven't considered yet. 🤔 Opened an issue: #12432 |
I have tried to ignore hydration errors before sending them to Sentry but nothing worked. Any ideas? 🙏 |
@RomanFausek use the pattern I outlined here: #6295 (comment) |
@lforst Only 421 is required? What about other errors? 422, 423, 425 etc? |
@RomanFausek you can pick which ones to ignore. What I am trying to say is, please do
instead of
|
Problem Statement
Using this code:
A Hydration warning will be raised. This is related to an existing issue in React 18. vercel/next.js#39425
This will result in a high volume of errors being logged by Sentry. which can only be mitigated by setting the supressHydrationWarning option
Solution Brainstorm
While the logged error is expected , the volume of these errors are unwelcome.
If possible, configure the ability to supress hydration warnings (and related errors) at the Sentry level. This would allow users to compromise by keeping the hydration warnings in app but skip reporting these via simple configuration.
The text was updated successfully, but these errors were encountered: