React: Bug: devtools Profiler causes unexpected errors

[henryqdineen]

We noticed that our app would behave differently during profiling runs and trigger errors. I'm not totally sure what the underlying issue is but I was able to put together a example app to reproduce. As far as I can tell it has to do with how devtools is overriding console.warn and console.error. In that case describeNativeComponentFrame() will call a function component with no args. This works fine as the error is caught in describeNativeComponentFrame() but in it looks like a useEffect() that accesses those props is still triggered and it does not expect props to be undefined.

I realize that having props in the dependencies array of the useEffect doesn't really make sense but I still think it probably shouldn't error.

React version: 16.13.1
React devtools version: 4.8.2

Steps To Reproduce

1. Open link to code example below
2. Click "Open In New Window" from the "Browser" tab
3. Observe a simple app with only <h1>Hello World</h1>
4. Open the React devtools Profiler tab
5. Click "Reload and start profiling"
6. Observe an error Uncaught TypeError: Cannot read property 'foo' of undefined

Link to code example:

https://codesandbox.io/s/cool-sun-wdrry

The current behavior

The expected behavior

The app should work as it while not profile. It should render a <h1>Hello World</h1>

[bvaughn]

Thanks for the report. I'll look at it sometime soon. :)

[ashr81]

@bvaughn Shall I work on a pull request for this issue?

[bvaughn]

@ashr81 If you'd like!

[ashr81]

@bvaughn Yes, I like to contribute.

[bvaughn]

I'm going to look into this this afternoon! :)

[bvaughn]

I'm not sure I understand yet what Profiling or DevTools has to do with this bug. I've exported the attached Code Sandbox and I can't repro this bug with DevTools or while profiling.

Edit I can reproduce this on Code Sandbox itself, just not locally. It's possible that this Code Sandbox is pinned to using react-devtools-inline v4.4.0 from January, so it's missing several bug fixes since then. I'm not sure which one would have fixed this though.

Edit 2 I have a bit more context now. Still looking though.

[ashr81]

You should see the bug when you open codesandbox in seperate tab in browser and start profiling.props are not passing into useEffect while profiling on only codesandbox. I tried it outside codesandbox, It is working fine.

[ashr81]

Profiling using same react-dev-tools works codesandbox fine with the latest React 17.0.0-rc.3

[bvaughn]

I see the problem here.

Logging a warning during render causes DevTools (if configured to "append component stacks" to warnings) to do its own shallow render of a component. The problem is that the useEffect from this shallow render is getting scheduled to be called later when we flush passive effects. This is not supposed to happen.

DevTools overrides the dispatcher before shallow rendering to prevent this from happening:
https://github.com/facebook/react/blob/e614e6965749c096c9db0e6ad2844a2803ebdcb6/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js#L88-L95

That being said, looking in the Chrome debugger– the source I'm seeing does not do that:

function describeNativeComponentFrame(fn, construct, currentDispatcherRef) {
  // If something asked for a stack inside a fake render, it should get ignored.
  if (!fn || reentry) {
    return '';
  }

  if (false) {}

  let control;
  const previousPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe It does accept undefined.

  Error.prepareStackTrace = undefined;
  reentry = true;
  let previousDispatcher;

  // ...

This reason for this is that I'm kind of clowny and didn't take into account the fact that the DevTools extension itself would be running in production mode, even if the application code (that logs warnings) is in DEV mode.

[bvaughn]

Feel like testing this (local) build of DevTools and seeing if it fixes the problem for you?
ReactDevTools.zip

[ashr81]

It's working fine now. tested it using the generated link on pull request you raised.

[bvaughn]

Thank you for confirming, and for the repro case. I'll try go get an update (with the fix) pushed to the browser stores shortly.