Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[V3] - Missing mocks in MockedProvider throws uncaught error #6559

Closed
niekert opened this issue Jul 8, 2020 · 11 comments
Closed

[V3] - Missing mocks in MockedProvider throws uncaught error #6559

niekert opened this issue Jul 8, 2020 · 11 comments

Comments

@niekert
Copy link

niekert commented Jul 8, 2020

I am not sure if this is a bug, or an intentional change (in #5474 ?), but the change in behavior is quite significant and caused our test suite to fail upon upgrading to v3.

Reproduction

  1. Render a MockedProvider in a test environment and pass it an empty array as mocks
  2. Render a component with a useQuery hook in your test
  3. Run the test.

Expected result
The missing mock Error, is part of the error property on the return value of useQuery

const { error } = useQuery(gql`query someQueryThatsNotInMockedProvider { }`);

console.log(error); // This error contains the missing mock error.

Actual result
Uncaught error, causing the test to fail completely.

Reproduction repo
This repo: https://github.com/niekert/apollo-3-missing-mocks-behavior

  1. run npm install
  2. run npm test apollo-2.spec.js - test passes on apollo Client 2 - missing mocks is part of return value of useQuery
  3. run npm test apollo-3.spec.js - test throws uncaught error: No more mocked responses for query
Error: Uncaught [Error: No more mocked responses for the query: query viewer {
      viewer {
        id
        __typename
      }
    }
    , variables: {}]

Version: @apollo/client@^3.0.0

Will happily provide more details if needed. Thank you!

@niekert
Copy link
Author

niekert commented Jul 16, 2020

As a temporary workaround, I managed to create a custom `MockLink and ignore the missing mocks errors, like so:

  const mockLink = new MockLink(mocks, true);
  const originalOnError = mockLink.onError;
  mockLink.setOnError((error) => {
    if (isMissingMockError(error)) {
      // Do nothing.
      return;
    }

    throw error;
  });

return <ApolloMockClient mocks={mocks} link={link}>{children}</ApolloMockClient>

@RicardoTrindade
Copy link

RicardoTrindade commented Jul 23, 2020

Also facing the same issue I believe. Tests were passing on v2 and now failing on v3. However, (and this might help debug the issue)
when I added the errorMock twice to the mocks array (mocks={[errorMock, mockA, mockB, errorMock]}) the test started passing on v3.

@RicardoTrindade
Copy link

RicardoTrindade commented Jul 23, 2020

After some more debugging it seems that this line
this.mockedResponsesByKey[key].splice(responseIndex, 1)
on mockLink.ts is removing the error response from mockedResponsesByKey thus causing the fail.

Can't really tell what the line is doing there but removing it makes a couple of tests fail 😅

@martdavidson
Copy link

Can confirm this is an issue, previously we were only mocking calls that we were interested in running tests against specific responses, and ignoring other calls that might've been made by the component / page under test.

Now it looks as though any unmocked call will throw an error and fail the test. @benjamn Can you clarify if this is intentional?

@dheardal
Copy link

dheardal commented Aug 7, 2020

As a solution you can pass an error link into the MockedProvider to handle the errors

const errorLink = onError(({ graphQLErrors, networkError }) => {
  if (graphQLErrors)
    graphQLErrors.map(({ message, locations, path }) =>
      console.log(
        `[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`,
      ),
    );

  if (networkError) console.log(`[Network error]: ${networkError}`);
});


<MockedProvider addTypename={false} link={errorLink} />

@GhaythFuad
Copy link

GhaythFuad commented Aug 19, 2020

As @RicardoTrindade mentioned, this.mockedResponsesByKey[key].splice(responseIndex, 1), is causing the mock to be removed once it is used, but if you look at the lines right below that line, you'll find:

var newData = response.newData;
        if (newData) {
            response.result = newData();
            this.mockedResponsesByKey[key].push(response);
        }

Which means you can provide a function to renew the mock every time, this is how I did it:

const dogMock = {
    request: {
      query: GET_DOG_QUERY,
      variables: { name: 'Buck' },
    },
    result: {
      data: { dog: { id: 1, name: 'Buck', breed: 'poodle' } },
    },
    newData : () => {return {data: { dog: { id: 1, name: 'Buck', breed: 'poodle' } }};}
  };

It's weird I didn't find this mentioned in the docs.

@tknickman
Copy link

We are also experiencing the same issue with our application when migrating apollo-client@2.3 to @apollo/client@3.2

In AC2, when writing tests and using the MockedProvider, queries that weren’t mocked threw an error at the query itself (returned in the errors field from useQuery as you mention @niekert). However, in AC3, queries that aren’t mocked throw an uncaught error at the test itself - causing the test to fail.

We have hundreds of tests that relied on this behavior (either intentionally or not) - mostly because when testing a single component, the developer didn’t need to worry about mocking all of the queries being sent in children components - only a mock for the queries in the component you are directly testing was needed.

I have setup a comparison (with the workaround suggested by @dheardal) as well here:
(code is identical with the only difference being the AC version)
Test working in AC2: https://github.com/tknickman/AC2-MockedProvider-Reproduction
Test breaking in AC3: https://github.com/tknickman/AC3-MockedProvider-Reproduction

Enable the workaround by following the instructions here:
https://github.com/tknickman/AC3-MockedProvider-Reproduction/blob/master/src/CountrySelect/CountrySelect.test.js#L54

I'm curious if:

  1. This change was intentional
  2. If so, why this new test error behavior was left out of the AC3 Migration Guide / ChangeLog

I'm considering adding our own implementation of the MockedProvider to use with our legacy tests for now to expedite this upgrade. Something like (untested pseudo code):

const MockedProvider = ({
    children,
    mocks = [],
    addTypename = true,
    ...props
  }) => {
    const mockLink = new MockLink(mocks, addTypename);
    mockLink.setOnError((errors) => {
      const { graphQLErrors, networkError } = errors;
      if (graphQLErrors)
        graphQLErrors.map(({ message, locations, path }) =>
          console.error(
            `[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`
          )
        );

      if (networkError) console.error(`[Network error]: ${networkError}`);

      if (errors) {
        console.error(errors);
      }
    });

    return (
      <MockedProvider
        link={mockLink}
        mocks={mocks}
        addTypename={addTypename}
        {...props}
      >
        {children}
      </MockedProvider>
    );
  };

@hwillson hwillson self-assigned this Sep 28, 2020
@hwillson
Copy link
Member

Hi all - we're looking into this and will post back with an update shortly.

@hwillson
Copy link
Member

hwillson commented Oct 1, 2020

We're planning on changing this behavior back to the way it was, where the error is reported through the link observable (error in the response object), instead of throwing. The only gotcha here is that it's entirely possible people using AC3 are now reliant on the new behavior (where the error is thrown instead of returned via the response object). Changing things back could be considered a breaking change, but this could also be labelled as a bug since we didn't mean for this change to impact people migrating to AC3 as much as it has. So ... we're still discussing this, but we're leaning towards changing things back in AC 3.3. Then if anyone wants to restore the thrown error functionality, they can define a custom error function like:

const mocks = // ...your mocks...
const mockLink = new MockLink(mocks);
mockLink.setOnError(error => { throw error });
// ... then call with something like
<MockedProvider link={mockLink}>

@pinguinjkeke
Copy link

It's working on beta 11

@hwillson hwillson removed their assignment May 3, 2021
@hwillson
Copy link
Member

hwillson commented May 3, 2021

#7110 was merged to help with this. Thanks!

@hwillson hwillson closed this as completed May 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants