diff --git a/.changeset/curly-cameras-carry.md b/.changeset/curly-cameras-carry.md new file mode 100644 index 00000000000..f45ed62e851 --- /dev/null +++ b/.changeset/curly-cameras-carry.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': patch +--- + +Ensure the `onError` callback is called when the `errorPolicy` is set to "all" and partial data is returned. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 884c5043481..3df84b8bdcd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,11 +40,15 @@ jobs: uses: changesets/action@v1 with: version: npm run changeset-version - publish: npm run changeset-publish + publish: npm run changeset-publish -- --tag next env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} NPM_TOKEN: ${{ secrets.NPM_TOKEN }} + - name: Echo values from changesets step + if: steps.changesets.outcome == 'success' + run: echo "${{steps.changesets.outcome}} v${{ fromJson(steps.changesets.outputs.publishedPackages)[0].version }}" + - name: Send a Slack notification on publish if: steps.changesets.outcome == 'success' && steps.changesets.outputs.published == 'true' id: slack @@ -56,6 +60,6 @@ jobs: # a comma-delimited list of channel IDs channel-id: 'C01PS0CB41G' # For posting a simple plain text message - slack-message: "Published @apollo/client v${{ fromJson(steps.changesets.outputs.publishedPackages)[0].version }}" + slack-message: "Published v${{ fromJson(steps.changesets.outputs.publishedPackages)[0].version }}" env: SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} diff --git a/config/bundlesize.ts b/config/bundlesize.ts index 38df3b2f8e4..6fd99531b19 100644 --- a/config/bundlesize.ts +++ b/config/bundlesize.ts @@ -3,7 +3,7 @@ import { join } from "path"; import { gzipSync } from "zlib"; import bytes from "bytes"; -const gzipBundleByteLengthLimit = bytes("32KB"); +const gzipBundleByteLengthLimit = bytes("32.03KB"); const minFile = join("dist", "apollo-client.min.cjs"); const minPath = join(__dirname, "..", minFile); const gzipByteLen = gzipSync(readFileSync(minPath)).byteLength; diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index f8dfc3175d0..3cc6e1fab1a 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -320,6 +320,8 @@ Array [ "mockObservableLink", "mockSingleLink", "subscribeAndCount", + "tick", + "wait", "withErrorSpy", "withLogSpy", "withWarningSpy", @@ -335,6 +337,8 @@ Array [ "mockObservableLink", "mockSingleLink", "subscribeAndCount", + "tick", + "wait", "withErrorSpy", "withLogSpy", "withWarningSpy", diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index d0bc999bd13..3254fa371ab 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -23,6 +23,7 @@ import { MockedProvider, MockSubscriptionLink, mockSingleLink, + tick, } from '../../../testing'; import { QueryResult } from "../../types/types"; import { useQuery } from '../useQuery'; @@ -1743,7 +1744,408 @@ describe('useQuery Hook', () => { expect(result.current.error!.message).toBe('error'); }); - it('should only call onError callbacks once', async () => { + it('calls `onError` when a GraphQL error is returned', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + errors: [new GraphQLError('error')], + }, + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('error'); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + new ApolloError({ graphQLErrors: [new GraphQLError('error')] }) + ); + }); + }); + + it('calls `onError` when a network error has occured', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + error: new Error('Could not fetch') + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + expect(result.current.error).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('Could not fetch'); + expect(result.current.error!.networkError).toEqual( + new Error('Could not fetch') + ); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + new ApolloError({ networkError: new Error('Could not fetch') }) + ); + }); + }); + + it('removes partial data from result when response has errors', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + data: { hello: null }, + errors: [new GraphQLError('Could not fetch "hello"')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + expect(result.current.error).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('Could not fetch "hello"'); + expect(result.current.error!.graphQLErrors).toEqual([ + new GraphQLError('Could not fetch "hello"') + ]); + }); + + it('does not call `onError` when returning GraphQL errors while using an `errorPolicy` set to "ignore"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + errors: [new GraphQLError('error')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, errorPolicy: 'ignore' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + expect(result.current.error).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeUndefined(); + + await tick(); + + expect(onError).not.toHaveBeenCalled(); + }); + + it('calls `onError` when a network error has occurred while using an `errorPolicy` set to "ignore"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + error: new Error('Could not fetch') + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, errorPolicy: 'ignore' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + expect(result.current.error).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('Could not fetch'); + expect(result.current.error!.networkError).toEqual( + new Error('Could not fetch') + ); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + new ApolloError({ networkError: new Error('Could not fetch') }) + ); + }); + }); + + it('returns partial data and discards GraphQL errors when using an `errorPolicy` set to "ignore"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + data: { hello: null }, + errors: [new GraphQLError('Could not fetch "hello"')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { errorPolicy: 'ignore' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toEqual({ hello: null }) + expect(result.current.error).toBeUndefined(); + }); + + it('calls `onCompleted` with partial data but avoids calling `onError` when using an `errorPolicy` set to "ignore"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + data: { hello: null }, + errors: [new GraphQLError('Could not fetch "hello"')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const onCompleted = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, onCompleted, errorPolicy: 'ignore' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toEqual({ hello: null }) + expect(result.current.error).toBeUndefined(); + + await waitFor(() => { + expect(onCompleted).toHaveBeenCalledTimes(1); + expect(onCompleted).toHaveBeenCalledWith({ hello: null }); + + expect(onError).not.toHaveBeenCalled(); + }); + }); + + it('calls `onError` when returning GraphQL errors while using an `errorPolicy` set to "all"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + errors: [new GraphQLError('error')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, errorPolicy: 'all' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('error'); + expect(result.current.error!.graphQLErrors).toEqual([ + new GraphQLError('error') + ]); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + new ApolloError({ graphQLErrors: [new GraphQLError('error')] }) + ); + }); + }); + + it('returns partial data when returning GraphQL errors while using an `errorPolicy` set to "all"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + data: { hello: null }, + errors: [new GraphQLError('Could not fetch "hello"')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, errorPolicy: 'all' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toEqual({ hello: null }); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('Could not fetch "hello"'); + expect(result.current.error!.graphQLErrors).toEqual([ + new GraphQLError('Could not fetch "hello"') + ]); + }); + + it('calls `onError` but not `onCompleted` when returning partial data with GraphQL errors while using an `errorPolicy` set to "all"', async () => { + const query = gql`{ hello }`; + const mocks = [ + { + request: { query }, + result: { + data: { hello: null }, + errors: [new GraphQLError('Could not fetch "hello"')] + } + }, + ]; + + const cache = new InMemoryCache(); + const wrapper = ({ children }: any) => ( + {children} + ); + + const onError = jest.fn(); + const onCompleted = jest.fn(); + const { result, waitForNextUpdate } = renderHook( + () => useQuery(query, { onError, onCompleted, errorPolicy: 'all' }), + { wrapper }, + ); + + expect(result.current.loading).toBe(true); + expect(result.current.data).toBe(undefined); + + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.data).toEqual({ hello: null }); + expect(result.current.error).toBeInstanceOf(ApolloError); + expect(result.current.error!.message).toBe('Could not fetch "hello"'); + expect(result.current.error!.graphQLErrors).toEqual([ + new GraphQLError('Could not fetch "hello"') + ]); + + await waitFor(() => { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + new ApolloError({ + graphQLErrors: [new GraphQLError('Could not fetch "hello"')] + }) + ); + + expect(onCompleted).not.toHaveBeenCalled(); + }) + }); + + it('calls `onError` a single time when refetching returns a successful result', async () => { const query = gql`{ hello }`; const mocks = [ { diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 2bf72a83329..6486118d329 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -503,10 +503,12 @@ class InternalState { private handleErrorOrCompleted(result: ApolloQueryResult) { if (!result.loading) { + const error = this.toApolloError(result); + // wait a tick in case we are in the middle of rendering a component Promise.resolve().then(() => { - if (result.error) { - this.onError(result.error); + if (error) { + this.onError(error); } else if (result.data) { this.onCompleted(result.data); } @@ -516,6 +518,12 @@ class InternalState { } } + private toApolloError(result: ApolloQueryResult): ApolloError | undefined { + return isNonEmptyArray(result.errors) + ? new ApolloError({ graphQLErrors: result.errors }) + : result.error + } + private getCurrentResult(): ApolloQueryResult { // Using this.result as a cache ensures getCurrentResult continues returning // the same (===) result object, unless state.setResult has been called, or diff --git a/src/testing/core/index.ts b/src/testing/core/index.ts index e7356452c71..c03a7e8cf69 100644 --- a/src/testing/core/index.ts +++ b/src/testing/core/index.ts @@ -11,4 +11,5 @@ export { export { createMockClient } from './mocking/mockClient'; export { default as subscribeAndCount } from './subscribeAndCount'; export { itAsync } from './itAsync'; +export { wait, tick } from './wait' export * from './withConsoleSpy'; diff --git a/src/testing/core/wait.ts b/src/testing/core/wait.ts new file mode 100644 index 00000000000..263f6697623 --- /dev/null +++ b/src/testing/core/wait.ts @@ -0,0 +1,7 @@ +export async function wait(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +export async function tick() { + return wait(0); +}