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

useLazyQuery promise .catch not catching errors, when useMutation does #9142

Closed
MattBred opened this issue Nov 30, 2021 · 3 comments
Closed

Comments

@MattBred
Copy link

MattBred commented Nov 30, 2021

I should be able to add a .catch to the useLazyQuery promise, which should be called when the response is a graphql or network error. Right now, the .catch does not fire in either case, and the error is present in the result variable inside the .then.

This is inconsistent with the useMutation which does fire the .catch on errors.


Intended outcome:

someQuery()
    .then((r) => console.log('query done!', r))
    .catch((e) => console.log('query error!', e))

I should get query error! when there is an error.

Actual outcome:

someQuery()
    .then((r) => console.log('query done!', r))
    .catch((e) => console.log('query error!', e))

I get query done! when there is an error.


How to reproduce the issue:

Click to open code example

The following example uses the MockedProvider but has the exact same result when using the normal ApolloProvider

import React from 'react';
import ReactDOM from 'react-dom';
import './index.css';
import {MockedProvider} from "@apollo/client/testing";
import {gql, InMemoryCache, useLazyQuery, useMutation} from "@apollo/client";

const cache = new InMemoryCache();
const SOME_MUTATION = gql`
    mutation SomeMutation {
        test
    }
`;
const SOME_QUERY = gql`
    query SomeQuery {
        test
    }
`;
const mocks = [
    {
        request: {
            query: SOME_MUTATION,
        },
        result: {
            errors: [{message: 'ERROR'}]
        }
    },
    {
        request: {
            query: SOME_QUERY,
        },
        result: {
            errors: [{message: 'ERROR'}]
        }
    }
]

function App() {
    const [someMutation] = useMutation(SOME_MUTATION);
    const [someQuery] = useLazyQuery(SOME_QUERY);

    const startMutation = () => {
        someMutation()
            .then((r) => console.log('mutation done!', r))
            .catch((e) => console.log('mutation error!', e))
    }

    const startUseLazyQuery = () => {
        someQuery()
            .then((r) => console.log('query done!', r))
            .catch((e) => console.log('query error!', e))
    }

    return (
        <div>
            <button onClick={startMutation}>Click me for mutation</button>
            <br/>
            <button onClick={startUseLazyQuery}>Click me for lazy query</button>
        </div>
    )
}

ReactDOM.render(
  <React.StrictMode>
      <div className="App">
          <MockedProvider cache={cache} mocks={mocks}>
              <App />
          </MockedProvider>
      </div>
  </React.StrictMode>,
  document.getElementById('root')
);

Versions

  System:
    OS: Linux 4.15 Ubuntu 18.04.6 LTS (Bionic Beaver)
  Binaries:
    Node: 12.22.7 - /usr/bin/node
    Yarn: 1.21.1 - /usr/bin/yarn
    npm: 6.14.15 - /usr/bin/npm
  Browsers:
    Chrome: 95.0.4638.54
    Firefox: 94.0
  npmPackages:
    @apollo/client: ^3.5.5 => 3.5.5
@benjamn
Copy link
Member

benjamn commented Dec 1, 2021

@MattBred Thanks for reporting this!

Seems like this Promise needs to be rejected on errors, but we're currently only using its resolve function. Can you take a look @brainkim?

@brainkim
Copy link
Contributor

brainkim commented Dec 2, 2021

I object to the bug label because this is new 3.5 functionality but yeah this seem right to do. Though, I don’t understand why people don’t just call ApolloClient.query() directly at some point.

@benjamn
Copy link
Member

benjamn commented May 5, 2022

I now believe I was wrong above, for the reasons I give in #9684. Throwing result.error needlessly discards other result properties that might be useful, and provides no new information. Each one of our three error policies can be achieved by returning an eventually-resolved Promise<QueryResult<TData, TVariables>> from the useLazyQuery execution function.

Here's an example of an issue in direct conflict with this one: #9669

benjamn added a commit that referenced this issue May 5, 2022
This perhaps gives some reassurance the goals of issue #9142 are still
achievable, in the sense that network errors can be obtained from the
Promise returned by the useLazyQuery execution function, even if those
errors are not thrown (and the Promise is never rejected).
@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.
Projects
None yet
Development

No branches or pull requests

3 participants