aerogear / offix

GraphQL Offline Client and Server
https://offix.dev
Apache License 2.0
758 stars 45 forks source link

React Native offline mutation is being sent twice #409

Closed kingsleyzissou closed 4 years ago

kingsleyzissou commented 4 years ago

Bug Report

It appears that optimistic responses are not removed from the cache in React Native, which results in duplicate items.

ganashreeravikumar commented 4 years ago

After debugging I found that the handleNetworkStatusChange is being called twice which will call listener({ online }) twice, thus ending up calling the mutation twice.

Here is a workaround I am using:

class NetworkStatus {
  listeners = [];
  prevState = false;

  ...
  ...

  handleNetworkStatusChange(state) {
    if ((!this.prevState && state.isInternetReachable) ||
        (this.prevState && !state.isInternetReachable)) {
      const online = state.isInternetReachable
      this.listeners.forEach((listener) => {
        listener({ online })
      })
    }

    this.prevState = state.isInternetReachable
  }
}
jacruzca commented 4 years ago

Thanks for the workaround @ganashreeravikumar. I tested it but it doesn't work on app restarts though. Steps in a real device:

Mutations are still fired multiple times :(

kingsleyzissou commented 4 years ago

@ganashreeravikumar thaks for debugging this. Slight modification of this should work for @jacruzca too.

class NetworkStatus {
  listeners = [];
  prevState = false;

  ...
  ...

  handleNetworkStatusChange(state) {
    const online = state.isInternetReachable
    if ((online !== this.prevState) {
      this.prevState = online;
      this.listeners.forEach((listener) => {
        listener({ online })
      })
    }
  }
}

This has been added now to the React Native example app (PR #420)

bureyburey commented 4 years ago

Also having this issue with the following steps (after adding the prevState changes) 1) go offline (airplane mode) 2) make an offline mutation 3) restart the app (while on airplane mode) 4) once the app is loaded, go back online 5) mutation sent twice

It also happens when putting the application on the background, returning to it and going back online (without restarting the app).

happens when going online when the application is in the background.

kingsleyzissou commented 4 years ago

@bureyburey

what platform are you using react native on? IOS or Android and is it on a simulator or an actual device?

Are you using the example app or your own?

I am just trying to replicate the issue. On an actual IOS device on the sample app, neither scenarios are causing a duplicate mutation. Will try test on the simulator

Edit: seems to be okay on the iOS simulator too

wtrocki commented 4 years ago

@kingsleyzissou That is why we need to have logging for storage. It looks like there is some particular storage issue for IOS that we do not see.

bureyburey commented 4 years ago

Not using the examlple app.

I had issues testing it on iOS since i only tested on the simulator which has issues with NetInfo going online => offline => online (see here)

I encountered the issue when testing on an actual android device on development build.

I will update here once i extensively test it on production build.

kingsleyzissou commented 4 years ago

I noticed that the netinfo issue with the iOS simulator too.

Yeah keep us updated

bureyburey commented 4 years ago

Update: This issue seems to NOT reproduce on android production build.

The mutation is only sent once as expected.

Tried these and it worked as expected:

1) activate airplane mode => offline mutate => deactivate airplane mode => mutation called once as expected

2) activate airplane mode => offline mutate => close and re-open app => deactivate airplane mode => mutation called once as expected

I guess this can be closed and sorry for the hassle 😅

kingsleyzissou commented 4 years ago

No problem at all. Thanks for getting back to us, if you have any issues again feel free to re-open.

While debugging this I found one or two other issues, so definitely not a hassle

bureyburey commented 4 years ago

Hey Offix guys, me again 😅

Seems this issue does happens in production build, tho not consistently.

As a temporary workaround i am thinking of creating an apollo link that will track the ids of the sent mutation using a global object and prevent sending a mutation twice.

Not sure how exactly i can remove the operations from the queue manually (and persist it) so i would love to hear your input regarding it.

Many thanks and looking forward to hear from you :)