urbanairship / android-library

Urban Airship Android SDK
Other
109 stars 123 forks source link

Upgrading to 16.3/16.3.1 stops takeOff from autoPilot #202

Closed barry-irvine closed 2 years ago

barry-irvine commented 2 years ago

❗For how-to inquiries involving Airship functionality or use cases, please contact (support)[https://support.airship.com/].

Preliminary Info

What Airship dependencies are you using?

fcm, automation and message center 16.2

What are the versions of any relevant development tools you are using?

Android Studio Bumble Bee and associated gradle plugin

Report

What unexpected behavior are you seeing?

When upgrading to the latest version of the libraries I was getting a crash because we were trying to access shared before takeoff. By putting some logging in the subclass of AutoPilot that is referenced from the manifest I could see that the auto pilot was no longer being accessed in the new versions.

What is the expected behavior?

Take off would work as it does in 16.2

What are the steps to reproduce the unexpected behavior?

Upgrade to 16.3 or 16.3.1 with a subclass of Autopilot referenced from the manifest

Do you have logging for the issue?

No

rlepinski commented 2 years ago

@barry-irvine We recently switched to using jetpacks app-startup library. Are you disabling it by any chance? We probably are missing documentation on workaround if its disabled.

barry-irvine commented 2 years ago

@rlepinski That must be it. We have this in the manifest:

    <!-- If you want to disable android.startup completely. -->
    <provider
        android:name="androidx.startup.InitializationProvider"
        android:authorities="${applicationId}.androidx-startup"
        tools:node="remove" />

I believe it is there so that Hilt and WorkManager play nicely together. I'll try the alternative configuration:

  <provider
      android:name="androidx.startup.InitializationProvider"
      android:authorities=\"${applicationId}.androidx-startup"
      android:exported="false"
      tools:node=\"merge">
  <!-- If you are using androidx.startup to initialize other components -->
  <meta-data
    android:name="androidx.work.impl.WorkManagerInitializer"
    android:value="androidx.startup"
    tools:node="remove" />
 </provider>
barry-irvine commented 2 years ago

Yes. That fixes it. Thanks!

rlepinski commented 2 years ago

Thanks, ill talk with the team to figure out where to document it. Another alternative is you can call Autopilot.autoamticTakeOff in your application#onCreate, but what you have is IMO better

barry-irvine commented 2 years ago

Annoyingly enough lint stops my build when I configure the manifest like that, claiming that I'm not removing the WorkManagerInitializer so I also had to disable the lint check for RemoveWorkManagerInitializer

barry-irvine commented 2 years ago

Actually I've done a bit more digging. It doesn't matter which way I disable WorkManagerInitializer in the manifest, my Hilt Workers still work UNTIL I upgrade Airship to 16.3.1 - in which case my @HiltWorker annotated classes are no longer available. I guess this is because your initializer is dependent on WorkManagerInitializer so it gets kicked off automatically when your Initializer starts which causes my Configuration.Provider classes not to work properly.

rlepinski commented 2 years ago

@barry-irvine Right, thats pretty annoying. How about removing Airship as well and calling Autopilot.automaticTakeOff(this) in application#onCreate ? We will look into removing the setup order since our SDK does handle retrying scheduling work but I would want to do some more testing to make sure it wont cause too much thrash

Thomasvhallxxx commented 2 years ago

@rlepinski That must be it. We have this in the manifest:

    <!-- If you want to disable android.startup completely. -->
    <provider
        android:name="androidx.startup.InitializationProvider"
        android:authorities="${applicationId}.androidx-startup"
        tools:node="remove" />

I believe it is there so that Hilt and WorkManager play nicely together. I'll try the alternative configuration:

  <provider
      android:name="androidx.startup.InitializationProvider"
      android:authorities=\"${applicationId}.androidx-startup"
      android:exported="false"
      tools:node=\"merge">
  <!-- If you are using androidx.startup to initialize other components -->
  <meta-data
    android:name="androidx.work.impl.WorkManagerInitializer"
    android:value="androidx.startup"
    tools:node="remove" />
 </provider>
barry-irvine commented 2 years ago

@rlepinski Any movement on this? At the moment I'm still on 16.2.0 because upgrading causes this crash and I can't remove hilt/workmanager from my project.

rlepinski commented 2 years ago

@barry-irvine You should be able to just disable Airship as well and add this to your Application#onCreate:

GlobalActivityMonitor.shared(this)
Autopilot.automaticTakeOff(this, true)

Removing work manager dependency caused a little bit of thrash so I am hesitant on removing it. We could maybe provide an alternative one you can register:

public class AirshipAltInitializer implements Initializer<Boolean> {

    @NonNull
    @Override
    public Boolean create(@NonNull Context context) {
        GlobalActivityMonitor.shared(context.getApplicationContext());
        Autopilot.automaticTakeOff((Application) context.getApplicationContext(), true);
        return UAirship.isTakingOff() || UAirship.isFlying();
    }

    @NonNull
    @Override
    public List<Class<? extends Initializer<?>>> dependencies() {
        return Collections.emptyList();
    }
}

And you can register with:

<provider
            android:name="androidx.startup.InitializationProvider"
            android:authorities="${applicationId}.androidx-startup"
            android:exported="false"
            tools:node="merge">
            <meta-data  android:name="com.urbanairship.AirshipAltInitializer"
                android:value="androidx.startup" />
        </provider>
rlepinski commented 2 years ago

Next release I moved the GlobalActivityMonitor setup within the takeOff/automaticTakeoff call so you only need to call it. Also added a new NoDependencyAirshipInitializer so instead of calling automaticTakeoff yourself you can just do this to your manifest:

        <provider
            android:name="androidx.startup.InitializationProvider"
            tools:node="merge"
            android:authorities="${applicationId}.androidx-startup"
            android:exported="false">

            <meta-data
                android:name="androidx.work.WorkManagerInitializer"
                android:value="androidx.startup"
                tools:node="remove" />

            <meta-data
                android:name="com.urbanairship.AirshipInitializer"
                tools:node="remove" />

            <meta-data
                android:name="com.urbanairship.NoDependencyAirshipInitializer"
                android:value="androidx.startup" />
        </provider>

The release will most likely be out next week.

sankartringapps commented 2 years ago

@rlepinski Am also facing the same issue and I prefer defining a own AirshipInitializer until we get NoDependencyAirshipInitializer in the next release but the method Autopilot.automaticTakeOff((Application) context.getApplicationContext(), true); is not public (package-private).

Can I use the other variant of the same method which has "earlyTakeOff = false" by default? will that cause any issues?

class CustomAirshipInitializer : Initializer<Boolean> {

    override fun create(context: Context): Boolean {
        GlobalActivityMonitor.shared(context.applicationContext)
        Autopilot.automaticTakeOff((context.applicationContext as Application))
        return UAirship.isTakingOff() || UAirship.isFlying()
    }

    override fun dependencies(): List<Class<out Initializer<*>?>> {
        return emptyList()
    }
}
rlepinski commented 2 years ago

@sankartringapps I apology for the delayed response. I must not of pressed comment last time.

There is nothing wrong with just calling automatic takeoff without the BOOL. It's just a flag that we expose to Autopilot if apps want to prevent Airship from taking off before the application#onCreate.

The release is being prepped to go out today

barry-irvine commented 2 years ago

Looking good to me. 👍 I think this can be closed now

bmesing commented 10 months ago

@rlepinski: pulling in WorkManagerInitializer even though it is disabled is really unexpected behaviour. It did cost me two days to figure out, why adding hilt work manager to my project did not work as expected. Libraries should not mess with global configuration. I strongly suggest to change default behaviour of your library. Basically what can happen is that you are breaking an existing application when pulling in Airship resulting in crashes at runtime.