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

Error is not retrieved from the cache #4138

Closed
Zdend opened this issue Nov 14, 2018 · 38 comments
Closed

Error is not retrieved from the cache #4138

Zdend opened this issue Nov 14, 2018 · 38 comments

Comments

@Zdend
Copy link

Zdend commented Nov 14, 2018

I have been trying to implement a standard UI where a particular resource can be found on a path like this /user/:id.
If I try to access a user that I don't have permissions to I expect server to return an error and a user node to be null.
However if I try to access the same resource again, user will be retrieved from the cache even though it's null but error field is undefined - that prevents me from reacting to the data correctly as I don't know on the second pass whether null means an error and what kind of error since error field is not present. The first request gives me correctly user and error.

Is it possible to setup cache policy to either not store null resources when errors are present or make sure that errors are cached as well?

My Setup:

const cache = new InMemoryCache();
const defaultOptions = {
    watchQuery: {
        fetchPolicy: 'cache-first',
        errorPolicy: 'all'
    },
    query: {
        fetchPolicy: 'cache-first',
        errorPolicy: 'all'
    },
    mutate: {
        errorPolicy: 'all'
    }
};
const client = new ApolloClient({
    shouldBatch: true,
    link: ApolloLink.from([new HttpLink({ uri: GRAPHQL_ENDPOINT })]),
    cache,
    defaultOptions
});

Thank you!

@maxlew
Copy link

maxlew commented Nov 21, 2018

Facing the same issue currently, we're using graphql-shield which throws an error when a permission check isn't met. Apollo caches the result as null but doesn't cache the error.

In the docs it mentions that errors should be cached, so I'm guessing this used to work.
"it saves both data and errors into the Apollo Cache so your UI can use them."

Also it looks like deafultOptions aren't respected anyway #2555, #3256, however setting errorPolicy: 'all' on the query works, just a pain to set it on every query that may throw an error.

You can make this work by using the following, however it's just skipping the cache and ignoring the issue entirely.

errorPolicy: 'all',
fetchPolicy: 'no-cache',

@pafflique
Copy link

We have the same issue. Error is undefined on second attempt - component tries to render null data and crashes. Used fetchPolicy: 'no-cache' workaround for now.

@mikebm
Copy link

mikebm commented Jan 22, 2019

Same issue. We have a query that we don't want to throw an exception during a server side render. Setting the errorPolicy to 'all' solves this, but the error is never sent to the client's Query component. Setting the fetchPolicy to no-cache does work, but it also causes the query to be re-issued.

@ivtpz
Copy link

ivtpz commented Feb 15, 2019

Same issue here. We are attempting to let users refetch on a per query basis when there are errors. Second time the query runs (if pulling from the cache) no errors are shown and we just get the cached data.

@joepuzzo
Copy link

Any updates on this? The workaround obviously defeats the purpose of caching.

@mikebm
Copy link

mikebm commented Mar 25, 2019

Any updates on this? The workaround obviously defeats the purpose of caching.

@joepuzzo I have fixed this in react-apollo. See change:
apollographql/react-apollo@9d2eac9#diff-f8938b597e7a7647b9ea5589d55d3a26

You can use 'errorPolicy' of 'all' now, and the Query component will be issued the error during a SSR. This, of course, assumes you are working with Query component and does not affect the usage of client.query.

@joepuzzo
Copy link

Ok awesome! So it looks like i just need to pull latest release then

@joepuzzo
Copy link

Hmm just tried it and im using error policy all. Still not loading the error from cache

@mikebm
Copy link

mikebm commented Mar 25, 2019

Hmm just tried it and im using error policy all. Still not loading the error from cache

Are you using apollo-react's Query component? It only works with that. If using client.query, then it won't work. I updated my earlier response with additional requirements around it. Query component is the only work around. Considering the re-design efforts involved in fixing it in the client cache, I personally wouldn't expect it to be fixed anytime soon or at all, but that's just my 2 cents.

@joepuzzo
Copy link

Yup im using import { Query } from 'react-apollo'; with <Query errorPolicy="all"

@joepuzzo
Copy link

And also NOOO thats really unfortunate because i started moving away from render props and the Query component completely and use a useQuery hook instead 😢

@joepuzzo
Copy link

But regardless strange that im not seeing the fix when using the Query comp 🤔

@mikebm
Copy link

mikebm commented Mar 25, 2019

Hmm just tried it and im using error policy all. Still not loading the error from cache

Ensure your errorPolicy is null. Then during the server side render, you should see the error prop be populated with your error, rather than it throwing an exception. Depending on your fetch-policy, you may need to set something like ssrForceFetchDelay: 1000 on your ApolloClient as well to prevent it from re-issueing the request on the client during hydration (catch-and-network policy for example).

@joepuzzo
Copy link

Im a little confused. Should i set my error policy to "all" or null?

@joepuzzo
Copy link

Currently my error policy is "all" which is what i need / would expect. Whats happening is the first request gets the error and data: null when i hit back and then go back to the page it simply pulls data: null from the cache as the original issue describes.

@mikebm
Copy link

mikebm commented Mar 25, 2019

Currently my error policy is "all" which is what i need / would expect. Whats happening is the first request gets the error and data: null when i hit back and then go back to the page it simply pulls data: null from the cache as the original issue describes.

That would be expected behavior depending on how your schema is setup. When you have errorPolicy set to all, it will try and return any usable response data that matches your schema response type. In this case, your schema type is nullable, so it caches null because it errored in a way which prevented it from converting any data to match the schema definition. If you do not want null to be returned/cached then you must make your response non-nullable.

@joepuzzo
Copy link

Problem is that if I were to make the response non nullable then I cant get errors and data returned cleanly. The status code returned turns into a 500 by express-graphql and then apollo will treat it differently due to the non 200 status code.

@joepuzzo
Copy link

To elaborate, the reason i choose to put nullable field on the schema is because when i make it non nullable the server will return 500 status code. When this happens, apollo client will say "Oh no a non 200 status code, well i better ignore the info that came back in the response and just let the user know there was a network error". Unless this is another bug in apollo client idk what to do.

@joepuzzo
Copy link

joepuzzo commented Mar 26, 2019

Looks like there has been issue about this ( non nullable vs nullable field ) for over a year #3034 ...

@joepuzzo
Copy link

So it looks like this was an issue and closed... very confused as to why it was closed as no clear solution was found for the original issue. apollographql/react-apollo#1781

@joepuzzo
Copy link

joepuzzo commented Mar 27, 2019

@ghost
Copy link

ghost commented Jun 13, 2019

Is there any progress on this? From my understanding of the docs:

{
  error: {
    graphqlErrors: {
      path: 'foo',
      ...
    }
  },
  data: {
    foo: null,
    bar: 123,
  }
}

should continue to pull that error from the cache and return it to the child renderer as long as errorPolicy: 'all' is set. In practice the error object is dropped entirely whenever this data is read from the cache. If data.foo becomes non-nullable this error becomes fatal on the server, which is not the desired behavior.

@Thomas-Smyth
Copy link

It would be nice for this issue to be looked into or for the docs to be updated to remove the false claims of what this repository is able to achieve.

I understand this is a bug and should be resolved rather than being removed from the documentation, but this has not been looked into for 8 months and the documentation has not been amended so people are evidently getting confused over it.

I just wasted several days work because I assumed this would work. Now I have had to delete much of my recent work and build my own custom store as a hacky way of storing errors until this is fixed.

@maxchehab
Copy link

@Thomas-Smyth Can you OS this custom store?

@Thomas-Smyth
Copy link

@Thomas-Smyth Can you OS this custom store?

I ended up going around it a different way in the end.

@okikejiani
Copy link

@Thomas-Smyth Can you OS this custom store?

I ended up going around it a different way in the end.

@Thomas-Smyth what did you do?

@Thomas-Smyth
Copy link

Thomas-Smyth commented Aug 1, 2019

@okikejiani I had planned to use error to return "You do not have permission to view ...". I would then get info from the returned error to find what aspect of the UI could not be rendered and then replace it with a "No permission" message.

When reloading the page the cache would get the data, but not the error so I would not know what to replace with a "No permission" message.

I got around it by just querying my API to see what they have permission to view as a separate part of the query and then using that to determine what parts of the UI should be replaced with a "No Permission" message.

@dulakm
Copy link

dulakm commented Sep 10, 2019

Any updates on this? The problem seems quite major to me.

@balnagy
Copy link

balnagy commented Sep 18, 2019

Hi, we are still facing similar. We had a workaround to refetch programmatically when we suspect an error occurred. This workaround stopped working when upgraded from ^2.5.5 to ^3.1.0. However, an additional consecutive refetch (refetch().then(refetch)) solved the problem, but it's quite annoying.

Is there any ongoing effort to fix this problem?

@3nvi
Copy link

3nvi commented Sep 27, 2019

Even if this isn't resolved, is it possible to get an answer from the team? I mean, even recommendations on workarounds (apart from no-cache) will do just fine.

Steps to reproduce in a React environment:

  1. Create a gql query that results into an error (for example, by explicitly returning an error from your server). Let's name it MY_GQL_QUERY.

  2. Create a basic React app that toggles between two components while using the query defined above:

import React from 'react;

const Component = ({ toggleComponent }) => {
const { loading, error } = useQuery(MY_GQL_QUERY, { fetchPolicy: 'cache-first' });

 return (
    <div>
        {loading && 'Loading query...'}
        {error && error.graphQLErrors[0].message}
       <button onClick={toggleComponent}>Render the other component</button>
    </div>
 )
}

/* This is the main app */
const App = () => {
  const [isFirstComponentShown, showFirstComponent] = React.useState(true);
  const toggleComponent = () => showFirstComponent(!isFirstComponentShown);
 
  // renders the same component under a different key to emulate 2 different components
  return isFirstComponentShown ? (
     <Component key="first" toggleComponent={toggleComponent} />
   ) :  (
     <Component key="second" toggleComponent={toggleComponent} />
   )
}
  1. Initially you will see loading... followed by the error that you have created.

  2. Navigate to the other component (its clone) by clicking on the button

  3. Instead of the seeing the error, you should have a JS exception. This is because the error key is not properly persisted in the cache and resets to undefined instead of keeping the value that received from the server.

@ankitjaincst
Copy link

I am using useQuery . I set errorPolicy: "all" . I have 2 queries in a query . Eg .
const GET_MULTI_QUERY = gqlquery GetMulti { ldapGroups(email: "myemail") listQueues(request: {}) { queues { uuid version name } } };

Now if one of them errors out , I expect to get data for the other one as well as get the error with what has been said in the documentation . However, I am not getting the error . I get error as undefined . I tried errorPolicy: 'none' , this sets data to null and only gives the error . Also, errorPolicy: 'ignore' does not work . I am using SSR but that should not matter .

This is a long thread and I guess the expectation is get the error as well as the data possible .

Any progress or anyone working on this ? This looks like a very basic requirement as the playgroung even works like that .

1 similar comment
@ankitjaincst
Copy link

I am using useQuery . I set errorPolicy: "all" . I have 2 queries in a query . Eg .
const GET_MULTI_QUERY = gqlquery GetMulti { ldapGroups(email: "myemail") listQueues(request: {}) { queues { uuid version name } } };

Now if one of them errors out , I expect to get data for the other one as well as get the error with what has been said in the documentation . However, I am not getting the error . I get error as undefined . I tried errorPolicy: 'none' , this sets data to null and only gives the error . Also, errorPolicy: 'ignore' does not work . I am using SSR but that should not matter .

This is a long thread and I guess the expectation is get the error as well as the data possible .

Any progress or anyone working on this ? This looks like a very basic requirement as the playgroung even works like that .

@PinkyJie
Copy link

I got the same issue, even I set errorPolicy: 'all', data.error has error for the first time, but after I went to another page and came back, the data.error became undefined. According to the doc, error should be cached in apollo cache, but it didn't.

@0xMarkian
Copy link

+1, reproducible on apollo@3
any updates on this?

@nelsonBlack
Copy link

in the default options delete mutate: { errorPolicy: 'all' } worked for me , have it as apollo.create({ link, cache: new InMemoryCache(), defaultOptions: { watchQuery: { fetchPolicy: "cache-and-network", errorPolicy: "ignore", }, query: { fetchPolicy: "network-only", errorPolicy: "all", }, }, });

@Melzmr
Copy link

Melzmr commented Jul 28, 2020

errorPolicy: 'all',
Doesn't work at all. No error info saved to the Cache :(

@samueljun
Copy link

Here are some relevant findings and repro steps proving that apollo's own two recommended methods for SSR getDataFromTree() vs renderToStringWithData() produce different results because of the lack of error caching #3897 (comment).

@hwillson
Copy link
Member

The docs were updated to remove the references that mentioned errors were saved in the cache. This is still something we want to do, but it isn't supported yet.

@apollographql apollographql locked and limited conversation to collaborators May 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests