-
Notifications
You must be signed in to change notification settings - Fork 774
Query with errorPolicy "all" cannot effectively use onError, onCompleted #4038
Description
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
anddata
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/react-components@3.1.5
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