search

facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
224.72k stars 45.84k forks source link

Provide ways to do post-mortem analysis of “Maximum update depth exceeded” error in production. #12811

Open dtinth opened 6 years ago

dtinth commented 6 years ago

Do you want to request a feature or report a bug? Feature

What is the current behavior? Our error logging systems has been reporting this error in production: “Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.”

However, we can’t reliably reproduce this error and it only affects a small percentage of our users. Moreover, we have more than 1,000 in-house components and several third-party components. So, it’s impractical to audit every single component to find out what caused it.

What is the expected behavior? It would be much easier for us to debug if, when the nested update count exceeds 1,000 (current NESTED_UPDATE_LIMIT), we could see what components are involved in this nested update chain.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? We are using React 16.3.1.

aweary commented 6 years ago

Hey @dtinth!

It would be much easier for us to debug if, when the nested update count exceeds 1,000 (current NESTED_UPDATE_LIMIT), we could see what components are involved in this nested update chain.

We could include the name of the component that was being worked on when the error was thrown, but if you're minifying your production build (which hopefully you are) this would probably be useless anyways?

We also only track the "component stack" that you get in warnings in DEV, so that data isn't available to include in the error message. @bvaughn could the production profiling build maybe be a way to help narrow down errors in production? Cross reference the timing of profiling data with error logs.

dtinth commented 6 years ago

Hello, thanks for your reply. 😄

bvaughn commented 6 years ago

We also only track the "component stack" that you get in warnings in DEV, so that data isn't available to include in the error message. @bvaughn could the production profiling build maybe be a way to help narrow down errors in production? Cross reference the timing of profiling data with error logs.

Not sure how useful this would be.

I don't think we would want to add any DEV-only behavior into the profiling build (other than profile times) because the purpose of that build is to be almost as fast as production.

As for the onRender callback itself being used to narrow down the cause of the error- maybe. But the callback wouldn't be fired for the actual render that fails- because this render would never be committed. So at best, you might be able to determine something about the last time the callback was fired.

aweary commented 6 years ago

I don't think we would want to add any DEV-only behavior into the profiling build (other than profile times) because the purpose of that build is to be almost as fast as production.

Yeah, I wasn't suggesting adding DEV-only behavior like the component stack. Just cross-referencing the timestamp of the profile data with the error logs, using the ID to narrow down what was rendering when the error occurred. But it looks like that wouldn't really work well, so nevermind :)

@dtinth

About component names, we can codemod or Babel all our React components to include a displayName property. As a last resort (e.g. in 3rd-party library), dumping the component’s render function source code might be helpful, even in its minified form?

Users shouldn't have to regress in production to debug issues. Having to include displayName for every build would definitely increase bundle sizes and slow things down.

aweary commented 6 years ago

Woops, clicked "Close and Comment" before I was done.

dumping the component’s render function source code might be helpful, even in its minified form?

I imagine this would be more confusing than helpful to most people. Compiled, minified source code would be very difficult to identify in most cases.

Since nestedUpdateCount seems to increment only when previousFlushedRoot === highestPriorityRoot, it would be great if we can know which component corresponds to this highestPriorityRoot.

The root doesn't correspond to a single component, but I think you're essentially suggesting the same thing I mentioned.

We could include the name of the component that was being worked on when the error was thrown

Which we can do, but again we run into the issue of the component name being minified.

About the component stack, in case the component is rendered to the DOM, having the DOM hierarchy (e.g. tag name with id, class, and data-testid from the root) would also be very useful for us to pinpoint the problem, since we also include useful debugging data in our app’s DOM.

You should be able to do this without React, right? If you're already using an error reporting service you should be able to just read from the DOM and include whatever debugging data you want. In this case React won't unmount the application, so all the markup should still be there.

dtinth commented 6 years ago

Users shouldn't have to regress in production to debug issues. Having to include displayName for every build would definitely increase bundle sizes and slow things down.

We’re making a bit of trade-off here: including the component’s display name probably adds around 40kb to our app size before gzip, but what we gain is the ability to introspect an app running in production, which I value more. 😃

To put our situation in context, some members of our team are not experienced with JS and React and sometimes code that e.g. calls setState in dangerous places gets slipped into our codebase. We admit that our tests are not perfect, so it’s likely that we get unanticipated front-end errors in production. This is what’s happening here — an unanticipated error on production that affects a small percentage of users, that we can’t reliably reproduce. To compensate for that, it’s better for us if, once that happens, we can track down which component caused it.

So far, the Maximum stack size exceeded (pre-React 16) and Maximum update depth exceeded (React 16+) are the hardest to track down because JavaScript stack trace doesn’t provide any information for us to pinpoint the problem.

I imagine this would be more confusing than helpful to most people. Compiled, minified source code would be very difficult to identify in most cases.

I agree, I was just illustrating an extreme case where we had to grep our bundle’s minified source to find the culprit.

You should be able to do this without React, right? If you're already using an error reporting service you should be able to just read from the DOM and include whatever debugging data you want.

No, unfortunately, without React telling which component, or which DOM element that corresponds to a component, is the culprit. For example, if our StrangeInput box is causing a nested update loop, if React could tell us that the error occurred on DOM element html > body > nav.NavBar > div.Search > input.StrangeInput, this would be very helpful.

kelseyyoung commented 6 years ago

We've just encountered this same problem as well with the "Maximum update depth exceeded" error. Our stack traces were all pointing to React code until, by some miracle, we got a single log that showed the component. Have there been any other conversations about this?

jwbay commented 5 years ago

Is the stack reliable for this error as far as the setState visible in the stack being the 50th nested update? I'm not sure where and how React takes over from the JS stack. Do the updates need to be synchronous to trigger this?

  at aa(./node_modules/fbjs/lib/invariant.js:42:1)
  at A(./node_modules/react-dom/cjs/react-dom.production.min.js:14:166)
  at pg(./node_modules/react-dom/cjs/react-dom.production.min.js:207:341)
  at enqueueSetState(./node_modules/react-dom/cjs/react-dom.production.min.js:148:225)
  at setState(./node_modules/react/cjs/react.production.min.js:12:344)
  at onClose(user code) <-- was this the 50th update? or the 1st? 🤔 
  at HTMLDocument.t.handleDocumentClick(user code)
  at HTMLDocument.d(/raven.3.26.4.js:2:4702)
choi2k commented 5 years ago

Users shouldn't have to regress in production to debug issues. Having to include displayName for every build would definitely increase bundle sizes and slow things down.

We’re making a bit of trade-off here: including the component’s display name probably adds around 40kb to our app size before gzip, but what we gain is the ability to introspect an app running in production, which I value more. 😃

To put our situation in context, some members of our team are not experienced with JS and React and sometimes code that e.g. calls setState in dangerous places gets slipped into our codebase. We admit that our tests are not perfect, so it’s likely that we get unanticipated front-end errors in production. This is what’s happening here — an unanticipated error on production that affects a small percentage of users, that we can’t reliably reproduce. To compensate for that, it’s better for us if, once that happens, we can track down which component caused it.

So far, the Maximum stack size exceeded (pre-React 16) and Maximum update depth exceeded (React 16+) are the hardest to track down because JavaScript stack trace doesn’t provide any information for us to pinpoint the problem.

I imagine this would be more confusing than helpful to most people. Compiled, minified source code would be very difficult to identify in most cases.

I agree, I was just illustrating an extreme case where we had to grep our bundle’s minified source to find the culprit.

You should be able to do this without React, right? If you're already using an error reporting service you should be able to just read from the DOM and include whatever debugging data you want.

No, unfortunately, without React telling which component, or which DOM element that corresponds to a component, is the culprit. For example, if our StrangeInput box is causing a nested update loop, if React could tell us that the error occurred on DOM element html > body > nav.NavBar > div.Search > input.StrangeInput, this would be very helpful.

We are facing the exactly the same issue... we use sentry as error reporting service and found it is REALLY REALLY REALLY hard to debug with just the stacktrace. No idea which component trigger the issue when we can't reproduce it :(

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

stale[bot] commented 4 years ago

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!

allancagadas commented 2 years ago

same here, hard to debug and replicate :c

cameronbraid commented 1 year ago

same here, and even though I can reproduce the issue interactively however I am still not able to identify which state mutation is the cause.

steodor commented 1 year ago

Why is this issue closed? We are still finding this error extremely hard to debug. Can anyone please reopen this or point to more recent developments, if any? Thanks!

ggcaponetto commented 1 year ago

This is still relevant IMO. Very difficult to debug without further information.

EgorBochkarev commented 11 months ago

I want to track this issue in my prod, but can't find out how to catch it. ErrorBoundary doesn't. Can you give any clue, thanks

pailhead commented 8 months ago

I have no idea how to isolate this on an inherited project :(

clarkmcc commented 5 months ago

No updates? I tried using React profiler but the profiler resets when this error is thrown.

shhivam commented 3 months ago

I have been facing this error for a while now. Scoured internet to see if there are any ways to debug it or know which component or line of code is causing this but no luck.

Long shot but have there been any updates?