-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Make hydration errors more actionable #26224
Comments
Seems like Next.js 13.2 has improved their error overlays and local debugging experience for these - #24519 (comment) vercel/next.js#45089 seems to be this PR Edit: These are dev only - which still means the production stacktraces still have the same problem - which is what we are looking to improve here. |
Thanks for reaching out.
I believe you should be receiving those in the Is that helpful? Should we be doing more? |
Appreciate the quick response!
So this works pretty decently to a certain extent, we just have to convince framework authors to expose these hooks more easily, as it's better than nothing. For 425 and 418, we get
So here Here's some basic boilerplate that will work to send this stuff to an error monitoring service: function onRecoverableError(error, errInfo) {
let context = {};
if (errInfo && errInfo.componentStack) {
// generating this synthetic error allows services like Sentry to apply sourcemaps
// to unminify the stacktrace and make it readable.
const errorBoundaryError = new Error(error.message);
errorBoundaryError.name = `React ErrorBoundary ${errorBoundaryError.name}`;
errorBoundaryError.stack = errorInfo.componentStack;
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause
error.cause = errorBoundaryError
// you can also just add the plain text as some kind of added event context
context.react = {
componentStack: errorInfo.componentStack
}
}
// feel free to replace with your favourite error monitoring service
Sentry.captureException(error, { context })
} It seems like 423 does not generate a So with that in mind two immediate items:
Seems like this has been a feature request in Next.js, and it seems Astro can also improve here. Remix thankfully seems to be super transparent with exposing
Without proper frames, we can never apply sourcemaps, so users are stuck with minified component names. As for anything else, I'm trying to brainstorm what other tools we can give to people to help identify what to fix here. Ideally most folks just want the line of code that they need to change. If bundle size is a concern from adding new logic, perhaps we can add an API that returns a errored response to the server that CSR had to occur. Then the server could report many details about what kind of HTML it tried to serve, and as along as you can trace the error somehow between backend/frontend, you might be able to debug this faster. |
Aren't these built-in ones like
Or maybe we should patch it onto the object. We already patch
Fair, this could work too.
Can you provide a repro case? I could have a look.
It's not obvious to me why the frames in the stack you mentioned aren't proper. They are reconstructed from actual stack lines merged together so they should give you React component functions. The only missing parts are components like |
Having Even though it would also be cool to have file/lineno/col for individual tags I don't think it is absolutely necessary - without it the component stack is already useful enough. Implementing this would probably mean having to patch the stack trace together somehow using synthetic errors which is probably quite expensive. |
@gaearon apologize for the delay, had gone on vacation last week! made a quick example to help address some of the points you made: https://github.com/AbhiPrasad/nextjs-hydration-error-example using a nextjs app. I had to monkeypatch
You're right that they mostly help. We generally get a
This + the URL of the page is usually good enough to figure out where the issue is. The problem is they aren't solely the built-in components all the time, so we get minified React component names in there for more complex use cases, which is still pretty confusing for users.
This would be great!
When running https://github.com/AbhiPrasad/nextjs-hydration-error-example in production mode, invariant 423 does not generate a console.log(error);
console.log(errorInfo);
console.log(errorInfo.componentStack);
This is partly a Sentry consequence, since we need the filename and line/col number to do source mapping, which helps with the case where there are minified component names in the stacktrace. Something like But we can live without this for now! Finally, when you generate a hydration issue, you get 3 errors thrown:
I understand why all of these errors must be thrown (they all signal different parts of the problem), but IMO the experience of getting all of these in the console/in your production error monitoring system is confusing and can be overwhelming for users, especially those on the more junior side. Perhaps we can think of a way of reducing the amount of errors thrown? |
Should we reduce the errors or describe better how to treat these? Something in the direction of -Text content does not match server-rendered HTML.
+Hydration error: Text content does not match server-rendered HTML.
- Hydration failed because the initial UI does not match what was rendered on the server.
+ Hydration failed because the initial UI does not match what was rendered on the server. To fix this error, fix all the hydration errors above. 423 helps understand the impact. Though it would be nice to understand if 423 implies 418 or 418 implies 423 and what value there is in having these separated. For 425 I understand because we might have multiple 425 but just one 418 if I understood these correctly. |
I feel like the better description is a great first step - especially because it clearly shows how the errors are related together.
Agree here - though it would also be nice to somehow get an idea of the potential consequence of this error. Maybe this is something that Sentry or other performance monitoring tools can help with (show how LCP suffered because 423 fired on a page). A cool idea I had mulling around was that when react does it's client-side re-render, we attach the
Yes, you might have multiple instances of 425, which I think should make sense to the average developer. Getting the |
Heya! I happened on this whilst helping @slorber look into a hydration issue that has surfaced as Docusaurus migrates to React 18: facebook/docusaurus#9379 (comment) We tried plugging in the errorInfo logging in place we got this:
It's more information than we had, but it's still not very actionable. Consider the
It's not obvious what this reveals, beyond that a div is involved. Are there other steps we should be following to get more meaningful output perhaps? The above was obtained by:
Browse to http://localhost:3000/ and opening devtools to look at the console. Interestingly (and not related to this) we've been seeing errors that only seem to surface on Ubuntu which is intriguing. |
For folks using Replay, which has DOM snapshots, we can help you debug this with a diff: https://changelog.getsentry.com/announcements/diff-hydration-errors-with-replay |
Diffing for hydration errors has now been added as a feature to React in this PR by @sebmarkbage: This has also be implemented in Next.js in the error overlay:
@AbhiPrasad does this do enough to address the wishes in this issue? Can the issue be closed now? |
These changes only affect dev mode right? I think production hydration errors still have these same data quality issues. Unfortunately there are a variety of environments where dev mode isn't good enough to debug this, see this twitter thread as an example. |
Just noting that the hydration diff improvements have landed in the React 19 beta
Yes, we do not log the diff in prod because it can include sensitive information. Using something like the DOM snapshot Sentry has (where you already can hide sensitive data) is a good solution for prod. |
Could you explain more about how the diff can include sensitive information? Isn't the error being thrown with details that the client fully knows? Or is this more of a security by obscurity thing? In general I understand the reservations though! Maybe a middle-ground is to expose an API so that users can opt-in to this behaviour if they know what they are doing (in prod bundles)? I can see the argument about how this expands the public API surface too much though. |
The client fully knows them, but the server you're reporting the logs to do not. It's also expensive to generate, so you don't want to take the perf hit in prod when it's already recoverable. |
I find this confusing because can't users send any arbitrary information to 3rd parties from the browser via a In the case of Sentry SDK looking at the DOM snapshot, it does scrub sensitive information client-side before sending a payload to Sentry. Couldn't the Sentry SDK (or other monitoring tools) use a similar strategy to clean up the recoverable error here? I think generally monitoring solutions have built up the competency to be able to remove sensitive information if they are able to access the relevant data in production. There is also a big incentive for our SDKs (and the developer using them) to be able to do this well on our side because of things like GDPR, HIPAA, CCPA, etc.
Yeah this is fair, but I think given hydration means you re-render anyway, isn't performance in a bad state already? Your LCP (and maybe CLS, INP) is going to suffer regardless of if you spend the time generating the error. This is where having it behind a configurable flag would be positive, then users can opt-in to this expense depending on their priorities. I do recognize I have put in 0 work to profile/benchmark this performance-wise, so I may be making incorrect assumptions - just lmk if I'm operating with the wrong mental model! Maybe an interesting follow-up is to try running some numbers on this. |
Yeah, but if you do a It's not just the raw performance of generating the diffs that matters, it's also the additional code and tracking that we need to do to have the data to do the diffs. You can generate the diffs in user-land client-side when we emit the recoverable error by storing the server DOM and reading the client DOM after hydration and construct a diff to upload - and if that sounds expensive that's why we don't include it in prod haha
How would you reliably do that with just a formatted string? My understanding of the way Sentry does this is with attributes, so you have structured data you can reliably omit info from. |
Sorry for the delay, just came back from vacation! I started iterating on a Sentry API for
Actually we do this at Sentry via PII scrubbing on our incoming exceptions. It works relatively well, but you're right that we do have to turn error strings into structured data. It's important to note though that we have prior work on this, I guess the other ask then here is, could we expose this as structured data instead of a logged out string? Could we throw additional fields into |
I do appreciate the need for structured error data, since I've implemented a few error dialogs like LogBox in React Native. But unfortunately the error message is not a public API and we may change it at any time between releases to optimize error creation and output. |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you! |
Hey folks! We at Sentry build error/performance monitoring SDKs and wanted to reach out to see if we could improve the state of hydration errors and make them more actionable. Specifically, we want to look at the stacktraces of hydration errors when you are using production react bundles.
Since React 18 has been getting more adoption, many of our users using React SSR apps have been getting flooded with hydration errors, like listed below:
When we discussed this with the community, many users simply wanted to just filter these errors because they were not actionable.
Looking at the implementation in the codebase a simple string error is thrown:
react/packages/react-reconciler/src/ReactFiberHydrationContext.js
Line 406 in c04b180
and because of how it is generated the stack trace is often not detailed enough to give users insights about what components/functions were problematic.
For example, here's a stacktrace I generated from a stock Next.js application with one of these hydration errors:
https://sentry-test.sentry.io/share/issue/b70ea591bbfc4728b33da495148ac9cd/
As you can see the frames all come from
react-dom
internals, which means users have no way to start investigating where to look beyond the URL of the page.In the end we've decided to filter these out from default from sentry: getsentry/sentry#45038, but we recognize this is not an ideal solution. Hydration errors are things people should fix, and we want to make it easier for people to fix them!
So with that in mind, are there ways we could make this easier for users? Could we attach a
componentStack
like we do with error boundaries for these errors? Could we point users to the element that was causing this issue? Any and all ideas/feeback greatly appreciated.The text was updated successfully, but these errors were encountered: