Open oscmedgon opened 3 years ago
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>
)
}
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.
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.
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.
+1 to the problem above. The mutation promise should either reject
or resolve with an error in case of errors.
Version: apollo-client@3.3.12
If you have options where errorPolicy: 'all'
the promise will resolve with { errors }
. However, for some reason, any onError
option won't be called. I've also noticed that onCompleted
is always called even if errors occur, resulting in data
being undefined. This isn't very obvious from the docs https://www.apollographql.com/docs/tutorial/mutations/#persist-the-users-token-and-id
I was expecting that onCompleted
would just be the result (including errors
and data
) not entirely sure why though.
When errorPolicy
is not set or set to none
useMutation()[1].errors
is not populated with "network errors".await useMutation()[0]()
promise calls result in undefined
. This means when "network errors"
or "graphql errors" occur they force the return to be undefined
(they don't reject the promise).onError
is only called for "network errors"onCompleted
is only called when no "network errors" or "graphql errors" occurWhen errorPolicy
is set to all
useMutation()[1].errors
is populated with "network errors".await useMutation()[0]()
promise calls result with errors defined. This means when "network errors"
or "graphql errors" occur the return will include { errors }
(they don't reject the promise).onError
is never called for "network errors" or "graphql errors"
Instead of using onError
use the errors return from either useMutation()[1].errors
or if you
need more control (await useMutation()[0]()).errors
.onCompleted
is always called even when "network errors" or "graphql errors" occur.
This means that onCompleted
really means after we've completed the mutation do this not when
the mutation is successful.This seems like a pretty serious oversight. Has no one started addressing this yet?
Another temporary solution might be:
try {
const {data} = await someMutation({
variables: { ... }
})
if (data.errors?.length > 0) {
// some errors
} else {
// success
}
} catch (_e) {
// some error
}
Another temporary solution might be:
try { const {data} = await someMutation({ variables: { ... } }) if (data.errors?.length > 0) { // some errors } else { // success } } catch (_e) { // some error }
In my experience, the value returned by await someMutation(...)
is itself undefined
so it fails trying to dereference data
.
The latest version of apollo 3.5.6
, broke my brevious workarround, I found a new workaround, it's a bit simpler, but it's awful that the native promise is always resolved instead of rejecting it when there is an error.
export function useGraphqlMutation({ query, callbacks, options = {} }) {
const [error, setError] = useState(null);
const [loading, setLoading] = useState(false);
useDebugValue(`loading: ${loading}`);
function getResponse(res) {
if (callbacks?.success && typeof callbacks.success === 'function') {
callbacks.success(res);
}
setLoading(false);
}
function getError(err) {
setError(err);
setLoading(false);
if (callbacks?.error && typeof callbacks.error === 'function') {
callbacks.error(err);
}
}
const [executeQuery, { data }] = useMutation(
query,
{
onCompleted: getResponse,
onError: getError,
...options,
},
);
function run(executionData) {
return new Promise((resolve, reject) => {
executeQuery(executionData)
.then(({ data: responseData, errors }) => {
if (errors) {
reject(errors);
} else {
resolve(responseData);
}
});
});
}
return {
run,
data,
error,
loading,
};
}
useGraphqlMutation
Hi, I am starting with JS. Could you show me example with using of this code?
Did the company behind apollo fail or something? This feels like a fairly critical bug. You call a mutation, it hits a network error as a part of that mutation and instead of rejecting the promise it blows up????
Feels like a pretty major issue to just sit unfixed for so long, do they even work on this repo any more?
I'm coming across a similar issue, is this still being investigated?
Following the above suggestions by @oscmedgon and @Nate-Wilkins, I've come up with a wrapper that keeps the same shape as the original useMutation
hook.
Without Typings (see below for Typescript):
import {useCallback} from "react";
import {ApolloError, useMutation} from "@apollo/client";
export const useGqlMutation = (mutation, options = {}) => {
const { onCompleted, onError, ...otherOptions } = options || {};
const [executeQuery, mutationResult] = useMutation(mutation, {
...otherOptions,
errorPolicy: 'all' // NOTE: populate all errors
});
const executeQueryWrapper = useCallback(async (options) => {
const result = await executeQuery(options as any);
if (result?.errors?.length) {
if (onError) {
onError(new ApolloError({ graphQLErrors: result.errors }))
}
throw result.errors;
}
// only call `onCompleted` if there was no error
if (onCompleted) {
onCompleted(result?.data);
}
return result;
}, [executeQuery, onCompleted, onError]);
return [executeQueryWrapper, mutationResult];
}
This will catch both onError
scenarios and promise errors.
Example usage:
const MY_ACTION = gql`
mutation MyMutationAction {
....
}
`;
const TestComponent = () => {
const [myAction, {loading}] = useGqlMutation(MY_ACTION, {
onError: (errors) => console.log('onError', errors),
onCompleted: (data) => console.log('onCompleted', data),
});
const handleClick = () => {
myAction()
.then((data) => console.log('promise resolved', data))
.catch((errors) => console.log('promise rejected', errors));
}
return (
<button disabled={loading} onClick={handleClick}>Test Mutation</button>
);
}
I am a bit confuse from the conversation above. I am using Apollo client 3.7.0 and createHttpLink
call.
In a case very similar to the original question, the executeQuery
call (in my case) always rejects. And it doesn't matter what kind of error it is (it rejects also for resolver errors). When I set errorPolicy: 'all'
, the promise is resolved with errors
field populated. For me this is not expected behavior, as the errrorPolicy
should only specify how the errors are propagated into the result.
What I would expect:
executeQuery
should reject.executeQuery
should resolve with either errors
populated and data: undefined
(for none
error policy], or errors: undefined
and data
partially populated (for ignore
error policy), or both data
partially populated and errors
populated (for all
error policy).From my pov, the resulting code should be like this.
MyComponent() {
const [executeQuery, { data, errors }] = useMutation(MUTATION);
function onSendData() {
executeQuery({variables: {myData: undefined}})
.then(({errors, data}) => {
// handle resolver errors
// errors also populated as a result of call `useMutation(MUTATION)` above
})
.catch(error => {
// handle network and validation errors
// in general cases when server returns 400 status code
})
}
return(
<h1>I'm not important<h1>
)
}
The worst thing is that this is not specified in the documentation at all.
And from historic issues, the functionality changes back and forth every a while. If the executeQuery
function will reject for every error (including resolver error), I am fine with that (ok, less fine with that, but I can still live with it) but please specify the behavior in documentation!
That's interesting. I would also agree documenting the desired behavior and then programming it might be the way to go.
Personally I don't care for the errorPolicy
at all because it changes the underlying behavior of error handling which is bizarre to me. A consistent error handling workflow is more preferable than a configurable one and in the case of executeQuery
I think the catch should be called whenever any error is "thrown" that way its consistent and doesn't require the client to update error handling in two spots.
@prageeth looks good. Only thing I think you missed was handling the executeQuery
errors that might occur that aren't returned and instead are "thrown". Granted when errorPolicy
is all
that shouldn't happen but :shrug:
over 2 years old, and not a single response from any maintainer.
EDIT: In my case I had a link which didn't properly handle observer errors leading to the uncaught error. After the link was patched I could catch the errors again.
Still getting this nasty bug in @apollo/client@3.8.4 !!
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 aPromise.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.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