radarlabs / react-native-radar

React Native module for Radar, the leading geofencing and location tracking platform
https://radar.com
Apache License 2.0
170 stars 32 forks source link

Headless JS support for Android (Inconsistent JS handling of Radar events in the background) #32

Closed noahtallen closed 6 years ago

noahtallen commented 6 years ago

We've been using Radar extensively for several users, and one strong trend we've noticed is that our JS enter/exit event handlers seem to miss several events. A basic version of how we're using it would be a user enters location A, then exits location A. After exiting the location, the entry shows up on their timeline for them to confirm, deny, or choose an alternate place.

Unfortunately, I can't seem to consistently catch every single enter/exit event. (I compare to the Radar console, which consistently has all of the events, albeit the Facebook place data seems to be out of date in a lot of cases). It's definitely much more reliable on iOS, as far as I can tell. I was wondering if you have tips on how to improve this. One thing I was looking into was Headless JS support. I'm did a very rough implementation earlier today for the package, but I don't think it worked, since the entire package kept giving me "error_network" for all location requests after the changes I made to the broadcast receiver. (Perhaps related to #30? It doesn't seem like the error was actually error_network, but probably something else going wrong).

I'd really appreciate any debugging tips you could give. I'd love to get this working as good as possible, and am definitely willing to submit PR's to help; I'm just not entirely sure what steps I should take to debug locally to make sure my implementation is correct.

One note, I've set up the Radar event handler in a redux-saga channel, since it was originally in the component hierarchy. I read that it needs to be outside of the component hierarchy so it keeps working in the background, so I moved it into a saga. Not sure if that's the best way to go, though.

russellcullen commented 6 years ago

Hey @ntomallen thanks for this report. We actually just fixed this in https://github.com/radarlabs/react-native-radar/commit/e367a20ff47fc606e79ed526f34eec1df132e416 and the most recent release 2.0.1.

For some context, when loading the JS bundle in the background we were going off thread without calling goAsync() in the broadcast receiver. This would cause the OS to kill the process before delivering events when memory constrained. More info on goAync() here https://developer.android.com/reference/android/content/BroadcastReceiver#goAsync()

Side note: I know you're blocked by #31 (sorry), so if you need a local workaround in the meantime, the change from that commit will work perfectly with previous versions.

noahtallen commented 6 years ago

Awesome! Excited to test that. Really hopeful we can get the update working soon :)