Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: convert errors into ApolloError #1225

Merged
merged 2 commits into from
Jul 19, 2021
Merged

feat: convert errors into ApolloError #1225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
031563a
chore: use local typescript version in vscode
backbone87 Jul 18, 2021
fb7fff6
refactor(composable): convert errors into ApolloError
backbone87 Jul 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"javascriptreact",
"vue"
],
"eslint.enable": true
"eslint.enable": true,
"typescript.tsdk": "node_modules/typescript/lib"
}
18 changes: 10 additions & 8 deletions packages/vue-apollo-composable/src/useMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { DocumentNode } from 'graphql'
import { MutationOptions, OperationVariables, FetchResult, TypedDocumentNode } from '@apollo/client/core'
import { MutationOptions, OperationVariables, FetchResult, TypedDocumentNode, ApolloError } from '@apollo/client/core'
import { ref, onBeforeUnmount, isRef, Ref, getCurrentInstance } from 'vue-demi'
import { useApolloClient } from './useApolloClient'
import { ReactiveFunction } from './util/ReactiveFunction'
import { useEventHook } from './util/useEventHook'
import { trackMutation } from './util/loadingTracking'
import { toApolloError } from './util/toApolloError'

/**
* `useMutation` options for mutations that don't require `variables`.
Expand All @@ -27,12 +28,12 @@ export type MutateFunction<TResult, TVariables> = (variables?: TVariables | null
export interface UseMutationReturn<TResult, TVariables> {
mutate: MutateFunction<TResult, TVariables>
loading: Ref<boolean>
error: Ref<Error | null>
error: Ref<ApolloError | null>
called: Ref<boolean>
onDone: (fn: (param: FetchResult<TResult, Record<string, any>, Record<string, any>>) => void) => {
off: () => void
}
onError: (fn: (param: Error) => void) => {
onError: (fn: (param: ApolloError) => void) => {
off: () => void
}
}
Expand All @@ -47,11 +48,11 @@ export function useMutation<
const vm = getCurrentInstance()
const loading = ref<boolean>(false)
vm && trackMutation(loading)
const error = ref<Error | null>(null)
const error = ref<ApolloError | null>(null)
const called = ref<boolean>(false)

const doneEvent = useEventHook<FetchResult<TResult, Record<string, any>, Record<string, any>>>()
const errorEvent = useEventHook<Error>()
const errorEvent = useEventHook<ApolloError>()

// Apollo Client
const { resolveClient } = useApolloClient()
Expand Down Expand Up @@ -94,11 +95,12 @@ export function useMutation<
doneEvent.trigger(result)
return result
} catch (e) {
error.value = e
const apolloError = toApolloError(e)
error.value = apolloError
loading.value = false
errorEvent.trigger(e)
errorEvent.trigger(apolloError)
if (currentOptions.throws === 'always' || (currentOptions.throws !== 'never' && !errorEvent.getCount())) {
throw e
throw apolloError
}
}
return null
Expand Down
44 changes: 22 additions & 22 deletions packages/vue-apollo-composable/src/useQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
FetchMoreOptions,
ObservableSubscription,
TypedDocumentNode,
ApolloError,
} from '@apollo/client/core'
import { throttle, debounce } from 'throttle-debounce'
import { useApolloClient } from './useApolloClient'
Expand All @@ -28,6 +29,7 @@ import { paramToRef } from './util/paramToRef'
import { paramToReactive } from './util/paramToReactive'
import { useEventHook } from './util/useEventHook'
import { trackQuery } from './util/loadingTracking'
import { toApolloError } from './util/toApolloError'

import type { CurrentInstance } from './util/types'

Expand Down Expand Up @@ -58,7 +60,7 @@ export interface UseQueryReturn<TResult, TVariables> {
result: Ref<TResult | undefined>
loading: Ref<boolean>
networkStatus: Ref<number | undefined>
error: Ref<Error | null>
error: Ref<ApolloError | null>
start: () => void
stop: () => void
restart: () => void
Expand All @@ -73,7 +75,7 @@ export interface UseQueryReturn<TResult, TVariables> {
onResult: (fn: (param: ApolloQueryResult<TResult>) => void) => {
off: () => void
}
onError: (fn: (param: Error) => void) => {
onError: (fn: (param: ApolloError) => void) => {
off: () => void
}
}
Expand Down Expand Up @@ -152,8 +154,8 @@ export function useQueryImpl<
*/
const result = ref<TResult | undefined>()
const resultEvent = useEventHook<ApolloQueryResult<TResult>>()
const error = ref<Error | null>(null)
const errorEvent = useEventHook<Error>()
const error = ref<ApolloError | null>(null)
const errorEvent = useEventHook<ApolloError>()

// Loading

Expand All @@ -166,7 +168,7 @@ export function useQueryImpl<

// SSR
let firstResolve: (() => void) | undefined
let firstReject: ((error: Error) => void) | undefined
let firstReject: ((apolloError: ApolloError) => void) | undefined
onServerPrefetch?.(() => {
if (!isEnabled.value || (isServer && currentOptions.value?.prefetch === false)) return

Expand All @@ -176,8 +178,8 @@ export function useQueryImpl<
firstResolve = undefined
firstReject = undefined
}
firstReject = (error: Error) => {
reject(error)
firstReject = (apolloError: ApolloError) => {
reject(apolloError)
firstResolve = undefined
firstReject = undefined
}
Expand Down Expand Up @@ -256,15 +258,10 @@ export function useQueryImpl<

processNextResult(queryResult)

// Result errors
// This is set when `errorPolicy` is `all`
if (queryResult.errors?.length) {
const e = new Error(`GraphQL error: ${queryResult.errors.map(e => e.message).join(' | ')}`)
Object.assign(e, {
graphQLErrors: queryResult.errors,
networkError: null,
})
processError(e)
// ApolloQueryResult.error may be set at the same time as we get a result
// when `errorPolicy` is `all`
if (queryResult.error !== undefined) {
Copy link

@zorji zorji Jul 30, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@backbone87 @Akryum

Is this change correct? I've tried the latest stable @apollo/client@3.3 and the next version 3.4.1.

Both of them don't have have .error set, they only have .errors set.

This means error is never going to trigger onError.

Here is my query options

    defaultOptions: {
      watchQuery: {
        fetchPolicy: 'no-cache',
        errorPolicy: 'all',
      },
      query: {
        fetchPolicy: 'no-cache',
        errorPolicy: 'all',
      },
      mutate: {
        errorPolicy: 'all',
      },
    },

In this case I have a partial error, i.e. both data and errors are returned. The old behaviour is it triggers onError but it no longer triggers

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested with @apollo/client@3.4.1

.error is set when errorPolicy=none and it is not set when errorPolicy=all

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the relevant PRs to the error field
apollographql/apollo-client@513d733#diff-80ebcd3dc48e85c12b2c8484c5355662ab98ceea358660eb3af536c584f6fa44R22

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my assumption was that ApolloQueryResult.error contains an ApolloError instance that already wraps ApolloQueryResult.errors, seems like this is not the case... so we need to create the ApolloError for ApolloQueryResult.errors outselves.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@backbone87 I have the similar assumption but I can't find anything in official clearly says should onError be triggered or not when errorPolicy=all. Looks like this might be an unclear behaviour on apollo/client itself.

I can see different PRs, some assume onError should be triggered and some assume it should not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the end vue-apollo wraps all this stuff anyway, so it can define its behavior (yay more convolution :))

processError(queryResult.error)
} else {
if (firstResolve) {
firstResolve()
Expand All @@ -280,22 +277,25 @@ export function useQueryImpl<
resultEvent.trigger(queryResult)
}

function onError (queryError: any) {
function onError (queryError: unknown) {
// any error should already be an ApolloError, but we make sure
const apolloError = toApolloError(queryError)

processNextResult((query.value as ObservableQuery<TResult, TVariables>).getCurrentResult())
processError(queryError)
processError(apolloError)
if (firstReject) {
firstReject(queryError)
firstReject(apolloError)
stop()
}
// The observable closes the sub if an error occurs
resubscribeToQuery()
}

function processError (queryError: any) {
error.value = queryError
function processError (apolloError: ApolloError) {
error.value = apolloError
loading.value = false
networkStatus.value = 8
errorEvent.trigger(queryError)
errorEvent.trigger(apolloError)
}

function resubscribeToQuery () {
Expand Down
18 changes: 11 additions & 7 deletions packages/vue-apollo-composable/src/useSubscription.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
Observable,
ObservableSubscription,
TypedDocumentNode,
ApolloError,
} from '@apollo/client/core'
import { throttle, debounce } from 'throttle-debounce'
import { ReactiveFunction } from './util/ReactiveFunction'
Expand All @@ -26,6 +27,7 @@ import { useEventHook } from './util/useEventHook'
import { trackSubscription } from './util/loadingTracking'

import type { CurrentInstance } from './util/types'
import { toApolloError } from './util/toApolloError'

export interface UseSubscriptionOptions <
// eslint-disable-next-line @typescript-eslint/no-unused-vars
Expand All @@ -45,7 +47,7 @@ type OptionsParameter<TResult, TVariables> = UseSubscriptionOptions<TResult, TVa
export interface UseSubscriptionReturn<TResult, TVariables> {
result: Ref<TResult | null | undefined>
loading: Ref<boolean>
error: Ref<Error | null>
error: Ref<ApolloError | null>
start: () => void
stop: () => void
restart: () => void
Expand All @@ -56,7 +58,7 @@ export interface UseSubscriptionReturn<TResult, TVariables> {
onResult: (fn: (param: FetchResult<TResult, Record<string, any>, Record<string, any>>) => void) => {
off: () => void
}
onError: (fn: (param: Error) => void) => {
onError: (fn: (param: ApolloError) => void) => {
off: () => void
}
}
Expand Down Expand Up @@ -119,8 +121,8 @@ export function useSubscription <

const result = ref<TResult | null | undefined>()
const resultEvent = useEventHook<FetchResult<TResult>>()
const error = ref<Error | null>(null)
const errorEvent = useEventHook<Error>()
const error = ref<ApolloError | null>(null)
const errorEvent = useEventHook<ApolloError>()

const loading = ref(false)
vm && trackSubscription(loading)
Expand Down Expand Up @@ -157,10 +159,12 @@ export function useSubscription <
resultEvent.trigger(fetchResult)
}

function onError (fetchError: any) {
error.value = fetchError
function onError (fetchError: unknown) {
const apolloError = toApolloError(fetchError)

error.value = apolloError
loading.value = false
errorEvent.trigger(fetchError)
errorEvent.trigger(apolloError)
}

function stop () {
Expand Down
16 changes: 16 additions & 0 deletions packages/vue-apollo-composable/src/util/toApolloError.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { ApolloError, isApolloError } from '@apollo/client'

export function toApolloError (error: unknown): ApolloError {
if (!(error instanceof Error)) {
return new ApolloError({
networkError: Object.assign(new Error(), { originalError: error }),
errorMessage: String(error),
})
}

if (isApolloError(error)) {
return error
}

return new ApolloError({ networkError: error, errorMessage: error.message })
}