fix: When doing a SSR with errorPolicy='all', then error should not be lost

[mikebm]

Closes #2680
Closes #2134
Closes #1972
Closes #1781
Closes #615

This change tracks the original observable used during a SSR. When the Query component is recreated due to a Query's promise resolving, then the original queryId used to track the response error is lost. By tracking the original observable, the newly instantiated Query component can reference the original observable to retrieve the actual result. This makes the feature work like the documentation states, which allows server side rendered components to properly handle errors rather than throwing exceptions.

[apollo-cla]

@mikebm: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[klarstrup]

Lots of noise in the diff from unrelated formatting changes :/

[mikebm]

Lots of noise in the diff from unrelated formatting changes :/

This was related to the code not aligning to the prettier config in package.json since I use the prettier plugin. I reverted said noise.

[benjamn]

Thanks so much @mikebm!

Rebased/squashed a bit and added an additional commit that simplifies the RenderPromises implementation (our fault not yours).

[mikebm]

Thanks so much @mikebm!

Rebased/squashed a bit and added an additional commit that simplifies the RenderPromises implementation (our fault not yours).

Perfect. I was thinking along the same lines but shied away from my first PR here to be too invasive :). Thank you much for quickly accepting this.

[mikaelgramont]

Many thanks for this fix, folks!

[Dattaya]

The error is present during the getDataFromTree(App) stage, but not during ReactDOM.renderToString(App). Is it on purpose? Because that's when I want to have an error, so my component can handle it: if (error) return <ErrorPage />, and the server would return an error page just like the client does when it happens after the navigation to a page with non-existent data.

[mikebm]

The error is present during the getDataFromTree(App) stage, but not during ReactDOM.renderToString(App). Is it on purpose? Because that's when I want to have an error, so my component can handle it: if (error) return <ErrorPage />, and the server would return an error page just like the client does when it happens after the navigation to a page with non-existent data.

getDataFromTree is already rendering your app to a string. It didn't use to do this in prior versions. It now returns a string which contains your application rendered via renderToStaticMarkup. In your case, rendering it again won't contain the renderPromises Provider which means it will only render it with what is in the Apollo cache, which will not contain your error. You should probably just be using the string result that is emitted from getDataFromTree/getMarkupFromTree. Any reason you aren't and are rendering it a second time?

[Dattaya]

Thank you for the explanation, I had no idea. So, does that mean that renderToStringWithData is deprecated, it used to return rendered app according to the docs.
I must be making another mistake, because apolloClient.extract() does not contain an error, and the client side script overrides the correct html from the server.

[mikebm]

Thank you for the explanation, I had no idea. So, does that mean that renderToStringWithData is deprecated, it used to return rendered app according to the docs.
I must be making another mistake, because apolloClient.extract() does not contain an error, and the client side script overrides the correct html from the server.

I still use it, it simply just calls getMarkupFromTree. extract won't cache the error for re-hydration because the InMemoryCache won't contain errors, just the cached result. You should be handling the errors on the server in this case. What I do is render a 404 or error component that sets the status code that is handled on the server.
Example:

export const Status: FunctionComponent<Props> = ({ code, children }) => (
    <Route
        render={({ staticContext }) => {
            if (staticContext) {
                staticContext.statusCode = code;
            }
            return children;
        }}
    />
);

On the server:

const context = {};
<StaticRouter context={context} location={req.path}>
	<AppComponent />
</StaticRouter>

Then on the server, you just check context.statusCode and set the http response code accordingly.

res.status(context.statusCode || 200).send(html);

[jdmoody]

@mikebm, why doesn't the InMemoryCache contain errors for re-hydration?

I'm running into the exact problem @Dattaya mentioned above. I have an Error component that renders specific information about the GraphQL error. The initial SSR shows the error info as expected, but upon client-side hydration, the error information is removed since extract doesn't cache errors.

I'm not sure I understand your solution. In the error case, does the server render static HTML that isn't re-hydrated by the client?

Thanks for all your help in this thread 🙏

[mikebm]

@mikebm, why doesn't the InMemoryCache contain errors for re-hydration?

I'm running into the exact problem @Dattaya mentioned above. I have an Error component that renders specific information about the GraphQL error. The initial SSR shows the error info as expected, but upon client-side hydration, the error information is removed since extract doesn't cache errors.

I'm not sure I understand your solution. In the error case, does the server render static HTML that isn't re-hydrated by the client?

Thanks for all your help in this thread 🙏

This is just how the Apollo team designed the InMemoryCache in ApolloClient. It only contains cache keys and cache data. In my specific use case, I handle the error during the SSR by using react-router's <Redirect /> component to either a 404, or 500 page depending on the error. On the client side, the apollo client will load whatever data it can from cache depending on your errorPolicy and fetchPolicy.

I use errorPolicy=all, which allows for partial data to be returned during an error, otherwise you get an exception during the SSR on the server.
Now this is where it gets interesting. If your schema allows for your resolver to return null, then it could cause null to be cached in the InMemoryCache, which would mean (depending on your fetchPolicy) that null will be returned on the client. If your schema resolver does not allow null, then nothing would be cached, and your client would re-issue the request and you would get the original error (assuming it still occurs). Ideally, you would have handled this error in the first place on the server though.

Your use case may be different, but I hope this helps.

[Dattaya]

Redirecting is the most elegant solution. So far I save error in window.__ERROR_TYPE__ and check if it's present on the client, it's quite ugly, but it works. Also, I had to create my own getDataFromTree as recommended in the react-apollo source to use renderToString instead of renderToStaticMarkup:

import { renderToString } from 'react-dom/server'
import { getMarkupFromTree } from 'react-apollo'
const getDataFromTree = (tree, context = {}) => getMarkupFromTree({ tree, context, renderFunction: renderToString })

[maxchehab]

@mikebm Are there alternatives to InMemoryCache that cache errors?

[mikebm]

@mikebm Are there alternatives to InMemoryCache that cache errors?

None that I know of.