Errors during SSR are rendered as "loading"

[kachkaev]

I'm writing a Next.js app that's based on their with-apollo example and would like to share a confusion with the server-side error handling. I believe it might be either a bug in apollo client or something we can consider as a feature request.

In the example, the logic of a HOC that deals with Apollo data is located in with-apollo-client.js and mine is not very different. When the code runs on the server, getDataFromTree() is called and then the React tree renders using the data from apollo.cache.extract(). This works fine if all requests have succeeded – the server sends a fully rendered tree with all the data, that's cool.

When a request fails during getDataFromTree(), the errors are suppressed with catch. This is generally fine, because we do not want a small faulty widget in a side panel to crash the whole page. However, because apollo.cache.extract() does not contain any information about the failed queries, failed query components render as if the data is still being loaded.

This peculiarity is a bit hard to spot in Next.js, because client-side rendering follows the server-side one and loading... gets replaced with a real error quickly (no data in cache → new query → render loading → register error → render error). However, this kind of "fix" makes the situation even harder to realise rather than resolved.

Imagine I have a page with a blog post, which is identified by a slug in the current URL. Unlike for a small side-widget I mentioned above, the success of a query that gets me the post is critical to the page. In this case, a failure means it's either 404 or 500:

import { Query } from "react-apollo";

export default ({ slug }) => (
  <Query query={BLOG_POST_QUERY} variables={{ slug }}>
    {({ data, loading, error }) => {
      if (loading) {
        return <span>loading...</span>;
      }

      if (error) {
        // fail the whole page (500)
        throw error;
      }

      const blogPost = data && data.blogPost;
      if (!blogPost) {
        // fail the whole page (404 - blog post not found)
        const e = new Error("Blog post not found");
        e.code = "ENOENT";
        throw e;
      }
      return <BlogPostRepresentation blogPost={blogPost} />
    }}
  </Query>
);

The errors that are thrown here get handled by Next's _error.js, which can render 404 - Post not found or 500 - App error, please reload using some custom logic. In neither case I want the server to return 200 even though the error will pop out on the client side shortly – search engines won't like this. The code above renders a proper 404 page when data.blogPost is null, but if a graphql server goes down for some time, all my blog posts - existing or non-existing - will return 200 and this can quickly ruin google search results for my website.

How to reproduce the issue:

1. Install with-apollo example

   npx create-next-app --example with-apollo with-apollo-app

2. Add this line to components/PostList.js:

    function PostList ({
      data: { loading, error, allPosts, _allPostsMeta },
      loadMorePosts
    }) {
   +  console.log('RENDERING POST LIST', { loading, error: !!error, allPosts: !!allPosts });
      if (error) return <ErrorMessage message='Error loading posts.' />

3. Launch it using yarn dev and open localhost:3000 in a browser. You will see:

   # server-side
   RENDERING POST LIST { loading: false, error: false, allPosts: true }
   RENDERING POST LIST { loading: false, error: false, allPosts: true }
   
   # client-side (uses cache, no loading)
   RENDERING POST LIST { loading: false, error: false, allPosts: true }
   RENDERING POST LIST { loading: false, error: false, allPosts: true }

   This looks fine.

4. Simulate a graphql server failure (e.g. open lib/init-apollo.js and replace api.graph.cool with unavailable-host).

5. Reload the page. You will still see Error loading posts., however your logs will say

   # server-side
   RENDERING POST LIST { loading: true, error: false, allPosts: false }
   
   # client-side (no cache found, loading from scratch and failing)
   RENDERING POST LIST {loading: true, error: false, allPosts: false}
   RENDERING POST LIST {loading: false, error: true, allPosts: false}

   (instead of loading: false, error: true on the server)

6. Turn off javascript in the browser via dev tools and reload the page.
   Expected visible content: Error loading posts.
   Actual visible content: Loading

Versions

  System:
    OS: macOS High Sierra 10.13.6
  Binaries:
    Node: 10.9.0 - /usr/local/bin/node
    Yarn: 1.9.4 - /usr/local/bin/yarn
    npm: 6.2.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 69.0.3497.81
    Firefox: 62.0
    Safari: 11.1.2
  npmPackages:
    apollo-boost: ^0.1.3 => 0.1.15 
    react-apollo: 2.1.0 => 2.1.0 
  npmGlobalPackages:
    apollo: 1.6.0

---

Meanwhile, my current workaround for critical server-side queries that are not allowed to fail will be something like this:

+ const isServer = typeof window === 'undefined';

  export default ({ slug }) => (
  <Query query={BLOG_POST_QUERY} variables={{ slug }}>
      {({ data, loading, error }) => {
+     // fail the whole page (500)
+     if ((isServer && loading) || error) {
+         throw error || new Error("500: Critical query failed");
+     }

      if (loading) {
          return <span>loading...</span>;
      }

-     if (error) {
-         // fail the whole page (500)
-         throw error;
-     }

      const blogPost = data && data.blogPost;
      if (!blogPost) {
          // fail the whole page (404 - blog post not found)
          const e = new Error("Blog post not found");
          e.code = "ENOENT";
          throw e;
      }
      return <BlogPostRepresentation blogPost={blogPost} />;
      }}
  </Query>
  );

I don't see how I would render server-side 500 instead of 200 otherwise and I can also imagine that quite a few developers are not aware of what's going on 🤔

WDYT folks?

[JohannesHome]

This is a big issue since this messes with SEO, loading pages being index etc. Apollo is swallowing errors, even though error handling has been implemented simply because it is not being propagated when doing SSR.

It works perfectly in the client. Is there any plans on fixing this? Or on how to do this the right way with apollo?

[jaydenseric]

@JohannesHome Apollo Client caches the objects received from query or mutation responses by their ids (normalization); it doesn't cache metadata about the actual network requests such as HTTP status. Where would that data live, if a response gets torn into multiple separately cached pieces? A query component might render data pieced together from cache from 5 separate requests. Loading and error states are local component state that can't be serialized on the server for hydration on the client. I doubt you will be able to SSR errors with Apollo anytime soon.

Some of my design goals for graphql-react were to think fresh about cache so that errors can be server side rendered, queries to multiple GraphQL APIs can be cached in one client instance, and remove the need for interface and union fragment matching. The implementation is waaaaay simpler; with a bundle impact around 2kb min gzip vs Apollo's ~32kb.

[JohannesHome]

@jaydenseric what are you saying?

My question: When an error occurs that is passed through by the Query component in the client-side, shouldn't that same behaviour occur on server side?

[jaydenseric]

Server side rendering happens in 2 phases, firstly getDataFromTree() populates a cache by fake rendering, then that cache is used to render on the server for real. Error state that was rendered within getDataFromTree() is lost when the actual render to string happens. Even if the getDataFromTree() render could be served, it has no way to put the error states into cache to serialize and send to the client, so you would get initial HTML with the errors, but then as soon as React runs on the client it would get wiped and show loading states (SSR/client markup mismatch).

[JohannesHome]

Sorry english is not my native tongue. So your saying no?

[jaydenseric]

shouldn't that same behaviour occur on server side

No, the behavior is not exactly the same on the server as the client. Even if it was, due to the Apollo cache format it has no way to communicate that there were loading errors to the client. So the client would notice some data is missing for some components and attempt to load them, only to discover the same errors the server discovered.

[JohannesHome]

Looks to me like this issue is a duplicate to apollographql/react-apollo#2134

I fixed the issue for me through catching the error from getDataFromTree() and rendering the app again with an error state that I pass in. This way the SSR error page looks the same as the page that the client would render.

[tobobo]

I've opened a PR to fix the docs with respect to the incorrect guidance around errorPolicy and cacheing. #4237

[hwillson]

This should no longer be an issue with current versions of apollo-client / react-apollo. Thanks!

[fenok]

And yet it still is.

[jacob-ebey]

This is still an issue. If you need a repo case, let me know and I can get you one.

[hwillson]

@jacob-ebey If you can provide a repro using React Apollo 3, that would be great!

[jacob-ebey]

@hwillson, Here is a repro case: https://github.com/jacob-ebey/apollo-ssr-bug

Just run yarn then yarn start to get going. If you remove the "error" field from the index.ts page's query, you'll be back in a successful situation.

(fyi, if you guys are hiring, I'd love to contribute to this project in a professional capacity)

[hwillson]

Thanks for the reproduction @jacob-ebey! We're definitely hiring - all of our open positions are here, but the position that's most directly related to this repo is here. Thanks again!