snowplow / snowplow-android-tracker

Snowplow event tracker for Android. Add analytics to your Android apps and games
http://snowplowanalytics.com
Apache License 2.0
109 stars 63 forks source link

Catch more generic exceptions for InstallReferrer (close #647) #681

Closed mscwilson closed 6 months ago

mscwilson commented 6 months ago

For issue #647

The InstallReferrerDetails class currently checks whether the InstallReferrer dependency is present or not, but if it is, then only catches RemoteExceptions. InstallReferrer below v1.1 doesn't have the googlePlayInstantParam property which causes a NoSuchMethodError.

Unfortunately the googlePlayInstantParam is the one required property for the referrer_details schema, so if InstallReferrer v1.0 is used it's not possible to add the entity at all.

This change catches the NoSuchMethodError.

VenomVendor commented 6 months ago

As the code will always crash without the dependency, why was the decision to catch Exception instead of updating transitive dependencies?.

Can this PR also update about the missing info in the doc

mscwilson commented 6 months ago

Hi @VenomVendor good shout about the docs!

I don't understand the first part of your message sorry. The code already caught when the dependency is completely absent, now it will also catch versions without the required functionality.

VenomVendor commented 6 months ago

I meant, why to catch the exception if we can update transitive dependency. Here, we're using compileOnly, instead if we could update to implementation. This exception handling can be removed, as the library with specific version will be bundled.

I was in an assumption that the dependency install-referrer is mandatory. Found this check isInstallReferrerPackageAvailable().