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

useMutation onError handler is not fired if error policy is set to "all" #10973

Closed
Imran99 opened this issue Jun 13, 2023 · 19 comments · Fixed by #11103
Closed

useMutation onError handler is not fired if error policy is set to "all" #10973

Imran99 opened this issue Jun 13, 2023 · 19 comments · Fixed by #11103
Labels

Comments

@Imran99
Copy link

Imran99 commented Jun 13, 2023

Issue Description

Intended Outcome

From the docs the onError policy is called unless the errorPolicy is set to ignore.

Actual Outcome

useMutation onError handler is not called when an error occurs on the graphql server and the error policy is set to all.

Versions

"@apollo/client": "^3.7.15"

Link to Reproduction

https://codesandbox.io/s/thirsty-darkness-crw7f8?file=/src/index.jsx

Reproduction Steps

  1. Invoke a mutation with an onError handler and errorPolicy: "all"
  2. When an error occurs the onError handler will not be called.

The code sandbox link reproduces the issue.

@jerelmiller jerelmiller added 🏓 awaiting-team-response requires input from the apollo team 🐞 bug labels Jun 14, 2023
@CH-PatrickWalmsley
Copy link

+1

Heads up to others - onComplete does not adhere to the behavior described in the docs either. 🤕

As a potential workaround you can use update on place of onError and onComplete. First param is a proxy to the cache. Second param is the raw response with both data and errors. Unlike onError and onComplete, update will not run after the component scope variables are updated so you must use the response contents passed to the method.

@bignimbus
Copy link
Contributor

Hi @Imran99 👋🏻 I'm looking at the reproduction and don't see where the error is being thrown that would trigger onError. When I add a rejected promise to the link in this fork, I see onError was called in the console upon executing the mutation. Apologies if I'm missing something obvious!

@CH-PatrickWalmsley let me know more about what you're seeing, do you have something runnable to take a look at? We do have a suite of unit tests that validate callback behavior, though I admit that sometimes they don't behave according to folks' expectations and are considering changing our approach in an upcoming major version.

Eager to hear from either/both of you on how to figure things out from here 🙏🏻

@bignimbus bignimbus added 🏓 awaiting-contributor-response requires input from a contributor and removed 🏓 awaiting-team-response requires input from the apollo team labels Jun 29, 2023
@Imran99
Copy link
Author

Imran99 commented Jun 29, 2023

Apologies @bignimbus I think I may have had some unsaved changes in my fork. I've added the throw back in which reproduces the issue: https://codesandbox.io/s/thirsty-darkness-crw7f8?file=/src/index.jsx

Screen Shot 2023-06-29 at 17 36 32

@chawomack
Copy link

+1

I'm noticing that onError does not get executed and the error field remains undefined.

@bignimbus
Copy link
Contributor

bignimbus commented Jun 29, 2023

Ok thanks for that, I'll share this with the team and get some more context. From what I understand, not every Error in your application can/should be expected to be handled through Apollo Client. Network errors and spec-compliant GraphQL errors from the server will be reported, but not necessarily client-side runtime errors. I'd categorize your example, which throws a plain JavaScript error inside the resolver of the mutation type, as being a client-side runtime error. If such an error were thrown on the server it would either be caught and passed down in the error payload or reported as a 5xx network error. If you look at the fork I shared above, you'll notice the error is passed to onError without issue.

Can you share a little more about what you're seeing in the application you're working on? Where is the error being thrown?

@bignimbus bignimbus removed the 🏓 awaiting-contributor-response requires input from a contributor label Jun 29, 2023
@Imran99
Copy link
Author

Imran99 commented Jun 29, 2023

Thanks @bignimbus. At the moment the error handling behaviour seems to be a bit inconsistent. If you set errorPolicy to none, onError is called. If you set errorPolicy to all, onError isn't called. I was expecting any error in the error response collection to trigger onError as this made more sense when considered with client errorPolicy settings.

image

Can you share a little more about what you're seeing in the application you're working on? Where is the error being thrown?

My scenario is similar to the one in the sample application. I have some unexpected error happening in the resolver that I want to handle with a generic oops message on a particular screen that makes the request.

@bignimbus
Copy link
Contributor

I have some unexpected error happening in the resolver

Is this a resolver on the server, a local resolver, or using a graphql.js resolver directly?

@Imran99
Copy link
Author

Imran99 commented Jun 30, 2023

irl the resolver is on the server yup. The behaviour of onError is the same.

@bignimbus
Copy link
Contributor

bignimbus commented Jun 30, 2023

Ok thanks 🙏🏻 are you able to paste the JSON returned from the server? Feel free to remove any sensitive data, I'm more curious to hear whether the response has an "errors" property that conforms to the spec.

@Imran99
Copy link
Author

Imran99 commented Jun 30, 2023

Ok i'll see if I can dig that up. Note I'm using apollo server on the server side and that is handling the error formatting.

@Imran99
Copy link
Author

Imran99 commented Jun 30, 2023

Sample error:

  [
    {
      "errors": [
        {
          "message": "Request failed with status code 400",
          "locations": [
            {
              "line": 5,
              "column": 9
            }
          ],
          "path": [
            "orders",
            "nodes",
            0,
            "fulfillmentOrders",
            0,
            "quotes"
          ],
          "extensions": {
            "code": "INTERNAL_SERVER_ERROR",
            "stacktrace": [
              "AxiosError: Request failed with status code 400",
              "...obfuscated"
            ]
          }
        }
      ],
      "data": {
        "orders": {
          "nodes": [
            {
              "fulfillmentOrders": [
                {
                  "quotes": null,
                  "__typename": "FulfillmentOrder"
                }
              ],
              "__typename": "Order"
            }
          ],
          "__typename": "OrderConnection"
        }
      }
    }
  ]

@bignimbus
Copy link
Contributor

Thanks for this info, it's much appreciated and helps narrow things down. I'm going to see if I can reproduce this in the fork and will get back to you!

@bignimbus
Copy link
Contributor

Hmm, still not having luck with a reproduction. Here's an example that returns data and errors in a manner similar to what you're showing here, and I am able to see onError was called in the console. Here's the link, let me know if anything seems "off" with that codesandbox 🙏🏻

@bignimbus bignimbus added the 🏓 awaiting-contributor-response requires input from a contributor label Jun 30, 2023
@jerelmiller
Copy link
Member

@bignimbus looks like you're using an errorPolicy of none. I'm able to reproduce if you change errorPolicy to all. onCompleted is called instead in that case.

@bignimbus
Copy link
Contributor

Ah my mistake, thanks for catching that @jerelmiller

@github-actions github-actions bot removed the 🏓 awaiting-contributor-response requires input from a contributor label Jul 1, 2023
@milesingrams
Copy link

Dealing with the same. errorPolicy: all and inline onError is not called in useQuery.

@jerelmiller
Copy link
Member

This has been fixed in #11103 and will be released with 3.8.0 in the next couple weeks. Thanks for reporting this!

@Imran99
Copy link
Author

Imran99 commented Jul 27, 2023

Thanks @jerelmiller !

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants