From 98b60ed6cd35d8a2d6f159633f30c15d98124c2e Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Mon, 27 Feb 2017 11:41:02 +1100 Subject: [PATCH 1/5] Ensure that SSR completes when the GraphQL server throws errors. See #406. We still end up rendering a loading screen but that is better than just bailing out of SSR completely. --- src/server.ts | 8 +++++++- test/react-web/server/index.test.tsx | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/server.ts b/src/server.ts index dbf965916e..c9a14540b9 100644 --- a/src/server.ts +++ b/src/server.ts @@ -128,7 +128,13 @@ export function getDataFromTree(rootElement, rootContext: any = {}, fetchRoot: b // wait on each query that we found, re-rendering the subtree when it's done const mappedQueries = queries.map(({ query, element, context }) => { // we've just grabbed the query for element, so don't try and get it again - return query.then(_ => getDataFromTree(element, context, false)); + return query.then(_ => getDataFromTree(element, context, false)) + // If there's an error in the query, we may as well stop, as currently + // we will "forget" this when the "rendering" SSR runs (i.e. we will + // re-run the query, and rendering in a loading state). + // If we change that in future, it may be worth running `getDataFromTree` + // on the subtree, just in case the user runs subqueries in the error state + .catch(e => null); }); return Promise.all(mappedQueries).then(_ => null); } diff --git a/test/react-web/server/index.test.tsx b/test/react-web/server/index.test.tsx index ac6effa146..211069a98c 100644 --- a/test/react-web/server/index.test.tsx +++ b/test/react-web/server/index.test.tsx @@ -263,6 +263,30 @@ describe('SSR', () => { ; }); + it('should handle errors thrown by queries', () => { + const query = gql`{ currentUser { firstName } }`; + const networkInterface = mockNetworkInterface( + { request: { query }, error: new Error('Failed to fetch'), delay: 50 } + ); + const apolloClient = new ApolloClient({ networkInterface, addTypename: false }); + + const WrappedElement = graphql(query)(({ data }) => ( +
{data.loading ? 'loading' : data.error}
+ )); + + const Page = () => (
Hi
); + + const app = (); + + return getDataFromTree(app) + .then(() => { + const markup = ReactDOM.renderToString(app); + // It renders in a loading state as errored query isn't shared between + // the query fetching run and the rendering run. + expect(markup).toMatch(/loading/); + }); + }); + it('should correctly skip queries (deprecated)', () => { const query = gql`{ currentUser { firstName } }`; From d13b4f5333614139a1d5a04b0519dcb97f883abc Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 14 Mar 2017 16:53:57 +1100 Subject: [PATCH 2/5] Throw errors from SSR, but still execute all queries. See https://github.com/apollographql/react-apollo/pull/488#issuecomment-284415525 --- src/server.ts | 21 ++++++++++++++------- test/react-web/server/index.test.tsx | 6 +++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/server.ts b/src/server.ts index c9a14540b9..1369bb3844 100644 --- a/src/server.ts +++ b/src/server.ts @@ -125,18 +125,25 @@ export function getDataFromTree(rootElement, rootContext: any = {}, fetchRoot: b // no queries found, nothing to do if (!queries.length) return Promise.resolve(); + const errors = []; // wait on each query that we found, re-rendering the subtree when it's done const mappedQueries = queries.map(({ query, element, context }) => { // we've just grabbed the query for element, so don't try and get it again return query.then(_ => getDataFromTree(element, context, false)) - // If there's an error in the query, we may as well stop, as currently - // we will "forget" this when the "rendering" SSR runs (i.e. we will - // re-run the query, and rendering in a loading state). - // If we change that in future, it may be worth running `getDataFromTree` - // on the subtree, just in case the user runs subqueries in the error state - .catch(e => null); + .catch(e => errors.push(e)); + }); + + // Run all queries. If there are errors, still wait for all queries to execute + // so the caller can ignore them if they wish. See https://github.com/apollographql/react-apollo/pull/488#issuecomment-284415525 + return Promise.all(mappedQueries).then(_ => { + if (errors.length > 0) { + const error = errors.length === 1 + ? errors[0] + : new Error(`${errors.length} errors were thrown when executing your GraphQL queries.`); + error.queryErrors = errors; + throw error; + } }); - return Promise.all(mappedQueries).then(_ => null); } export function renderToStringWithData(component) { diff --git a/test/react-web/server/index.test.tsx b/test/react-web/server/index.test.tsx index 0f113098f0..1fc8971d8c 100644 --- a/test/react-web/server/index.test.tsx +++ b/test/react-web/server/index.test.tsx @@ -279,7 +279,11 @@ describe('SSR', () => { const app = (); return getDataFromTree(app) - .then(() => { + .catch((e) => { + expect(e).toBeTruthy; + expect(e.queryErrors.length).toEqual(1); + + // But we can still render the app if we want to const markup = ReactDOM.renderToString(app); // It renders in a loading state as errored query isn't shared between // the query fetching run and the rendering run. From b6187e217a128013322654672a0c45ee40038a4f Mon Sep 17 00:00:00 2001 From: Caleb Meredith Date: Tue, 14 Mar 2017 14:51:10 -0400 Subject: [PATCH 3/5] Update server.ts --- src/server.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/server.ts b/src/server.ts index 1369bb3844..764fe8a0ce 100644 --- a/src/server.ts +++ b/src/server.ts @@ -129,8 +129,11 @@ export function getDataFromTree(rootElement, rootContext: any = {}, fetchRoot: b // wait on each query that we found, re-rendering the subtree when it's done const mappedQueries = queries.map(({ query, element, context }) => { // we've just grabbed the query for element, so don't try and get it again - return query.then(_ => getDataFromTree(element, context, false)) - .catch(e => errors.push(e)); + return ( + query + .then(_ => getDataFromTree(element, context, false)) + .catch(e => errors.push(e)) + ); }); // Run all queries. If there are errors, still wait for all queries to execute From 4afb2707c34b3473b2e0108a57428f713b00f2f2 Mon Sep 17 00:00:00 2001 From: Caleb Meredith Date: Tue, 14 Mar 2017 14:52:22 -0400 Subject: [PATCH 4/5] Update index.test.tsx --- test/react-web/server/index.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/react-web/server/index.test.tsx b/test/react-web/server/index.test.tsx index 1fc8971d8c..6112182287 100644 --- a/test/react-web/server/index.test.tsx +++ b/test/react-web/server/index.test.tsx @@ -280,7 +280,7 @@ describe('SSR', () => { return getDataFromTree(app) .catch((e) => { - expect(e).toBeTruthy; + expect(e).toBeTruthy(); expect(e.queryErrors.length).toEqual(1); // But we can still render the app if we want to From 10fe30abfc0875a4edcf1602c81d9f432a61ffd9 Mon Sep 17 00:00:00 2001 From: Caleb Meredith Date: Fri, 17 Mar 2017 10:17:05 -0400 Subject: [PATCH 5/5] Update Changelog.md --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 2abe0a2b2a..b9f914d16b 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ Expect active development and potentially significant breaking changes in the `0.x` track. We'll try to be diligent about releasing a `1.0` version in a timely fashion (ideally within 1 or 2 months), so that we can take advantage of SemVer to signify breaking changes from that point on. ### vNext +- Make sure that all queries resolve or reject if an error was thrown when server side rendering. [PR #488](https://github.com/apollographql/react-apollo/pull/488) ### 1.0.0-rc.1 - Update dependency to Apollo Client 1.0.0-rc.1 [PR #520](https://github.com/apollographql/react-apollo/pull/520)