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.

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

There was a undescribed "Breaking change" in useMutation hook #9065

Open
TriPSs opened this issue Nov 15, 2021 · 11 comments
Open

There was a undescribed "Breaking change" in useMutation hook #9065

TriPSs opened this issue Nov 15, 2021 · 11 comments

Comments

@TriPSs
Copy link

TriPSs commented Nov 15, 2021

Since this commit there was a "breaking change" in the useMutation hook.

Intended outcome:
When doing the following code:

const { errors } = await doMutation()

errors was filled if there was an error from the API

Actual outcome:
Error is now thrown instead of returned.

How to reproduce the issue:

// Make sure this mutation returns a error
const { errors } = await doMutation()

// Errors wont be called since 
console.log(errors)

Versions
Everything after 3.4.0

Tasks

Preview Give feedback
No tasks being tracked yet.
@TriPSs
Copy link
Author

TriPSs commented Nov 15, 2021

Also noticed that the following code does not work:

await doMutation({
  onError: (error) => console.log(error), // This is never called, errors keeps getting throwed
})

@brainkim brainkim self-assigned this Nov 16, 2021
@brainkim
Copy link
Contributor

The commit you’re referencing was only merged in 3.5, and was mainly a rote refactoring of the hook so I didn’t have to reason about code across 15 files, so I don’t think that’s the behavior change you’re looking for.

Might be related to this issue: #8793

@brainkim
Copy link
Contributor

Closing as a duplicate of #8793, let me know if you don’t think was right!

@brainkim brainkim removed their assignment Nov 17, 2021
@TriPSs
Copy link
Author

TriPSs commented Nov 18, 2021

@brainkim don't really think its a related to #8793 as that ticket wants to use the onError callbacks of the doMutation and I used const { errors } = doMutation and since the error is thrown inside unless you provide a onError it will never be returned.

I think either the interface needs to be updated or (my preference) the errors should always be returned.

@brainkim
Copy link
Contributor

brainkim commented Nov 18, 2021

@TriPSs
So at one point, the execute() function didn’t reject? It just returned the errors in an array? Sorry, I don’t think this behavior is documented or unit tested, and I only started working on Apollo Client during the 3.3 days.

@TriPSs
Copy link
Author

TriPSs commented Nov 18, 2021

Yea, if the line where it now throws the error would return it just like it does a couple of lines above it the behaviour would be how it was.

Currently we have it this was in a lot of places, so if this behaviour won't work anymore we would have to refractor all off those and add empty try catches to it otherwise our error reporting tool will send un needed messages.

@TriPSs
Copy link
Author

TriPSs commented Nov 18, 2021

I think I can enable the behaviour I want by setting the following on my Apollo client:

defaultOptions: {
    mutate: {
      errorPolicy: 'all'
    }
  }

@brainkim could you confirm this?

@jembach
Copy link

jembach commented Dec 9, 2021

Hi @TriPSs,

I'm having the same problem. It is in my opinion an unexpected behavior because when i can get an error returned i expect that it does not throw an error. In addition it isn't documented:

Currently my solution looks like this:

class ApolloCatchError implements ApolloQueryResult<undefined> {
  data = undefined;

  loading = false;

  networkStatus = NetworkStatus.error;

  errors = [];

  constructor(public error: ApolloError) {}
}

  async function fetchUser() {
    return gqlClient
      .query<QueryCurrentUserData, QueryCurrentUserVariables>({
        query: QUERY_CURRENT_USER,
        fetchPolicy: 'network-only',
      })
      .catch((error: ApolloError) => new ApolloCatchError(error));
  }

This is a typesafe solution providing the same result as @TriPSs and me expect

@buzzb0x
Copy link

buzzb0x commented Dec 21, 2022

Hi there, it's been months, and I see that this issue has been fixed on useLazyQuery but is still present on useMutation. Are you planning to fix it? In the meantime, I suggest updating the documentation to reflect this known issue.

Thanks for your work 🙆‍♂️

@lhguerra
Copy link

lhguerra commented Sep 20, 2023

TriPSs comment about changing the error policy worked for me, this should be documented! The error policy default value does not work as described in the docs!

@lhguerra
Copy link

image

The highlighted part does not work at all, at least not in React. The onError and onComplete handlers are not executed at all when GraphQL Errors or Network Errors happen. It only worked with the "all" value, but the description for the "none" value suggests I would have at least the onError callback fired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants