Query with errorPolicy "all" cannot effectively use onError, onCompleted

[tdeedlesATX]

Hi 👋 ! A team of us are working on transitioning a production application to Apollo with use of hooks in React. We have the following use case:

• use a query with errorPolicy set to "all" (so that error and data may both have value)
• use onError, onCompleted attributes to perform some side effects

import {useQuery} from '@apollo/react-hooks`
import gql from 'graphql-tag'

const someSideEffect = (error, data) => {
   if (!data) console.error(error)
   else if (data && error) { 
      console.info(error)
      alert(`some of the data is shiny: ${data.isShiny}`)
   } else {
      console.log('no error 🙌')
   }
}

const QUERY = gql `
  query get_stuff {
     stuff {
       isShiny
       someFinnickyFieldThatErrorsOutSometimes
     }
  }
`

function Stuff() {
  const {data, loading} = useQuery(QUERY, {
     errorPolicy: 'all',
     onCompleted: (data) => someSideEffect(null, data),
     onError: (error) => someSideEffect(error),
  });

  if (loading) return 'loading...'
  if (!loading && data) return <div>{JSON.stringify(data, null, 4)}</div>;
}

Intended outcome:

I'd expect to be able to perform "completed" and "errored" functionality (one possibility: access data in my onError callback if the errorPolicy="all"), since this would be a congruent outcome of the data, error, loading pattern commonly used when both data and error may be populated.

Actual outcome:

If there is a partial error from one of the fields, only onError is ever called.
I think issue can be found in this either/or method (here in react-apollo, or here in apollographql/apollo-client). I'd wonder if onError could receive data as a 2nd ordinal argument in the event that there is both an error and data ?

How to reproduce the issue:

I've reproduced here: https://codesandbox.io/s/inspiring-surf-pcrgj?file=/src/App.js

Possible Solution

Possible solution 1: provide data as a 2nd ordinal argument to onError

  const {data, loading} = useQuery(QUERY, {
     errorPolicy: 'all',
     onCompleted: (data) => someSideEffect(null, data),
     // Perhaps It'd be SO nice if onError had access to data ❤, but it does not 💔
     onError: (error, data) => someSideEffect(error, data),
  });

One might also argue additionally (or alternatively) that error in the onCompleted callback should be made available. Hence, in the case of errorPolicy="all" this dichotomy between "completed" and "errored" is somewhat difficult to discern. Hence:

Possible Solution 2: Perhaps if developer does not define onError and errorPolicy is all, then onCompleted is always called with both data and error?

Possible Solution 3: Alternatively, and more generally it should just always call "onCompleted" with access to both error and data and we could remove the notion of onError entirely?**

Many solutions to this perceived bug. These three solution proposals above are ranked in order from least to most invasive.

Version

@apollo/[email protected]

  System:
    OS: macOS Mojave 10.14.6
  Binaries:
    Node: 12.16.1 - ~/.volta/tools/image/node/12.16.1/6.13.4/bin/node
    npm: 6.13.4 - ~/.volta/tools/image/node/12.16.1/6.13.4/bin/npm
  Browsers:
    Chrome: 83.0.4103.116
    Edge: 83.0.478.61
    Firefox: 76.0
    Safari: 13.0.4

[tdeedlesATX]

While I see this as a bug, I'd understand if the team would want to categorize as a feature request. Please advise whether you'd need me to file in feature requests: https://github.com/apollographql/apollo-feature-requests