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

src/Query.tsx

@@ -172,6 +172,12 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
       ...opts,
       fetchPolicy,
     });
+
+    // Register the SSR observable, so it can be re-used once the value comes back.
+    if (this.context && this.context.renderPromises) {
+      this.context.renderPromises.registerSSRObservable(this, observable);
+    }
+
     const result = this.queryObservable!.currentResult();
 
     return result.loading ? observable.result() : false;
@@ -284,7 +290,16 @@ export default class Query<TData = any, TVariables = OperationVariables> extends
     const opts = this.extractOptsFromProps(props);
     // save for backwards compat of refetcherQueries without a recycler
     this.setOperations(opts);
-    this.queryObservable = this.client.watchQuery(opts);
+
+    // See if there is an existing observable that was used to fetch the same data and
+    // if so, use it instead since it will contain the proper queryId to fetch
+    // the result set. This is used during SSR.
+    if (this.context && this.context.renderPromises) {
+      this.queryObservable = this.context.renderPromises.getSSRObservable(this);
+    }
+    if (!this.queryObservable) {
+      this.queryObservable = this.client.watchQuery(opts);
+    }
   }
 
   private setOperations(props: QueryProps<TData, TVariables>) {

src/getDataFromTree.ts

@@ -1,50 +1,50 @@
 import * as React from 'react';
 import * as PropTypes from 'prop-types';
 import Query from './Query';
+import { ObservableQuery } from 'apollo-client';
+import { DocumentNode } from 'graphql';
 
-// Like a Set, but for tuples. In practice, this class is used to store
-// (query, JSON.stringify(variables)) tuples.
-class Trie {
-  private children: Map<any, Trie> | null = null;
-  private added = false;
-
-  has(...keys: any[]) {
-    let node: Trie = this;
-    return keys.every(key => {
-      const child = node.children && node.children.get(key);
-      return !!(child && (node = child));
-    }) && node.added;
-  }
+type QueryInfo = {
+  seen: boolean;
+  observable: ObservableQuery<any, any> | null;
+}
 
-  add(...keys: any[]) {
-    let node: Trie = this;
-    keys.forEach(key => {
-      const map = node.children || (node.children = new Map);
-      const child = map.get(key);
-      if (child) {
-        node = child;
-      } else {
-        map.set(key, node = new Trie());
-      }
-    });
-    node.added = true;
-  }
+function makeDefaultQueryInfo(): QueryInfo {
+  return {
+    seen: false,
+    observable: null,
+  };
 }
 
 export class RenderPromises {
   // Map from Query component instances to pending fetchData promises.
   private queryPromises = new Map<Query<any, any>, Promise<any>>();
 
-  // A way of remembering queries we've seen during previous renderings,
-  // so that we never attempt to fetch them again in future renderings.
-  private queryGraveyard = new Trie();
+  // Two-layered map from (query document, stringified variables) to QueryInfo
+  // objects. These QueryInfo objects are intended to survive through the whole
+  // getMarkupFromTree process, whereas specific Query instances do not survive
+  // beyond a single call to renderToStaticMarkup.
+  private queryInfoTrie = new Map<DocumentNode, Map<string, QueryInfo>>();
+
+  // Registers the server side rendered observable.
+  public registerSSRObservable<TData, TVariables>(
+    queryInstance: Query<TData, TVariables>,
+    observable: ObservableQuery<any, TVariables>,
+  ) {
+    this.lookupQueryInfo(queryInstance).observable = observable;
+  }
+
+  // Get's the cached observable that matches the SSR Query instances query and variables.
+  public getSSRObservable<TData, TVariables>(queryInstance: Query<TData, TVariables>) {
+    return this.lookupQueryInfo(queryInstance).observable;
+  }
 
   public addQueryPromise<TData, TVariables>(
     queryInstance: Query<TData, TVariables>,
     finish: () => React.ReactNode,
   ): React.ReactNode {
-    const { query, variables } = queryInstance.props;
-    if (!this.queryGraveyard.has(query, JSON.stringify(variables))) {
+    const info = this.lookupQueryInfo(queryInstance);
+    if (!info.seen) {
       this.queryPromises.set(
         queryInstance,
         new Promise(resolve => {
@@ -65,7 +65,6 @@ export class RenderPromises {
   public consumeAndAwaitPromises() {
     const promises: Promise<any>[] = [];
     this.queryPromises.forEach((promise, queryInstance) => {
-      const { query, variables } = queryInstance.props;
       // Make sure we never try to call fetchData for this query document and
       // these variables again. Since the queryInstance objects change with
       // every rendering, deduplicating them by query and variables is the
@@ -75,12 +74,25 @@ export class RenderPromises {
       // rendering of an unwanted loading state, but that's not nearly as bad
       // as getting stuck in an infinite rendering loop because we kept calling
       // queryInstance.fetchData for the same Query component indefinitely.
-      this.queryGraveyard.add(query, JSON.stringify(variables));
+      this.lookupQueryInfo(queryInstance).seen = true;
       promises.push(promise);
     });
     this.queryPromises.clear();
     return Promise.all(promises);
   }
+
+  private lookupQueryInfo<TData, TVariables>(
+    queryInstance: Query<TData, TVariables>,
+  ): QueryInfo {
+    const { queryInfoTrie } = this;
+    const { query, variables } = queryInstance.props;
+    const varMap = queryInfoTrie.get(query) || new Map<string, QueryInfo>();
+    if (!queryInfoTrie.has(query)) queryInfoTrie.set(query, varMap);
+    const variablesString = JSON.stringify(variables);
+    const info = varMap.get(variablesString) || makeDefaultQueryInfo();
+    if (!varMap.has(variablesString)) varMap.set(variablesString, info);
+    return info;
+  }
 }
 
 export default function getDataFromTree(

test/client/getDataFromTree.test.tsx

@@ -1568,5 +1568,49 @@ describe('SSR', () => {
         expect(markup).toMatch(/James/);
       });
     });
+
+    it('should pass any GraphQL errors in props along with data during a SSR when errorPolicy="all"', done => {
+      const query: DocumentNode = gql`
+        query people {
+          allPeople {
+            people {
+              name
+            }
+          }
+        }
+      `;
+      const link = mockSingleLink({
+        request: { query },
+        result: {
+          data: {
+            allPeople: {
+              people: null,
+            },
+          },
+          errors: [new Error('this is an error')],
+        },
+      });
+
+      const client = new ApolloClient({
+        link,
+        cache: new Cache({ addTypename: false }),
+      });
+
+      const app = (
+        <ApolloProvider client={client}>
+          <Query query={query} errorPolicy="all">
+            {({ data, error }: any) => {
+              expect(data).toMatchObject({ allPeople: { people: null } });
+              expect(error).toBeDefined();
+              expect(error.graphQLErrors[0].message).toEqual('this is an error');
+              done();
+              return null;
+            }}
+          </Query>
+        </ApolloProvider>
+      );
+
+      getDataFromTree(app);
+    });
   });
 });