SmartQuery error() handler gets called twice even though query was retrieved once from the network

[fabis94]

Describe the bug
For some reason error() handler of my query gets called twice, once as an Error and the 2nd time as a GraphQLError. The query results definitely aren't in a cache or anything, so why does this happen?

To Reproduce
Nothing special, the query is registered like this:

apollo: {
        failingQuery: {
            query: failingQuery,
            error(err) {
                alert(err.message);
            }
        }
    },

And the alert is invoked twice.

Expected behavior
The handler should be called once, why would it be called twice?

Versions
vue: 2.6.11
@vue/apollo-option: 4.0.0-alpha.11 and also 3.x
apollo-client: 3.3.6

Additional context
No special setup is done here, it's a basic query that hasn't been cached

[fabis94]

To me it looks like the issue is in smart-query.js, specifically the catchError method.

Whenever an error arrives here, it gets reported with super.catchError, but then the process continues on and calls this.nextResult. Inside the definition of this.nextResult() however, the error is reported a 2nd time, except this time it gets wrapped in a generic Error making it lose a lot of information (why is this necessary?).

Seems like a bug to me, I don't see why the error reporting part in this.nextResult() even needs to be there. Is it in any way possible for nextResult() to be invoked without catchError() also being invoked? Only that would explain why error handling code is duplicated there.

[Narretz]

Hi @fabis94 did you find a way to fix this issue? I'm also encountering this after I updated to "@apollo/client": "^3.3.11" from 2.x (and using vue-apollo 3.0.7) @Akryum do you have any thoughts on this? This just seems broken.

[fabis94]

Nope, I'm considering creating a fork since this project doesn't seem to be actively maintained

[Narretz]

That would be the nuclear option ... a big problem is also that there don't seem to be a lot of tests, and many changes are committed without automatic tests. That's a nightmare scenario for such a popular library, because you might break something without knowing it.

[Narretz]

I can confirm that this block seems to be the problem:

https://github.com/vuejs/vue-apollo/blob/92a4a11e2627d39981f2c9ec1e46e1fc9e6dcb09/packages/vue-apollo/src/smart-query.js#L172-L184

Commenting it out solves the double error problem with errorPolicy none

This problem also affects errorPolicy: 'all', because with the above block, it'll call both error and result, while the docs and code comments state that only result should be called (with data and an errors array). ignore seems to work with or without

Since this doesn't happen with Apollo 2.x, there might be an internal change there that caused this. Would make it more difficult to remove this block.
Come to think about it, when I was on Apollo Client 2.x, I received the Errors prefixed with GraphQL from this function. So something might have changed that we now get the actual error.

[Narretz]

@Akryum can you please take a look at this? This is a serious issue.

[fabis94]

I ended up creating a fork that solves some of the most obvious issues I encountered, the double error reporting and inconsistent result reporting in the ApolloQuery component:

v3...fabis94:patch-1 (this is for vue apollo 3.x)

If the author can comment on these and tell me what I should be doing differently, I can create a formal PR from this and fix these issues

[flavioleggio]

Any news on this one? I ran in the same issue

[nicolas-anaxago]

same here :(

[Akryum]

Commenting it out solves the double error problem with errorPolicy none

The code you are commenting is necessary to retrieve the errors if errorPolicy is set to all, as validated by this test:

After commenting the code:

The actual fix I came up with is to ignore the errors from the response if Apollo Client is handling the error already:

After updating the test:

That would be the nuclear option ... a big problem is also that there don't seem to be a lot of tests, and many changes are committed without automatic tests. That's a nightmare scenario for such a popular library, because you might break something without knowing it.

Thanks for the "there are no tests" comment. BTW you can contribute and send PRs with more tests 😉

[Narretz]

Hi @Akryum thanks for the fix. Is it possible to backport this to the v3 branch? Or is v4 around the corner?