React-apollo: useMutation calls onCompleted on error

[achtan]

Hi,
Intended outcome:

const [changePasswordHandler, { loading, error }] = useMutation(changePasswordMutation, {
    onCompleted(...params) {
      log.debug('onCompleted', { params })
      props.onChange()
    },
    onError(...error) {
      log.debug('onCompleted', { error })
    },
  })

if mutation return errors field dont call onCompleted callback

Actual outcome:

onCompleted callback is called when mutation return errors

How to reproduce the issue:

Version
System:
OS: macOS 10.14.6
Binaries:
Node: 8.15.1 - /usr/local/bin/node
Yarn: 1.15.2 - /usr/local/bin/yarn
npm: 6.7.0 - /usr/local/bin/npm
Browsers:
Chrome: 76.0.3809.100
Firefox: 67.0.4
Safari: 12.1.2
npmPackages:
apollo-cache-inmemory: 1.6.3 => 1.6.3
apollo-cache-persist: 0.1.1 => 0.1.1
apollo-client: 2.6.4 => 2.6.4
apollo-link: 1.2.12 => 1.2.12
apollo-link-context: 1.0.18 => 1.0.18
apollo-link-http: 1.5.15 => 1.5.15
react-apollo: 3.0.0 => 3.0.0

[ben-m-j-taylor]

@achtan Looking at the docs this seems like the expected behaviour as these callbacks are intended as an alternative to using the loading and error properties returned from the hook. So you would want onCompleted to be called alongside onError in the event of any errors being returned so that any loading logic (show/hide spinner etc.) could be handled as it would be normally and then any errors can be handled separately in their respective callback.

From the docs:

We can destructure loading and error properties off the mutation result returned by useMutation, in order to track the state of our mutation in our UI. The useMutation hook also supports onCompleted and onError options in case you would like to provide callbacks instead.

[achtan]

I was under the impression that onCompleted is called only when no error occur...

so where do i put my redirect-after-mutation-success ? would be nice to have onSuccess callback...

ok so i'm closing this issue...

[ben-m-j-taylor]

@achtan Based on the type definition for onCompleted it looks like you can access the data property that is returned by the mutation.

onCompleted?: (data: TData) => void;

You could potentially use this to determine the success of the mutation e.g.

onCompleted = (data) => {
    // Check for the presence of a property on data that would indicate success
    if (data && data.someProperty) {
        // redirect-after-mutation-success logic
    }
}

[annazus]

The online docs must be updated. It is misleading - states that onCompleted is

A callback executed once your mutation _successfully_ completes

[drajk]

Could use something similar to this wrapper I wrote to expose a onSuccess() callback.

https://medium.com/@draj.dev/onsuccess-for-apollo-react-hooks-dbe679a9efdb

[cbishop-fv]

It appears that useMutation and useQuery do not behave the same in these cases. useQuery DOES call onError on and error where useMutation calls onCompleted not on Error. This seems odd. I'm also opening an issue around useMutation. The error prop is never updated on error. I believe this is due to the fact that onError is not called.

[rynbyjn]

It appears that useMutation and useQuery do not behave the same in these cases. useQuery DOES call onError on and error where useMutation calls onCompleted not on Error. This seems odd.

I am also seeing this happening, however the error prop for the mutation was being updated with the correct error. My solve for now is not to use onCompleted and onError and go back to the promise implementation:

mutation().then(({ data, errors }) => {
  if (errors && errors[0]) {
     // onError(errors[0])
  } else {
     // onCompleted(data)
  }
}).catch((e) => {
  // onError
})

I'm using apollo-client 2.6.8 in a react-native 0.62.2 project.

[xdumaine]

I started to use onCompleted but then found this issue and was concerned. I was, however, not able to reproduce the behavior described in the issue. In my testing, for both useQuery and useMutation, onError is called only on error and onCompleted is called only on success. For brevity: my experience is that the docs are correct, and I could not reproduce this issue.

Does anyone have an example that illustrates onCompleted being called on error? it seems that it may have been fixed at somepoint. I'm on [email protected] and [email protected]

The query case: https://github.com/apollographql/apollo-client/blob/379bbb024a0909714e949d76584a3b76067649cb/src/react/data/QueryData.ts#L444

The mutation case: https://github.com/apollographql/apollo-client/blob/379bbb024a0909714e949d76584a3b76067649cb/src/react/data/MutationData.ts#L69