Skip to content
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

Error: "Commit tree does not contain fiber "5766". This is a bug in React DevTools." #20431

Closed
dixitk13 opened this issue Dec 11, 2020 · 15 comments · Fixed by #21377
Closed

Error: "Commit tree does not contain fiber "5766". This is a bug in React DevTools." #20431

dixitk13 opened this issue Dec 11, 2020 · 15 comments · Fixed by #21377

Comments

@dixitk13
Copy link

Describe what you were doing when the bug occurred:

  1. Profiling recharts library
  2. Filtered on commits greater than 3s
  3. Walking through the commits doing a next > next

Please do not remove the text below this line

DevTools version: 4.10.1-f160547f47

Call stack: at updateTree (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:19729:21)
at getCommitTree (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:19594:26)
at ProfilingCache_ProfilingCache.getCommitTree (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:20115:11)
at CommitFlamegraphAutoSizer (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:34161:33)
at Hh (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:12807:7)
at qi (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:13457:7)
at mk (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:16074:86)
at lk (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:15459:11)
at kk (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:15451:23)
at ck (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:15435:5)

Component stack: at CommitFlamegraphAutoSizer (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:34144:50)
at div
at div
at div
at SettingsModalContextController (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:28206:3)
at Profiler_Profiler (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:35772:50)
at ErrorBoundary_ErrorBoundary (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:29219:5)
at PortaledContent (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:29336:32)
at div
at div
at ProfilerContextController (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:32934:3)
at TreeContextController (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:24322:3)
at SettingsContextController (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:24811:3)
at ModalDialogContextController (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:29404:3)
at DevTools_DevTools (chrome-extension://fmkadmapgofadopljbjfkapdkoienihi/build/main.js:36207:3)

@bvaughn
Copy link
Contributor

bvaughn commented Dec 11, 2020

Can you reliably reproduce this? If so, can you please share a repro case with us? (The bug is unlikely to be fixed without out.)

@dixitk13
Copy link
Author

Should I be providing the profiler json? Because using a recording profiler json or the use case, I mentioned above, it reproduces always.

Or are you looking for a repository where this can be reproduced?

@bvaughn
Copy link
Contributor

bvaughn commented Dec 11, 2020

Ah, sorry my original request was unclear.

No, profiler JSON would likely just show that an element is indeed missing. A repro case (where I can run the code, see where/how/why DevTools misses the element) is what I need to fix.

@dixitk13
Copy link
Author

Argh. So this is going to sound weird.

I have a project where I can reproduce this, and I can do that always (see the date @ screenshot) but I can't ship it out due to a lot of internal tooling.
So, I tried a similar use-case in my personal-{laptop, project} and I cannot it reproduce in that project.
I'll try some more and see if I can reproduce it in my personal project.

The image below is from the project which I can't ship 😞
image

@bvaughn
Copy link
Contributor

bvaughn commented Dec 12, 2020

@dixitk13 I would be willing to sign an NDA or similar agreement if that would help. Otherwise, it would be super helpful if you were able to reduce it to something that you could ship. (Maybe you could start with your full app and remove things until you get to the smallest case that repros the bug? Then you could share just that?)

@bvaughn
Copy link
Contributor

bvaughn commented Dec 21, 2020

Friendly ping. Any update or chance of one?

@dixitk13
Copy link
Author

Hey @bvaughn, try the below:

Use this repo react-profiler-sample. The movie attached should help w/ repro steps. I also left some silly comments in App.tsx - if you wanna try those out as well (somehow I feel they contribute to this trace - I could be wrong there).

I haven't experimented much w/ the different yarn, node and npm versions, but below the ones which kinda did it.

➜  react-profiler-sample git:(master) yarn -v
1.19.1
➜  react-profiler-sample git:(master) node -v
v10.17.0
➜  react-profiler-sample git:(master) npm -v
6.11.3
react-profiler-error-720-2.mov

@bvaughn
Copy link
Contributor

bvaughn commented Dec 22, 2020

Thank you for the repro!

Holiday time so I might not take a look right away, but I'll try to look soonish.

@IDrissAitHafid
Copy link
Contributor

IDrissAitHafid commented Dec 23, 2020

I knew there will be so many edge cases for these unfound fiber errors 😢

Anyway, based on @dixitk13 repro, the problem (for this case) is that there's a Maximum call stack size exceeded error thrown I think by the recursive function mountFiberRecursively due to the big number of elements that it needs to mount.

I reproduced it in this sandbox app.

Here's the steps to reproduce it:

  1. Start profiling
  2. Click Show button
  3. Click Hide button
  4. Stop profiling

@bvaughn
Copy link
Contributor

bvaughn commented Jan 4, 2021

Anyway, based on @dixitk13 repro, the problem (for this case) is that there's a Maximum call stack size exceeded error thrown I think by the recursive function mountFiberRecursively due to the big number of elements that it needs to mount.

That's good insight. Can confirm this on my side as well, (though only if I disable the host component filter).

My intuition when writing DevTools was that it would be very uncommon for apps to have React trees deep or wide enough that the recursive mountFiberRecursively function (or similar functions, e.g. #19839) would run out of stack. I think that's true, but given how large the user base is I guess it makes sense that it happens sometimes.

I took a stab at rewriting the recursive methods in the backend/renderer in #16627 but didn't get them all (mountFiberRecursively, updateFiberRecursively, unmountFiberChildrenRecursively, findReorderedChildrenRecursively) because it would have been a big change and would make the code harder to read.

Maybe worth picking back up and finishing that effort. Anyone interested? The good news is, DevTools has decent unit test coverage.

@hectorcoronado
Copy link

hectorcoronado commented Mar 11, 2021

Anyway, based on @dixitk13 repro, the problem (for this case) is that there's a Maximum call stack size exceeded error thrown I think by the recursive function mountFiberRecursively due to the big number of elements that it needs to mount.

That's good insight. Can confirm this on my side as well, (though only if I disable the host component filter).

My intuition when writing DevTools was that it would be very uncommon for apps to have React trees deep or wide enough that the recursive mountFiberRecursively function (or similar functions, e.g. #19839) would run out of stack. I think that's true, but given how large the user base is I guess it makes sense that it happens sometimes.

I took a stab at rewriting the recursive methods in the backend/renderer in #16627 but didn't get them all (mountFiberRecursively, updateFiberRecursively, unmountFiberChildrenRecursively, findReorderedChildrenRecursively) because it would have been a big change and would make the code harder to read.

Maybe worth picking back up and finishing that effort. Anyone interested? The good news is, DevTools has decent unit test coverage.

Hi @bvaughn, I may not be able to start till next week, but I'd like to give it a shot if you're cool with it

@bvaughn
Copy link
Contributor

bvaughn commented Mar 11, 2021

Sounds great @hectorcoronado. Keep me posted.

@liao02x
Copy link

liao02x commented Apr 22, 2021

Friendly ping on this. Had the same issue multiple times and fallback to console log debugging

@bvaughn
Copy link
Contributor

bvaughn commented Apr 28, 2021

Circling back around to this.

Using the excellent repro steps (#20431 (comment)) shared by @dixitk13, I can repro this faster if I change the default items (in initState) from 100 to 5000.

Then just loading the app, I notice the following error (in the console):

RangeError: Maximum call stack size exceeded
at shouldFilterFiber (react_devtools_backend.js:5480)
at mountFiberRecursively (react_devtools_backend.js:6286)
...

If I ignore this error and profile anyway:

  1. Profile
  2. Change the number of list items from 5,000 -> 0
  3. Notice several errors (in the DevTools extension console, likely caused by the stack overflow above):

Error: Cannot remove node "4631" because no matching node was found in the Store.
Error: Cannot add child "4637" to parent "4635" because parent node was not found in the Store.
Error: Cannot add child "4650" to parent "4638" because parent node was not found in the Store.
Error: Cannot remove node "4650" because no matching node was found in the Store.

  1. Stop profiling
  2. See an error now in DevTools UI (related to the previous errors):

Error: Commit tree does not contain fiber "4631". This is a bug in React DevTools.

So the underlying cause here, as pointed out by @IDrissAitHafid, is the original stack overflow error.

@bvaughn
Copy link
Contributor

bvaughn commented Apr 28, 2021

Converting all of the recursive methods in DevTools to be iterative would be a bit lift but I think I can fix this particular case by iterating over siblings rather than recursing. I'll post a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants