Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

All Errors Catched #57

Closed
timsuchanek opened this issue May 20, 2016 · 23 comments · Fixed by #89
Closed

All Errors Catched #57

timsuchanek opened this issue May 20, 2016 · 23 comments · Fixed by #89
Assignees
Labels
Milestone

Comments

@timsuchanek
Copy link

When using the connect method from react-apollo, all errors that occur in one of the child components, are catched by apollo and fail silently.

Development gets nearly impossible then.

Any ideas why that could be the case?
Using apollo-client 0.3.5 and react-apollo 0.3.3

Thanks!

@jbaxleyiii
Copy link
Contributor

@johnthepink didn't you run into this case as well?

@jbaxleyiii
Copy link
Contributor

@timsuchanek do you have some code I can use to replicate and write a test? Where is the child component failing and what is react-apollo doing?

@jbaxleyiii jbaxleyiii added the bug label May 20, 2016
@jbaxleyiii jbaxleyiii self-assigned this May 20, 2016
@jbaxleyiii
Copy link
Contributor

As soon as I can replicate I'll fix ASAP!

@jbaxleyiii
Copy link
Contributor

For instance:

  it('doesn\'t catch all errors of children ', () => {
    const store = createStore(() => ({ }));
    const query = gql`
      query people {
        allPeople(first: 1) {
          people {
            name
          }
        }
      }
    `;

    const data = {
      allPeople: {
        people: [
          {
            name: 'Luke Skywalker',
          },
        ],
      },
    };

    const networkInterface = mockNetworkInterface({
      request: { query },
      result: { data },
    });

    const client = new ApolloClient({
      networkInterface,
    });

    const mapQueriesToProps = () => ({
      errorTest: {
        query,
      },
    });

    let fail: any;

    class ErrorChild2 extends React.Component<any, any> {
      render(){
        return (
          <div>
            {(() => {
              return fail.test.foobar;
            })()}
            <Passthrough {...this.props} />;
          </div>
        )
      }
    }

    class ErrorChild1 extends React.Component<any, any> {
      render(){
        return fail.test.foobar;;
      }
    }

    @connect({ mapQueriesToProps })
    class Container1 extends React.Component<any, any> {
      render() {
        return <ErrorChild1 {...this.props} />;
      }
    };

    @connect({ mapQueriesToProps })
    class Container2 extends React.Component<any, any> {
      render() {
        return <ErrorChild2 {...this.props} />;
      }
    };

    const mountMethod1 = () => {
      return mount(
        <ProviderMock store={store} client={client}>
          <Container1 pass='through' baz={50} />
        </ProviderMock>
      );
    }

    const mountMethod2 = () => {
      return mount(
        <ProviderMock store={store} client={client}>
          <Container2 pass='through' baz={50} />
        </ProviderMock>
      );
    }

    expect(mountMethod1).to.throw;
    expect(mountMethod2).to.throw;

  });

Correctly passes? So two different error types both throw correctly

@timsuchanek
Copy link
Author

Hmm of course when I try to create a minimal example, also all errors throw correctly.

So I am using this boilerplate: https://github.com/graphcool-examples/react-apollo-todo-example

Probably some other side effects are involved in interfering the proper error propagation.

My conclusion was that the problem should be react-apollo when I substituted the connect method with the one from react-redux. Then all errors threw again correctly.

As soon as I have a reproducible example, I'll post it here

@johnthepink
Copy link
Contributor

@timsuchanek I have been running in to this issue as well. I'll do some digging.

@johnthepink
Copy link
Contributor

@timsuchanek would you be willing to share the class that isn't throwing errors in your app?

@johnthepink
Copy link
Contributor

@timsuchanek it looks like our issue didn't have to do with react-apollo, but with react's error reporting directly. In certain cases, React swallows exceptions and doesn't report them.

Here is the open issue on the react repo facebook/react#2461

For our project, I added react-transform-catch-errors, and altered our babel settings, and now we have the errors that you would expect on components using connect and the children of those components.

I know you mentioned you were using that boilerplate. It doesn't look like it includes a way to deal with catching the errors in react lifecycle functions. So maybe that will help?

@timsuchanek
Copy link
Author

Hey guys, thanks for the input so far!
I still can't figure out a reproducible example, but this is my quick fix, using it in the render() method of my connect'ed component:

    if (this.props.data.errors) {
      console.error(this.props.data.errors.stack);
    }

@stubailo
Copy link
Contributor

@jlevycpa
Copy link

Here is a repro that should help. See the Readme for details. I've never put something like this together before so any feedback is appreciated.

https://github.com/jlevycpa/apollo-error-repro

@jlevycpa
Copy link

I updated to the latest version of react-apollo but I'm still seeing this issue. When I update the reproduction I posted above, the behavior changes slightly. The behavior of an error in the Item component is now the same as the behavior in the Container component - an error in the apollo store and no error in the console. The cryptic console error Cannot read property 'replaceChild' of null seems to be gone.

@jbaxleyiii did you try running the reproduction with the update? Did you have different results? Maybe I didn't update correctly or something?

@jbaxleyiii
Copy link
Contributor

still not fixed

@jbaxleyiii jbaxleyiii reopened this Jul 6, 2016
@jlevycpa
Copy link

jlevycpa commented Jul 7, 2016

Is it possible that this line is the problem? It looks like the rendered child is getting stored instead of being returned directly. I'm wondering if that is breaking the call stack instead of letting the error bubble all the way up through the return calls in the render methods. If I have time I will test this theory.

It looks like the reason for storing the rendered child is to prevent re-renders. But isn't that what ShouldComponentUpdate is for?

@saikat
Copy link

saikat commented Jul 14, 2016

@jbaxleyiii not sure if you have already started on this bug, but I believe the test here has a logic bug in it: https://github.com/apollostack/react-apollo/blob/master/test/client/connect/queries.tsx#L1965

count gets incremented to 1 the first time render gets hit with data.loading set to true
count gets incremented to 2 the second time render gets hit with data.loading set to false. It then hits that line in the code and the test passes, never actually hitting the key line in https://github.com/apollostack/react-apollo/blob/master/test/client/connect/queries.tsx#L1970.

Fixing that alone doesn't make the test a correct reproducible test, but it's a start.

@skuridin
Copy link

skuridin commented Nov 15, 2016

I still have this bug in latest version of Apollo. Using it together with redux.

networkError: "Error: Attempted to update component `SourceItem` that has already been unmounted (or failed to mount)"

It's not a network error.

@skuridin
Copy link

skuridin commented Nov 16, 2016

Update: It happens only if I use Apollo's reducer and middleware with redux. Without connecting it to my redux store I can see errors, but they are all caught in apollo-client/store.js.

Store.js:13 Caught an exception! ReferenceError: test is not defined(…)

@jbaxleyiii Why this issue is closed?

@jbaxleyiii
Copy link
Contributor

@skuridin looks like this was a bug in apollo client that was reintroduced. I believe a fix is already in place for it in the next release. react-apollo as the issue point was refactored and fixed to correctly error. Including tests added to prevent this regression

@helfer
Copy link
Contributor

helfer commented Nov 16, 2016

@jbaxleyiii does this still happen with 0.5.4? I'm not aware of any bug having been reintroduced.

@DimitryDushkin
Copy link

DimitryDushkin commented Dec 28, 2016

Is there any official docs about error handling in mutations?

I've found one way — mutate(..).catch(err => alert(err));

@davidgljay
Copy link

I am on 1.0.0-rc.5 of both react-apollo and apollo-client and am still seeing this issue :(

Any render error causes apollo's query data to not appear, the error itself is swallowed.

@webmobiles
Copy link

please help me; where use throw in this code:

const SignInWithData = graphql(signInMutation)(withRouter(SignInFormContainer));


const mapDispatchToProps = dispatch => ({
  signInDispatcher(token) {
    dispatch(signIn(token));
  },
});

const SignInWithDataAndState = connect(
  null,
  mapDispatchToProps,
)(SignInWithData);

export default SignInWithDataAndState;

@mbarmettler
Copy link

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

Successfully merging a pull request may close this issue.