Apollo-client: useMutation.mutate() promise doesnt catch errors and always fulfills with undefined data

[oscmedgon]

Intended outcome:

I have a series of mutations that I need to execute in parallel at some point, and since useMutation.mutate() returns a promise I did a Promise.all, I got some errors due to mismatched variables, and the promise was fulfilled instead of rejected, and In the promise response, I got an array of undefined values, no matter if the request went wrong or it succeeds.

Actual outcome:

What I expected was to reject the promise when there are any errors, can be not passing some required variables, any error on the backend or the database.

How to reproduce the issue:

To reproduce the error you just need a mutation that it's going to fail, for example the model requires myData but is undefined.

MyComponent() {
    const [executeQuery, { data }] = useMutation(MUTATION);
    function onSendData() {
        executeQuery({variables: {myData: undefined}})
            // I'm being executed also when an error happen
            .then(console.log)
            // I'm never gonna be executed, no matter what happens
            .catch(console.error)
    }
    return(
        <h1>I'm not important<h1>
    )
}

Versions

System:
OS: Linux 5.4 Ubuntu 20.04 LTS (Focal Fossa)
Binaries:
Node: 12.18.1 - /usr/local/lib/nodejs/bin/node
npm: 6.14.5 - /usr/local/lib/nodejs/bin/npm
Browsers:
Chrome: 84.0.4147.125
Firefox: 79.0
npmPackages:
@apollo/client: ^3.1.3 => 3.1.3

[tyagow]

When I migrate to Apollo 3.0 I had to use onCompleted:

MyComponent() {
    const [executeQuery, { data, error }] = useMutation(MUTATION, {
      onCompleted: () => { // I run after mutation is completed}
    });
    if (error) console.error(error)
    function onSendData() {
        executeQuery({variables: {myData: undefined}})
    }
    return(
        <h1>I'm not important<h1>
    )
}

[oscmedgon]

Yes, I'm familiar with this approach, but in the specific case I'm working on I have several mutations running in parallel and managing that would be dirty and complicated. The promise approach fits my needs without adding extra layers to control which request has succeeded and which ones have failed.

[oscmedgon]

I managed to work around the problem with my own mutation wrapper

export function useGraphqlMutation({ query, callbacks, options = {} }) {
    const [error, setError] = useState(null);
    const [loading, setLoading] = useState(false);
    const [promise, setPromise] = useState(null);
    useDebugValue(`loading: ${loading}`);


    function getResponse(res) {
        if (callbacks?.success && typeof callbacks.success === 'function') {
            callbacks.success(res);
        }
        promise.resolve(res);
        setLoading(false);
    }

    function getError(err) {
        setError(err);
        setLoading(false);
        if (callbacks?.error && typeof callbacks.error === 'function') {
            callbacks.error(err);
        }
        promise.reject(err);
    }

    const [executeQuery, { data }] = useMutation(
        query,
        {
            onCompleted: getResponse,
            onError: getError,
            ...options,
        },
    );

    function run(executionData) {
        executeQuery(executionData);
        return new Promise((resolve, reject) => {
            setPromise({ resolve, reject });
        });
    }

    return {
        run,
        data,
        error,
        loading,
    };
}

Using this custom hook adds the expected behavior to the useMutation, I think that this should be fixed, because it doesn't have any sense to respond with a promise and always fulfill without considering the result of the mutation.

[tyagow]

I managed to work around the problem with my own mutation wrapper

export function useGraphqlMutation({ query, callbacks, options = {} }) {
    const [error, setError] = useState(null);
    const [loading, setLoading] = useState(false);
    const [promise, setPromise] = useState(null);
    useDebugValue(`loading: ${loading}`);


    function getResponse(res) {
        if (callbacks?.success && typeof callbacks.success === 'function') {
            callbacks.success(res);
        }
        promise.resolve(res);
        setLoading(false);
    }

    function getError(err) {
        setError(err);
        setLoading(false);
        if (callbacks?.error && typeof callbacks.error === 'function') {
            callbacks.error(err);
        }
        promise.reject(err);
    }

    const [executeQuery, { data }] = useMutation(
        query,
        {
            onCompleted: getResponse,
            onError: getError,
            ...options,
        },
    );

    function run(executionData) {
        executeQuery(executionData);
        return new Promise((resolve, reject) => {
            setPromise({ resolve, reject });
        });
    }

    return {
        run,
        data,
        error,
        loading,
    };
}

Using this custom hook adds the expected behavior to the useMutation, I think that this should be fixed, because it doesn't have any sense to respond with a promise and always fulfill without considering the result of the mutation.

Nice thanks for sharing.