[Bug] Android Geofence NullReferenceException

[shinyorg]

[Bug] Android Geofence NullReferenceException

[aritchie]

287 isn’t in public nuget yet. Please wait for public releases before filing defects on it. I only work on master so I don’t always have things in order yet

[aritchie]

Your NRE is happening during the geofence process. Is it coming from your delegate code perhaps?

1. I'm actually going to move this to an individual call instead of an array.
2. based on 1, this won't matter
3. This is by design. StartupTasks must all pass (unless the author of the task doesn't need them to) as it was designed for wiring up infrastructure. If they fail, they should crash otherwise you can start seeing missing behavior in your app potentially.

[aritchie]

I'm going to consider this closed case for now. Anything related can be a separate issue at this point.

[aritchie]

I only work on dev I meant. So things keep pushing to myget

[aritchie]

I couldn't reproduce this. By crash, are you meaning it is showing up in the logging in the sample?

[dahlbyk]

I haven't seen it yet in the Sample (not certain what Shiny build it's using - that would be a neat addition to the Environment screen), but it has now happened twice in the real app I'm close to releasing.

I do want to correct myself: it's being tracked as an error, not a crash, presumably here:

shiny/src/Shiny.Locations/Platforms/Android/GeofenceManagerImpl.cs

Lines 47 to 56 in cfdb474

	try
	{
	var regions = await this.Repository.GetAll();
	foreach (var region in regions)
	await this.Create(region);
	}
	catch (Exception ex)
	{
	Log.Write(ex);
	}

A few comments on that log call in particular:

1. What if each region.Create() had its own try/catch to include region details with the exception?
2. AggregateException being a generally unhelpful wrapper around the actual error(s), I wonder about having the Shiny Logging infrastructure check for AggregateException and log the InnerExceptions directly.
3. Thoughts on having the infrastructure that calls IShinyStartupTask.Start() handle the general try/catch, rather than individual tasks needing to do it?

I need to check with the guy testing the app, but this error may be due to attempting to recreate the geofence after revoking permission for Locations.

[dahlbyk]

1. What if each region.Create() had its own try/catch to include region details with the exception?

Took a shot at this: #40. I believe this will avoid this particular AggregateException as well.

[dahlbyk]

I need to check with the guy testing the app, but this error may be due to attempting to recreate the geofence after revoking permission for Locations.

I can't rule out permissions, but I thought I'd mention I have a separate crash report specifically related to permissions:

android.os.Parcel.createException

Parcel.java, line 1950
java.lang.SecurityException: Geofence usage requires ACCESS_FINE_LOCATION permission

android.os.Parcel.createException Parcel.java:1950
android.os.Parcel.readException Parcel.java:1918
android.os.Parcel.readException Parcel.java:1868
com.google.android.gms.internal.zzeb.zzb
com.google.android.gms.internal.zzbzl.zza
com.google.android.gms.internal.zzbzu.zza
com.google.android.gms.internal.zzbzc.zza
com.google.android.gms.common.api.internal.zzm.zzb
com.google.android.gms.common.api.internal.zzc.zza
com.google.android.gms.common.api.internal.zzbr.zzb
com.google.android.gms.common.api.internal.zzbr.zza
com.google.android.gms.common.api.internal.zzbp.handleMessage
android.os.Handler.dispatchMessage Handler.java:102
android.os.Looper.loop Looper.java:193
android.os.HandlerThread.run HandlerThread.java:65

I'd be curious to read your thoughts on the "Shiny way" to handle this, pre-create and if permissions are lost after the geofence is originally added to Shiny's repository.

Your NRE is happening during the geofence process. Is it coming from your delegate code perhaps?

I'd expect my code to appear in a stack trace if it were, but I doubt it:

        private class GeofenceDelegate : IGeofenceDelegate
        {
            public async Task OnStatusChanged(GeofenceState newStatus, GeofenceRegion region)
            {
                Analytics.TrackEvent("Geofence Change", new Dictionary<string, string>
                {
                    { "id", region?.Identifier },
                    { "center", region?.Center?.ToString() },
                    { "radius", region?.Radius?.ToString() },
                    { "status", newStatus.ToString() },
                });

                switch (newStatus)
                {
                    case GeofenceState.Entered:
                        await Api.Post.GeofenceTrigger(region.Identifier, true);
                        break;
                    case GeofenceState.Exited:
                        await Api.Post.GeofenceTrigger(region.Identifier, false);
                        break;
                }
            }
        }

[dahlbyk]

Your NRE is happening during the geofence process.

I was looking for where it might have been logged. You're thinking it's in here:

shiny/src/Shiny.Locations/Platforms/Android/GeofenceProcessor.cs

Line 27 in cfd1ed4

public async Task Process(Intent intent)

via

shiny/src/Shiny.Locations/Platforms/Android/GeofenceBroadcastReceiver.cs

Lines 23 to 29 in cfd1ed4

	public override void OnReceive(Context context, Intent intent) => this.Execute(async () =>
	{
	if (intent.Action == INTENT_ACTION)
	await ShinyHost
	.Resolve<GeofenceProcessor>()
	.Process(intent);
	});

shiny/src/Shiny.Core/Platforms/Android/Dispatcher.cs

Lines 11 to 21 in cfd1ed4

	public static void Execute(this BroadcastReceiver receiver, Func<Task> task)
	{
	var pendingResult = receiver.GoAsync();
	task().ContinueWith(x =>
	{
	if (x.IsFaulted)
	Log.Write(x.Exception);
	pendingResult.Finish();
	});
	}

Odd that the error was reported as GeofenceManagerImpl.Process(Intent).

[aritchie]

Odd that the error was reported as GeofenceManagerImpl.Process(Intent).

The code of where Process was happening has moved in dev which is why you aren't seeing it relating to your error.

I'd be curious to read your thoughts on the "Shiny way" to handle this, pre-create and if permissions are lost after the geofence is originally added to Shiny's repository.

The permissions error makes sense and there isn't much I can do about that. This is the reason I try to blindly restore the geofences on boot. If permission has been pulled, you'll likely want to do check when your app is in the foreground. I could downgrade the log to an event instead of an error. May even add a category specifically around this type of issue

I'd expect my code to appear in a stack trace if it were, but I doubt it:

I'm still not seeing where the NRE could be. Is there more to the stack trace in your appcenter log?

[dahlbyk]

I'd expect my code to appear in a stack trace if it were, but I doubt it:

I'm still not seeing where the NRE could be. Is there more to the stack trace in your appcenter log?

There is not. Not sure if that's due to AggregateException or how Dispatcher Task handling works or something else. I can bump to the latest beta to see if this persists.

Beyond #40, which now seems unlikely to be related to what I am seeing, I am not sure how to get a better trace on my actual error. 🤔

I did try revoking Location permission for the sample app with geofences configured…no errors logged so far. 🤷‍♂️

[dahlbyk]

Note: I reformatted the original exception stack trace as code, as some of the generics (specifically <OnReceive>) were being interpreted as HTML. I'm looking closer at the Process() code that moved in 1efa4eb, and there are a couple NRE opportunities:

shiny/src/Shiny.Locations/Platforms/Android/GeofenceManagerImpl.cs

Lines 60 to 68 in 5cff99d

	var e = GeofencingEvent.FromIntent(intent);
	if (e == null)
	return;
	foreach (var triggeringGeofence in e.TriggeringGeofences)
	{
	var region = await this.Repository.Get(triggeringGeofence.RequestId);
	if (region != null)
	{

I took a closer look at the GeofencingEvent docs and I'm reasonably certain it's an unhandled geofence error. PR incoming (separate from #40).

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.