mapbox / mapbox-events-android

Mapbox telemetry and core libraries for Android.
https://docs.mapbox.com/android/core/overview
MIT License
60 stars 48 forks source link

Crash in location engine callback #429

Open ben-j69 opened 5 years ago

ben-j69 commented 5 years ago

Hello we are currently encountering an issue with the google location engine implemented in the SDK on Circle CI. I did not find anyway to customize the callback !

Steps to reproduce

  1. activateLocationComponent on CircleCI

Expected behavior

Should display the user position

Actual behavior

Crash the app : java.lang.IllegalArgumentException: latitude must be between -90 and 90 at com.mapbox.mapboxsdk.geometry.LatLng.setLatitude(LatLng.java:132) at com.mapbox.mapboxsdk.geometry.LatLng.<init>(LatLng.java:79) at com.mapbox.mapboxsdk.geometry.LatLng.<init>(LatLng.java:90) at com.mapbox.mapboxsdk.location.LocationAnimatorCoordinator.getPreviousLayerLatLng(LocationAnimatorCoordinator.java:171) at com.mapbox.mapboxsdk.location.LocationAnimatorCoordinator.feedNewLocation(LocationAnimatorCoordinator.java:82) at com.mapbox.mapboxsdk.location.LocationComponent.updateLocation(LocationComponent.java:1363) at com.mapbox.mapboxsdk.location.LocationComponent.access$1000(LocationComponent.java:97) at com.mapbox.mapboxsdk.location.LocationComponent$CurrentLocationEngineCallback.onSuccess(LocationComponent.java:1522) at com.mapbox.mapboxsdk.location.LocationComponent$CurrentLocationEngineCallback.onSuccess(LocationComponent.java:1510) at com.mapbox.android.core.location.GoogleLocationEngineImpl$GoogleLocationEngineCallbackTransport.onLocationResult(GoogleLocationEngineImpl.java:114) at com.google.android.gms.internal.location.zzau.notifyListener(Unknown Source:4) at com.google.android.gms.common.api.internal.ListenerHolder.notifyListenerInternal(Unknown Source:17) at com.google.android.gms.common.api.internal.ListenerHolder$zaa.handleMessage(Unknown Source:5) at android.os.Handler.dispatchMessage(Handler.java:105) at com.google.android.gms.internal.base.zap.dispatchMessage(Unknown Source:8) at android.os.Looper.loop(Looper.java:164) at android.app.ActivityThread.main(ActivityThread.java:6541) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)

Configuration

Android versions: 26 Device models: CircleCI Emulator Mapbox SDK versions: implementation 'com.mapbox.mapboxsdk:mapbox-android-sdk:8.4.0' implementation 'com.mapbox.mapboxsdk:mapbox-sdk-services:4.9.0' implementation 'com.mapbox.mapboxsdk:mapbox-android-navigation:0.41.0' implementation 'com.mapbox.mapboxsdk:mapbox-android-navigation-ui:0.41.0'

LukasPaczos commented 5 years ago

Hey @ben-j69, what are the actual values that the Google location engine returns? We're not hitting this issue in our instrumentation tests on device farms and I would suspect the emulator providing incorrect data.

Generally, if you want to test the LocationComponent integration, I wouldn't rely on data that's provided by either emulators or farm devices. You can initialize the component with a null engine instead and push updates with LocationComponent#forceLocationUpdate.

@alfwatt is it expected that the LocationEngine can return NaN or infinities as latitude/longitude?

ben-j69 commented 5 years ago

@LukasPaczos I did not found anyway to check this values since I can not override this location engine implementation and it only occur on CI environment.

I am writing a fix like "if ci does not use location" but the sdk should not crash at this point.

LukasPaczos commented 5 years ago

Per docs, [Location should always return valid values](https://developer.android.com/reference/android/location/Location.html#getLatitude()). Transferring this to the events repository for now. Let me know if you'd like to handle it differently @alfwatt.

alfwatt commented 4 years ago

Thanks @LukasPaczos, I'll ask @harvsu to have a look

alfwatt commented 4 years ago

Valid: https://github.com/mapbox/mobile-telemetry/issues/458

harvsu commented 4 years ago

@alfwatt @LukasPaczos GoogleLocationEngineImpl in Events repo (Telemtry SDK) does not alter any location information delivered by GMS (google play service )'s Location Engine and is sent as is to downstream SDK. Primary investigation into this issue confirms that the invalid latitude is coming from somewhere upstream, either the GMS or emulator itself.

harvsu commented 4 years ago

@ben-j69 Sorry for the delay! It seems to be some unknown issue with the emulator that you are using. We have not seen this occur in any of our tests. We need more information to get to the bottom of this. If you can get back to us with the following details, we will be able to debug this issue better.

  1. The emulator configuration in which you are running into this issue.
  2. Steps to reproduce this issue.
0xGuybrush commented 3 years ago

Hello,

just wanted to say that we're hitting this 'in the field' with real users. Please let me know if it's better to log a separate issue (as know the OP mentioned emulators), but the error we're hitting is the same.

Trying to debug, I've added logging for lat/lng/alt values to our crash reporter and are seeing values like this:

{Latitude=91.1466, Longitude=91.1466, Altitude=-0.5211749509147909}

i.e. when we hit this issue it seems like a valid Longitude is getting duplicated in the Latitude field — here it's just 4 decimals of precision, but have also seen it with 7 decimal places of precision.

Don't have much historic data, but I can see that it's happened for two of our users in the last couple of days — interestingly both on OnePlus devices running 7.1.1.

Obviously that's a very small sample size. I'll try to keep a close eye on it & loop back with details now I've found this ticket, if that's useful? Please let me know any other info I can try and gather on this too.

Thanks Dave

0xGuybrush commented 2 years ago

Seeing this still. Also with different device — Samsung Galaxy A90 5G (SM-A908N) but interestingly still running Android Nougat (this time 7.1.2).

I'm not sure what type of info you're tracking via the Telemetry of the SDK. If I could give you more info above the device/event that could help you track it down, please let me know.

Unfortunately I can't reproduce this on a test device, but still happening to end-users 'in the wild'.

Thanks Dave