errorPolicy all behaves the same as ignore for server side rendering

[ganemone]

I believe there is a bug with error handling in react-apollo when using server side rendering which causes the error field in a query result to be undefined, even when the errorPolicy is set to all.

Intended outcome:

When a <Query> component is server side renderered with errorPolicy=all, and an error thrown in a resolver, the render props function should be passed a result with an error field being populated as documented here.

Actual outcome:

The error field is undefined on the server.

How to reproduce the issue:

I have created a reproduction example here: https://codesandbox.io/s/3xx4w96o6

I also have a version on github here: https://github.com/ganemone/apollo-ssr-error-handling

Version

• apollo-client@latest
• react-apollo@latest

I believe this is a similar issue to #2655 and #615 however there was some issues with people being rude in those threads so I am creating a new issue here with more clear reproduction steps.

I think this should be somewhat high priority since there is not a viable errorPolicy for gracefully handling errors in server side rendering right now. none will cause renderToStringWithData to throw, and all behaves the same as ignore, so you are forced to either render a full error page or ignore errors completely.

[robinweser]

Just took me an hour to find there's probably a bug here. @ganemone Any news on this from your side? Would love to get it fixed as soon as possible - might dig into the code later.

For now, what helped is downgrading to 2.1.0 (maybe even higher, 2.1.0 is just fine for me)
Edit: Seems that up to 2.3.0 it works just fine.

[ganemone]

@rofrischmann I can still reproduce this bug with all versions of react-apollo including 2.1.0 and 2.3.0. Do you have a reproduction case of the errorPolicy: 'all' working with any previous version of react-apollo?

[KevinGrandon]

@hwillson @benjamn @jbaxleyiii - Hello Apollo friends. Seems this issue has bitten a few people and I'm hoping that the reproduction cases in the original report are useful. Any chance we could get some eyes on this issue or guidance for fixing this ourselves? Thanks!

[ganemone]

I've done some investigation and I think I know why this is happening. The renderToStringWithData function does two render passes. First pass to fetch data via executing all queries in the render tree, and the second pass to actually render the app via react renderToString. The second pass can use the data fetched in the first pass because it is hydrated with the same instance of the apollo cache.

The problem arises from the fact that the error is not stored in the apollo cache, at least not in the same way results from a query are stored. If a query fails on the first render, executing that same query on the second render pass does not return the error from the first pass. You can verify this behavior directly in apollo-client.

const obs = client.watchQuery({
  query, // some query that will fail
  errorPolicy: 'all'
});
obs.subscribe({
  error: e => {
    let nextObs = client.watchQuery({
      query, 
      fetchPolicy: 'cache-only'
    });
    console.log(result.currentResult()); // no error is returned
  },
});

It seems the issue is that we are forced to create multiple instances of the query observable due to the two pass nature of renderToStringWithData.

I would love to help contribute back and fix this issue, but it seems like it would require some significant changes either to the apollo-client behavior, the renderToStringWithData function, or something else.

I also imagine this will be easier to solve when react has server side render support for suspense. Thoughts?

[mikaelgramont]

I can also vouch that downgrading to 2.1.0 does not solve the issue, and completely agree with this:

I think this should be somewhat high priority since there is not a viable errorPolicy for gracefully handling errors in server side rendering right now. none will cause renderToStringWithData to throw, and all behaves the same as ignore, so you are forced to either render a full error page or ignore errors completely.

In the meantime, I'd love to hear suggestions for a workaround (even an ugly, temporary one)!

[mikebm]

I have a slightly better understanding on why this isn't working, but am still digging to figure out more.

The issue is the queryId is different on the ObservableQuery. When the promise is resolved, it tries to get the errors from the QueryManager's queryStore via:
const queryStoreValue = this.queryManager.queryStore.get(this.queryId);
This queryId is different than the one used to initially retrieve the data and send it to the props of the Query Component, so it always gets back no errors.

It looks like this is because the Query Component itself is a new instance. I wonder if it would be better to use the cacheKey the InMemoryCache utilizes for the queryId value rather than a newly generated number. Of course, my knowledge on the internals is very limited.

Update: I have fixed this locally and will be submitting a PR tomorrow. I just need to do a little more testing. Hopefully it is accepted.