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

NetInfo.isConnected.addEventListener behaves incorrectly #22966

Closed
3 tasks done
benjarwar opened this issue Jan 11, 2019 · 12 comments
Closed
3 tasks done

NetInfo.isConnected.addEventListener behaves incorrectly #22966

benjarwar opened this issue Jan 11, 2019 · 12 comments
Labels
API: NetInfo Bug 🌐Networking Related to a networking API. Resolution: Locked This issue was locked by the bot.

Comments

@benjarwar
Copy link

benjarwar commented Jan 11, 2019

Environment

  React Native Environment Info:
    System:
      OS: macOS High Sierra 10.13.6
      CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
      Memory: 172.94 MB / 16.00 GB
      Shell: 3.2.57 - /bin/bash
    Binaries:
      Node: 8.9.4 - /usr/local/bin/node
      Yarn: 1.13.0 - /usr/local/bin/yarn
      npm: 6.4.1 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
      Android SDK:
        API Levels: 23, 25, 26, 27
        Build Tools: 23.0.1, 25.0.2, 26.0.1, 27.0.3, 28.0.3
        System Images: android-23 | Intel x86 Atom_64
    IDEs:
      Android Studio: 3.1 AI-173.4819257
      Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.6.3 => 16.6.3 
      react-native: 0.57.8 => 0.57.8 
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native-git-upgrade: 0.2.7

Description

The callback passed to NetInfo.isConnected.addEventListener does not fire consistently nor accurately in iOS, 0.57.8. We cannot reliably determine whether or not the user is connected when following the documented method.

This is an enormous problem on our project, where we're trying to restrict requests and functionality when the user is offline.

Reproducible Demo

I created a test app with react-native init:

https://github.com/benjarwar/NetInfoTest

Simply added a NetInfo.isConnected event listener that console logs the connection status on connectionChange:

https://github.com/benjarwar/NetInfoTest/blob/master/App.js#L21-L25

Testing locally in iOS, on initial load it logs true. When I disable WiFi, it correctly logs false. But thereafter I get incorrect behavior:

  • Usually it doesn't log anything further when subsequently enabling/disabling WiFi, so our app thinks the user is disconnected even when WiFi has been restored
  • Sometimes, upon re-enabling, it logs true but then immediately/incorrectly logs false afterwards with WiFi still on
@benjarwar
Copy link
Author

@mmmulani you've worked on RCTNetInfo most recently. Any thoughts here?

@mmmulani
Copy link
Contributor

I've mainly done crash fixes on RCTNetInfo, not any actual feature improvement.

@karanjthakkar can you take a look at this?

@karanjthakkar
Copy link
Contributor

@mmmulani Thanks for tagging me.

@benjarwar I tested out your example and it seems like this issue has been happening for atleast the last 6 commits, which was before I touched this piece of code. Now I know this isn't the answer you are hoping for, but I don't have the bandwidth to debug the reason behind this falkiness atm. If you are sure it was working on some version of RN in the past, then the only thing I can recommend is to git bisect from the point to now and figure out which commit broke it. That will help someone who would be willing to look into this 🙂

@benjarwar
Copy link
Author

@karanjthakkar thanks for taking a look. I'll try to triage further.

@nol13
Copy link

nol13 commented Jan 16, 2019

Simulator or device? When i switch wifi on an off on laptop connectionChange still only fires once in simulator, but this has been working pretty reliably for us in 57.8 on device. (I actually thought this was fixed, appears still an issue though since when I remove the hacky code our offline status check goes bonkers.)

Edit: hacky code removed, no longer needed?

@HighSoftWare96
Copy link

I have similar problems on NetInfo too. The listener behaves unpredictably even when there are changes on the phone status like when it goes in stand-by.

@benjarwar
Copy link
Author

benjarwar commented Jan 17, 2019

@nol13 interesting. In testing this further now, it seems not to be occurring now on my actual device (I swear it was previously!), but still consistently occurs in the iOS Simulator.

So is this a known problem? Are there other issues or documentation about NetInfo behaving sporadically, specifically in the Simulator only?

If anything, it behooves a note in the documentation, especially for such a critical feature.

I'll try out your hacky solution. It feels a little redundant and brittle to call isConnected.fetch() twice in a nested manner with that timeout, though...

Also, what's up with needing to check the network status on app state change? Have you encountered inaccuracies when the app comes back into focus? This sounds like what @HighSoftWare96 is describing.

@nol13
Copy link

nol13 commented Jan 17, 2019

Ya, that's what the app state check was for. It would sometimes be offline for a split second right after unlocking, but the event wouldn't fire when it went back on. The double check is because only the second check was reliable, and even then sometimes (every few days) it needed a bit of a timeout as well.

That code was originally to work around NetInfo as it existed in 55.3, but after the 57.8 upgrade didn't notice any issues so figured I'd post what we had. I suspect the double check and AppState check may not be needed anymore. I'll edit post to avoid confusion.

And ya, I tried last night with just..

_checkNetworkStatus = (isConnected) => {
    this.setNetworkStatus(isConnected);
}

and it completely blew up on me even on device, but worked fine when I did a single NetInfo.isConnected.fetch().

yet now just tried again and above seems to be working without issue :/

@benjarwar
Copy link
Author

benjarwar commented Jan 17, 2019

@nol13 your code doesn't seem to help in the Simulator. The problem seems to be with the connectionChange event. It simply doesn't fire reliably after the first disconnection event.

What's interesting, though, is if I add a check for NetInfo.isConnected.fetch() in a setInterval, then the connectionChange dispatches reliably again in the Simulator. Like, calling NetInfo.isConnected.fetch() resets/unclogs the event dispatch stream somehow.

I've updated my NetInfoTest demo with a setInterval NetInfo check that displays the current connectivity value in the app. Note that it's not necessary that the setInterval check actually does anything. This seems to work reliably for me both in the Simulator and on an actual device (both running iOS 11.4).

I still feel like this functionality needs some attention. These "solutions" are flaky and non-deterministic.

Example code:

  constructor(props) {
    super(props);
    this.state = { isConnected: "false" };
  }

  componentDidMount() {
    NetInfo.isConnected.addEventListener("connectionChange", isConnected => {
      console.log("addEventListener isConnected", isConnected);
      this.setState(() => ({ isConnected: isConnected ? "true" : "false" }));
    });

    setInterval(() => {
      NetInfo.isConnected.fetch().then(isConnected => {
        console.log("setInterval isConnected", isConnected);
      });
    }, 1000);
  }

@eviliana
Copy link

eviliana commented Jan 22, 2019

I have similar problems on NetInfo too. The listener behaves unpredictably even when there are changes on the phone status like when it goes in stand-by.

I have the same problem but only for Android devices, isConnected returns false if the device wakes up after 10-15 minutes (but no sooner than 5-10 mins).

@elliottkopp
Copy link

elliottkopp commented Feb 7, 2019

I'm having the same issue and tried @benjarwar's workaround of using a setInterval to call isConnected.fetch() and that didn't seem to "unclog" it for me. No matter what I do, it just never fires again after the first time. Works 100% of the time on an actual device.

Right now, what I had to do was, within the callback I remove the event listener when it gets fired. Then I re-add it a few seconds later with a setTimeout. Definitely not ideal and I can't actually release this code. But it works for local development at least.

I'm on react-native version 0.50.4 which is a little over a year old now. I'm just implementing this now so I can't say whether it worked before or not. But I really doubt that this has been broken for over a year. I wonder if it's an emulator issue since it works on the device 100%?

Emulator version 10.0 (SimulatorApp-851.2 CoreSimulator-581.2).

@cpojer
Copy link
Contributor

cpojer commented Feb 12, 2019

Heads up: we moved react-native-netinfo into its own repository, which should allow for making changes and fixes much faster. Please continue the discussion about this issue there: react-native-netinfo/react-native-netinfo#7

@cpojer cpojer closed this as completed Feb 12, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Feb 12, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API: NetInfo Bug 🌐Networking Related to a networking API. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

10 participants