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

Network-only refetch after network error does not update networkStatus #1622

Closed
msmfsd opened this issue Apr 24, 2017 · 38 comments
Closed

Network-only refetch after network error does not update networkStatus #1622

msmfsd opened this issue Apr 24, 2017 · 38 comments
Assignees
Labels

Comments

@msmfsd
Copy link

msmfsd commented Apr 24, 2017

Versions

NOTE: this needs a seperate backend grpahql server to re-create so I have not created a react-apollo-error-template repo.

Intended outcome:
Refetching a query after network error should always update loading to true and networkStatus to 4 - until either success or error happens.

Actual outcome:
NetworkStatus remains at 8 and loading remains set to false on refetch. Only once server is up again and refetch completes susseccfully does networkStatus update to 7, and loading remains false the entire time.

How to reproduce the issue:

  • Create a simple component with a query and options of fetchPolicy: 'network-only', notifyOnNetworkStatusChange: true.
  • Have your component render a loading... text if loading is true.
  • Have your component render a refetch button<a onClick={() => refetch()>Try again</a> if error is true.
  • turn off your API server that has the graphql endpoint and load your component, it should render loading and then error states.
  • Click your refetch button a few times.
  • Turn back on your server.
  • Click refetch button again.
  • Note that networkStatus is staying on 8.

Related issues
See apollographql/react-apollo#642

@helfer
Copy link
Contributor

helfer commented Apr 28, 2017

@msmfsd I remember recently fixing a related issue. Can you check that this is still the case with the latest version?

If it is, could you create a reproduction with the error template? It should be possible to simulate a network error by simply throwing an error in the network interface the first time it's called: https://github.com/apollographql/react-apollo-error-template/blob/master/src/graphql/networkInterface.js

@msmfsd
Copy link
Author

msmfsd commented May 1, 2017

@helfer I updated libs and no change. I did create an error template and could not replicate the error at all. We have a large, complex app connected to a ruby backend with relay graphql and the issue happens on some components and not on others - and the components are quite similar..

We are using a refetching state as a workaround and it works fine so perhaps I will revisit this at a later point in time. Thanks for all help and I will update these issues if I can in future.

@msmfsd
Copy link
Author

msmfsd commented May 1, 2017

For anyone looking for the refetch workaround:

import React, { Component } from 'react'
import { graphql } from 'react-apollo'
import gql from 'graphql-tag'

class SomeComponent extends Component {

  state = { refetching: false }

  refetch = (): void => {
    this.setState({ refetching: true })
    this.props.refetch()
      .then(() => { this.setState({ refetching: false }) })
      .catch(() => { this.setState({ refetching: false }) })
  }

  render() {
    const { data: { loading, error, viewer } } = this.props
    const { refetching } = this.state

    if (loading || refetching) {
      return (<div>loading..</div>)
    } else if (error) {
      return (
        <div>
          <p>Error</p>
          <a onClick={this.refetch}>Retry</a>
        </div>
      )
    } else {
      return (<div>Name: {viewer.name}</div>)
    }

  }
}

const Viewer = gql`
  query Viewer {
    viewer {
      name
      uuid
    }
  }
`

export default graphql(Viewer, {
  options: {
    fetchPolicy: 'network-only',
    notifyOnNetworkStatusChange: true,
  },
})(SomeComponent)

@helfer
Copy link
Contributor

helfer commented May 2, 2017

Thanks @msmfsd!

If anyone could make a reproduction, that would be greatly appreciated. I think it's related to a similar issue on react-apollo about loading state not updating.

@helfer helfer added the 🐞 bug label May 2, 2017
@stale stale bot closed this as completed Jul 29, 2017
@stale
Copy link

stale bot commented Jul 29, 2017

This issue has been automatically closed because it has not had recent activity after being marked as stale. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to Apollo Client!

@jer-sen
Copy link
Contributor

jer-sen commented Sep 27, 2017

@jbaxleyiii : here are some precisions.

I work on a React Native project with:

I have a component "connected" using graphql HOC. I have a setInterval to trigger a refetch of the query. If a refetch fails, then the data.error remains, even if a refetch has succeeded after the fail.

Is it enough information for your to have a look at this issue ?

@tim-soft
Copy link
Contributor

@Jay1337 I'm having the same problem although I'm using Apollo client 1.x

I have a HOC query with a polling interval. If some network error occurs once, every future request will carry the past error despite returning good data.

@stale
Copy link

stale bot commented Oct 20, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

@jer-sen
Copy link
Contributor

jer-sen commented Oct 20, 2017

Hey! This is still an important and open issue!

@jbaxleyiii jbaxleyiii self-assigned this Oct 21, 2017
@jbaxleyiii jbaxleyiii added this to the Post 2.0 milestone Oct 21, 2017
@msmfsd
Copy link
Author

msmfsd commented Oct 23, 2017

@Jay1337 agreed! apollo please fix loading state updating issues.

@wojcikiewiczm
Copy link

+1

@SkReD
Copy link

SkReD commented Nov 12, 2017

@jbaxleyiii Looks like i fill issue with same problem in react-apollo issue. I put investigation result and reproduction steps in last sections of bugreport

@jbaxleyiii
Copy link
Contributor

I'm going to add this to the post 2.0 bug milestone to make sure it is prioritized and fixed for everyone! Sorry for the delay!

@jbaxleyiii jbaxleyiii removed this from the Post 2.0 milestone Nov 13, 2017
@webmobiles
Copy link

this Error is happing in 2.0.4 , it's very simple to test in network-only, turn off the server , and navigate page and when those pages give error, turn on the server, and try again, the props.query.error value keeps ... with the error, despite apollo is calling the page and on the network tab , data is sent from the server to the client, the problem that data is not loading... how I can help, in where code i can look for ?

@n1ru4l
Copy link
Contributor

n1ru4l commented Dec 11, 2017

@webmobiles I think a failing test for this scenario would be helpful

@webmobiles
Copy link

@n1ru4l how I could to do a failing test ? I can send you in private an acess to the app?

@adambom
Copy link

adambom commented Dec 15, 2017

For anyone having this issue, I seem to be having better luck after upgrading to the latest release of apollo-link-retry, which seems to have just been released. At the time of this comment, that version was 2.0.0 (I had been using version 1.0.x).

When I shut down my dev server and run a query, the query fails. Then I restart my server and do a refetch and it works as anticipated.

-- edit --
This is only kind of true. If I manually refetch a component it works, but if I call client.reFetchObservalbleQueries() that does not work, and all my polling breaks.

@msmfsd
Copy link
Author

msmfsd commented Dec 18, 2017

Thanks @adambom - wish there was some update on these and similar issues, seems like v2 has been live for ages with these issues..

@msmfsd
Copy link
Author

msmfsd commented Dec 18, 2017

@adambom can you possibly provide some code example of your use of apollo-link-retry?

@adambom
Copy link

adambom commented Dec 18, 2017

I think they version all their libraries differently: apollo-client has been at 2.0 for a while but the link packages are mostly still at 1.x. Here's the release I'm talking about: https://github.com/apollographql/apollo-link/releases/tag/apollo-link-retry%402.0.0

I'm not doing anything too different from the documentation example. If you just follow the example here you should get the behavior I'm describing.

That said, I've customized the behavior a little bit. I don't want network errors to crash the app, so I've prevented it from doing so by forcing apollo to retry on network errors forever. Graphql errors will still be handled the normal way, but here's what I've done:

export const errorBus = new EventEmitter();

const withRetries = new RetryLink({
  delay: {
    max: 30 * 1000,
  },
  attempts: (count, operation, error) => {
    console.log(`Retrying operation ${operation.operationName} (Attempt ${count})`);

    if (!!error) {
      errorBus.emit('error', error);
    }
    return !!error;
  }
});

const httpLink = new HttpLink({ uri: [Config.GRAPHQL_URI, 'graphql'].join('/') });

const errorHandler = onError(options => {
  errorBus.emit('error', options);
});

let link = ApolloLink.from([
  errorHandler,
  withRetries,
  httpLink,
]);

export const client = new ApolloClient({
  link,
  cache: new InMemoryCache(),
});

So the idea here is that if there's ever a network error (RetryLink ignores GraphqlErrors for some reason - by design) we just keep retrying. You could could program it to give up at some point, if you wanted, but in the event of a network error, I'm assuming it's because of bad connectivity and I want to reconnect at some point in the future. To notify any components that an error has occured, I create an EventEmitter and emit events. Anyone who cares there's been an error can subscribe to that emitter.

@msmfsd
Copy link
Author

msmfsd commented Dec 18, 2017

thanks @adambom much appreciated 👍

@jedwards1211
Copy link
Contributor

jedwards1211 commented Jan 10, 2018

@helfer currently network errors crash the app in that they can't be caught, and they remove the query from the store making it impossible to refresh. Transient network errors are sure to happen from time to time.
Is there any reason that an uncaught network or graphql error should put the app into an unrecoverable state?

@geirman
Copy link

geirman commented Jan 25, 2018

I'm seeing similar issues with react-apollo @2.0.4 in my react-native app. I've created an issue on Stack Overflow, that I'd love to get some eyeballs on.
https://stackoverflow.com/questions/48440527/how-to-refresh-component-data-with-apollo-refetch-data

Sounds like my expected results are valid...

  • refetch causes networkStatus to be reset from 8 to 4 (e.g. refetching)
  • refetch causes data.loading to be true
  • refetch automatically refreshes props.data with successful results or new errors

It also sounds like some of you are seeing your props.data refreshing. I'll check out apollo-link-retry as suggested @adambom above

However, I'm not having any luck refreshing the props.data so far, nor am I seeing any change to the data.error.message or data.networkStatus

UPDATE: While super interesting and also useful, apollo-link-retry turned out not to be the solution to my problem. I will use it though to smooth out the rough edges when network status is flaky.

@aheissenberger
Copy link

aheissenberger commented Feb 18, 2018

I have the same problem with :
"apollo-client": "^2.2.5",
"react-apollo": "2.0.0",

Problem

this.props.data.refetch() is not updating the render props after an network error (http 401 unauthorized)

Demo

Here is a demo based on the react-apollo-error-template and starts a minimal node server to simulate the network errors:
https://github.com/aheissenberger/react-apollo-error-401

This problem is a duplicate of
#2513

installing yarn add react-apollo@^2.1.0-beta.2 fixed the problem

@n1ru4l
Copy link
Contributor

n1ru4l commented Feb 22, 2018

@adambom Your workaround with RetryLink is the only option that is a possible to workaround #2258.

I also thought about using a link that reads the data from the cache in case of a network error (and publishes that to the the observable). However since I could not figure out how to read connections from the cache this was not possible for me.

@cpunion
Copy link

cpunion commented Mar 23, 2018

Easy to reproduce, hard to fix. Block me too long. 😭

@cpunion
Copy link

cpunion commented Mar 23, 2018

@aheissenberger It's gone after install react-apollo@^2.1.0-beta.3. Thank you.

@luccasmaso
Copy link

react-apollo@^2.1.0 fixes it 🙌

@billneff79
Copy link

It looks like it is fixed in 2.1.0, but is broken again in 2.1.1

@czystyl
Copy link

czystyl commented Mar 30, 2018

I can confirm that it works on 2.10, but 2.1.1 broke it again.

@zhenwenc
Copy link

Confirmed that its broken in 2.1.3, sigh...

@CianciuStyles
Copy link

Seems to be still broken in 2.1.4...

@Nizman92
Copy link

Nizman92 commented Aug 6, 2018

Same in 2.1.6.....

@hwillson hwillson removed this from the 2.0 upgrade blockers milestone Aug 10, 2018
@hwillson
Copy link
Member

hwillson commented Sep 3, 2018

I've updated the most recent reproduction (provided in #1622 (comment)) to use apollo-client 2.4.1 and react-apollo 2.1.11, and everything appears to be working properly. If anyone is still encountering this issue, please provide a small runnable reproduction (using current day apollo-client / react-apollo versions), and we'll take a look. Thanks!

@hwillson hwillson closed this as completed Sep 3, 2018
@jmarceli
Copy link

jmarceli commented Sep 20, 2018

Unfortunately, in my case even after update to [email protected] and [email protected] triggering refetch() still doesn't set loading to true (I'm using network-only fetchPolicy). I will try to provide some example soon.

EDIT:
Sorry I just didn't know that notifyOnNetworkStatusChange={true} is required to toggle loading state.

Everything works as expected :)

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

No branches or pull requests