From 59e02c8ed5638d7d0d64dd914edbc85c26d5f3ba Mon Sep 17 00:00:00 2001 From: Anand Sundaram Date: Fri, 22 Jun 2018 21:54:19 -0400 Subject: [PATCH 1/2] fix: Don't obscure errors from inner wrapped components. --- src/getDataFromTree.ts | 49 ++++++++++------ test/client/getDataFromTree.test.tsx | 83 +++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 18 deletions(-) diff --git a/src/getDataFromTree.ts b/src/getDataFromTree.ts index f67419f8fb..3a53dd9ba1 100755 --- a/src/getDataFromTree.ts +++ b/src/getDataFromTree.ts @@ -193,9 +193,10 @@ function getPromisesFromTree({ return promises; } -export default function getDataFromTree( +function getDataAndErrorsFromTree( rootElement: React.ReactNode, rootContext: any = {}, + storeError: Function, ): Promise { const promises = getPromisesFromTree({ rootElement, rootContext }); @@ -203,24 +204,38 @@ export default function getDataFromTree( return Promise.resolve(); } - const errors: any[] = []; - const mappedPromises = promises.map(({ promise, context, instance }) => { return promise - .then(_ => getDataFromTree(instance.render(), context)) - .catch(e => errors.push(e)); + .then(_ => getDataAndErrorsFromTree(instance.render(), context, storeError)) + .catch(e => storeError(e)); }); - return Promise.all(mappedPromises).then(_ => { - if (errors.length > 0) { - const error = - errors.length === 1 - ? errors[0] - : new Error( - `${errors.length} errors were thrown when executing your fetchData functions.`, - ); - error.queryErrors = errors; - throw error; - } - }); + return Promise.all(mappedPromises); +} + +function processErrors(errors: any[]) { + switch (errors.length) { + case 0: + break; + case 1: + throw errors.pop(); + default: + const wrapperError: any = new Error( + `${errors.length} errors were thrown when executing your fetchData functions.`, + ); + wrapperError.queryErrors = errors; + throw wrapperError; + } +} + +export default function getDataFromTree( + rootElement: React.ReactNode, + rootContext: any = {}, +): Promise { + const errors: any[] = []; + const storeError = (error: any) => errors.push(error); + + return getDataAndErrorsFromTree(rootElement, rootContext, storeError).then(_ => + processErrors(errors), + ); } diff --git a/test/client/getDataFromTree.test.tsx b/test/client/getDataFromTree.test.tsx index 86d35eb853..ad82446aff 100644 --- a/test/client/getDataFromTree.test.tsx +++ b/test/client/getDataFromTree.test.tsx @@ -692,6 +692,87 @@ describe('SSR', () => { }); }); + it('should return multiple errors in nested wrapped components without circular reference to wrapper error', () => { + const lastNameQuery = gql` + { + currentUser { + lastName + } + } + `; + interface LastNameData { + currentUser: { + lastName: string; + }; + } + const firstNameQuery = gql` + { + currentUser { + firstName + } + } + `; + interface FirstNameData { + currentUser: { + firstName: string; + }; + } + + const userData = { currentUser: { lastName: 'Tester', firstName: 'James' } }; + const link = mockSingleLink( + { + request: { query: lastNameQuery }, + result: { data: userData }, + delay: 50, + }, + { + request: { query: firstNameQuery }, + result: { data: userData }, + delay: 50, + }, + ); + const apolloClient = new ApolloClient({ + link, + cache: new Cache({ addTypename: false }), + }); + + interface Props {} + + type WithLastNameProps = ChildProps; + const withLastName = graphql(lastNameQuery); + + const BorkedComponent = () => { + throw new Error('foo'); + }; + + const WrappedBorkedComponent = withLastName(BorkedComponent); + + const ContainerComponent: React.StatelessComponent = ({ data }) => ( +
+ {!data || data.loading || !data.currentUser ? 'loading' : data.currentUser.lastName} + + +
+ ); + + type WithFirstNameProps = ChildProps; + const withFirstName = graphql(firstNameQuery); + + const WrappedContainerComponent = withFirstName(ContainerComponent); + + const app = ( + + + + ); + + return getDataFromTree(app).catch(e => { + expect(e.toString()).toEqual(expect.stringContaining('2 errors were thrown')); + expect(e.queryErrors.length).toBeGreaterThan(1); + expect(e.toString()).not.toEqual(e.queryErrors[0].toString()); + }); + }); + it('should handle errors thrown by queries', () => { const query = gql` { @@ -737,7 +818,7 @@ describe('SSR', () => { return getDataFromTree(app).catch(e => { expect(e).toBeTruthy(); - expect(e.queryErrors.length).toEqual(1); + expect(e.queryErrors).toBeUndefined(); // But we can still render the app if we want to const markup = ReactDOM.renderToString(app); From e4fee507efcd8410cdf1e268f600064871a00fb1 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 17 Jul 2018 14:41:50 -0400 Subject: [PATCH 2/2] Changelog update --- Changelog.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Changelog.md b/Changelog.md index 65b7d71cae..b0aa24603d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,12 @@ # Change log +## vNext + +- Fixed an issue in `getDataFromTree` where queries that threw more than one + error had error messages swallowed, and returned an invalid error object + with circular references. Multiple errors are now preserved. + [@anand-sundaram-zocdoc](https://github.com/anand-sundaram-zocdoc) in [#2133](https://github.com/apollographql/react-apollo/pull/2133) + ## 2.1.9 (July 4, 2018) - Added `onCompleted` and `onError` props to the `Query` component, than can