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

selector error hiding in connect.js subscribeUpdates #1985

Closed
jrodriguesimg opened this issue Jan 30, 2023 · 13 comments
Closed

selector error hiding in connect.js subscribeUpdates #1985

jrodriguesimg opened this issue Jan 30, 2023 · 13 comments

Comments

@jrodriguesimg
Copy link

When a selector throws an error after the first time it is called, the error is being hidden by redux toolkit.
Example:

...
  const value = useSelector((() => {
    let firstTime = true;
    // the actual selector function
    return () => {
      if(firstTime) {
        console.log("first time")
        firstTime = false;
      } else {
        console.log("second time")
        throw "this error will be hidden by redux-toolkit"
      }
      console.log("end")
    }
  })
...

The expected console output would be:

first time
end
second time
Error: this error will be hidden by redux-toolkit

However the last line never shows. It will cause the component where the selector is used to not render as long as it errors.
I found out that the reason for this is that the error is caught and never thrown. I know it should be thrown in unsubscribeWrapper but it is not being called on my app.

// connect.js
...
    try {
      // Actually run the selector with the most recent store state and wrapper props
      // to determine what the child props should be
      newChildProps = childPropsSelector(latestStoreState, lastWrapperProps.current);
    } catch (e) {
      error = e; // <== where the error is caught and never thrown
      lastThrownError = e;
    }
...

It makes it hard to debug errors. Specially when the app has many selectors and because sometimes it is unnoticeable that a bug was introduced. Is there any reason to hide errors here?

@markerikson
Copy link
Contributor

markerikson commented Jan 30, 2023

This is a React-Redux question, not Redux Toolkit :) Moving this issue over there.

Also, I'm confused because you're showing a usage of useSelector, but the bit of internal code you're pointing to is from connect.

Can you attach a CodeSandbox showing an actual example of this happening?

@markerikson markerikson transferred this issue from reduxjs/redux-toolkit Jan 30, 2023
@jrodriguesimg jrodriguesimg reopened this Jan 30, 2023
@jrodriguesimg
Copy link
Author

You are right I'm going to close this issue and I'll create a new one in react-redux
The issue also happens with connect, but for useSelector the issue is in react-dom.develpment.js

function checkIfSnapshotChanged(inst) {
  var latestGetSnapshot = inst.getSnapshot;
  var prevValue = inst.value;

  try {
    var nextValue = latestGetSnapshot();
    return !objectIs(prevValue, nextValue);
  } catch (error) {
    return true;
  }
}

@markerikson
Copy link
Contributor

@jrodriguesimg : I did already move this issue - we're in the React-Redux repo now :)

@markerikson
Copy link
Contributor

And yes, our own useSelector implementation in React-Redux v7, and useSyncExternalStore in React-Redux v8 / React 18, intentionally swallow the error and force a re-render. If it ends up throwing an error during the re-render, you should see that because it will crash the React component tree. Otherwise, the selector worked fine during the re-render and rendering continued okay.

That's part of why I'm asking for an actual example of this failing.

@jrodriguesimg jrodriguesimg reopened this Jan 30, 2023
@jrodriguesimg
Copy link
Author

jrodriguesimg commented Jan 30, 2023

I used the counter example as a base: Code Sandbox. On this code sand box the components are all unmounted when an error is thrown, but on my app they are not being unmounted. What is causing the unmouting of the components in the Code Sandbox?

@markerikson
Copy link
Contributor

markerikson commented Jan 30, 2023

@jrodriguesimg that's why I'm asking for you to show me an example of where this isn't working the way you expect.

But also, the selector function you're passing in doesn't seem like it's going to actually do anything meaningful, because it's always going to create a new instance of that selector every time useSelector runs.

@jrodriguesimg
Copy link
Author

@markerikson thanks a lot for your help so far.

On my application the components do not unmount when the selectors throw an error. That means that the selector function is always the same. So when I add <TestComponent /> I see these logs:

TestComponent: first time
TestComponent: end
TestComponent: second time
TestComponent: second time
TestComponent: second time
...

Something on my app must be interacting with the mechanism used by react redux to unmount the components. My app is complex, so it is hard to know what exactly is stoping that process. Do you what is the mechanism used by react-redux to cause the unmounting? Is it the <Provider store={store}> that unmounts it's children? Or is the useSelector that triggers an unmounting of the component?

@markerikson
Copy link
Contributor

@jrodriguesimg : I really need you to provide an example of the code that you're trying to run. It doesn't have to be a complete app, just a minimal example of "I expected this to fail, and it doesn't", or something along those lines. But it needs to be realistic (and the way you were defining the selector with the closure for the sandbox earlier isn't realistic).

In general, React will unmount components if an error is thrown while rendering a component. If no error is thrown while rendering, then React just keeps rendering normally.

But in order to see what's happening in your app, I need an example that shows me what your code is actually doing so I can understand it.

@jrodriguesimg
Copy link
Author

jrodriguesimg commented Jan 31, 2023

@markerikson: It is hard for me to give you an example. It is a complex application. The best I can do is to make the exemple more realistic CodSanbox. The only way for me to be able to reproduce my issue in code sandbox would be for me to know what is causing the issue.

My issue is that the error is being caught by react-redux, but for some reason my app does not error.
You said:

And yes, our own useSelector implementation in React-Redux v7, and useSyncExternalStore in React-Redux v8 / React 18, intentionally swallow the error and force a re-render. If it ends up throwing an error during the re-render, you should see that because it will crash the React component tree.

But the re-render does not happen on my app. Can you provide more details of how that re-render is triggered?

I think in here you are talking about the re-render:

In general, React will unmount components if an error is thrown while rendering a component.

Probably my app is doing something unexpected, and stopping that re-render from happening.

Edit:

How is react-redux provoking the re-render from the call to useSelector? Is it some interaction with the that is mounted as parent of the application? Is the error caught by useSelector thrown somewhere else? Did I miss the documentation where that process is explained?

@markerikson
Copy link
Contributor

markerikson commented Jan 31, 2023

Now I'm really confused.

If I open up that sandbox, the React component tree does indeed crash because the selector is unconditionally throwing an error.

This is exactly the behavior I would expect, and matches what I said above: if an error is thrown while rendering, React unmounts the component tree.

So at this point I don't know what you're trying to show me, because it sounds like you're trying to describe a case where the error is throwing and React/React-Redux do not crash.

Note that React-Redux does have different behavior between useSelector and connect, and you mentioned using connect in the issue title.

@jrodriguesimg
Copy link
Author

The sand box does not replicate my issue.
It is hard for me to replicate the issue on a sandbox without knowing how the useSelector triggers the re-render after an error.

@phryneas
Copy link
Member

phryneas commented Feb 1, 2023

useSelector does nothing. It calls useSyncExternalStore and that's it. React's useSyncExternalStore triggers the rerender.

The only thing I could think about here is that you see a discrepancy between the shim that React 16.8 and 17 ship with and the final uSES implementation of React 18.

@markerikson
Copy link
Contributor

markerikson commented Feb 1, 2023

@jrodriguesimg : In general, both useSelector and connect should catch errors thrown by selectors during dispatch, and force a re-render.

If the error occurs again while rendering, that will force React to crash and unmount the UI. If the selector succeeds while rendering, then everything's fine.

We've asked several times for examples that actually show us something similar to what you're seeing. We're not asking you to solve the issue, but to show us code that "fails" (doesn't behave the way you want/expect) so that we can explain why it does that.

I'm going to close this due to lack of reproduction. As far as I can see everything's behaving as expected.

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

No branches or pull requests

3 participants