VersionName causes the library to crash when using valid symver

simonsickle commented 3 years ago

Hello, our team uses a simple symver to determine what our version, avoiding headaches of trying to figure out which build is being used. The format we use is X.Y.Z-V where X is the major, Y is the minor, Z is the patch, and V is the version code of the APK.

Expected

As a consumer of this library, I should be able to use a valid symver for the version number without causing a crash. Furthermore, any string is valid for versionName - this field should not be used in a way that is unsafe with that in mind. Please refer to symver docs and google docs on versionName

Excerpt From Version Docs

The value is a string so that you can describe the app version as a <major>.<minor>.<point> string, or as any other type of absolute or relative version identifier. The versionName has no purpose other than to be displayed to users.

Actual

Because of this version, the following exception is throw at runtime. This is because the symver check being done by apptentive doesn't account for the valid symver format.

Crash Log

E/Apptentive: java.lang.NumberFormatException: For input string: "0-14644112"
        at java.lang.Long.parseLong(Long.java:594)
        at java.lang.Long.parseLong(Long.java:636)
        at com.apptentive.android.sdk.Apptentive$Version.compareTo(Apptentive.java:1773)
        at com.apptentive.android.sdk.Apptentive$Version.equals(Apptentive.java:1794)
        at com.apptentive.android.sdk.storage.VersionHistory.getTimeAtInstallForVersionName(VersionHistory.java:94)
        at com.apptentive.android.sdk.module.engagement.logic.FieldManager.doGetValue(FieldManager.java:118)
        at com.apptentive.android.sdk.module.engagement.logic.FieldManager.getValue(FieldManager.java:59)
        at com.apptentive.android.sdk.module.engagement.logic.ConditionalClause.evaluate(ConditionalClause.java:68)
        at com.apptentive.android.sdk.module.engagement.logic.LogicalClause.evaluateOperator(LogicalClause.java:79)
        at com.apptentive.android.sdk.module.engagement.logic.LogicalClause.evaluate(LogicalClause.java:68)
        at com.apptentive.android.sdk.module.engagement.interaction.model.InteractionCriteria.isMet(InteractionCriteria.java:41)
        at com.apptentive.android.sdk.module.engagement.interaction.model.Invocation.isCriteriaMet(Invocation.java:44)
        at com.apptentive.android.sdk.module.engagement.interaction.model.Targets.getApplicableInteraction(Targets.java:43)
        at com.apptentive.android.sdk.conversation.Conversation.getApplicableInteraction(Conversation.java:216)
        at com.apptentive.android.sdk.module.engagement.EngagementModule.doEngage(EngagementModule.java:92)
        at com.apptentive.android.sdk.module.engagement.EngagementModule.engage(EngagementModule.java:81)
        at com.apptentive.android.sdk.Apptentive.engageLocalAppEvent(Apptentive.java:1399)
        at com.apptentive.android.sdk.Apptentive.access$100(Apptentive.java:63)
        at com.apptentive.android.sdk.Apptentive$28.execute(Apptentive.java:1367)
        at com.apptentive.android.sdk.conversation.ConversationDispatchTask.executeGuarded(ConversationDispatchTask.java:68)
        at com.apptentive.android.sdk.conversation.ConversationDispatchTask.execute(ConversationDispatchTask.java:44)
        at com.apptentive.android.sdk.util.threading.DispatchTask.run(DispatchTask.java:39)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:214)
        at android.os.HandlerThread.run(HandlerThread.java:67)

CaseyApptentive commented 3 years ago

Thanks for sharing this, @simonsickle . Your explanation makes sense and it's not uncommon to use custom version naming schemes with Apptentive, though we have not had reports of this causing issues.

A few questions to help us track this down:

Which SDK version are you seeing this problem with?
Have you been able to successfully use Apptentive with similar versioning schemes in the past? If so, was that on a different SDK version?
Could you share full logs from your test, from app launch until the crash? If using logcat, you can add an "Apptentive" filter to only show the relevant parts.

Thanks again!

simonsickle commented 3 years ago

Currently, we see this in 5.5.3 but we have had to fork the library from master as we had a critical android 11 blocker (see the PR I created for this).

I believe this exact crash may be from the library version which didn't get set at first when we forked the library.

I'm not totally sure where this would be used as the upgrade dialogs appear to be based off of the versionCode instead.

haddaj commented 3 years ago

We have the same issue when the version name contains characters, the Apptentive sdk crash

CaseyApptentive commented 3 years ago

Hi @simonsickle and @haddaj . I apologize for the issue. Would you mind reaching out to support@apptentive.com with your report? That will help us keep your CSM updated on the issue so they can see it through.

Thank you! We appreciate the reports.

suraj-raj-pi commented 3 years ago

We ran into the same issue. Our app version names are configured based on the product flavour like "1.1.0", "1.1.0-staging", etc.

Currently seeing this issue with v5.6.1 This was working fine while using previous version of the SDK with similar versioning schemes. Not sure from which version of the SDK this was noticed.

HarryAWoodworth commented 3 years ago

Hi @suraj-raj-pi, thank you for reporting this issue. Can you reach out to support@apptentive.com with more details of when the crash occurs, and logs if possible? Also, what version names have you seen this crash occur with so far?

CaseyApptentive commented 3 years ago

We've continued to dig into this issue. We're unable to repro it, though we have an idea of what may be the root cause.

Is this issue impacting any apps in production right now, or is it only impacting dev builds? If this is impacting prod builds, can you reach out to support@apptentive.com with details on which app you're working on, that way we can help remediate it as soon as possible?

Thanks again for the reports.

suraj-raj-pi commented 3 years ago

Our production builds versionName follows the format - “8.0.0” , “10.1.0”, etc. so there is no impact there. It is with our dev builds and internal test builds which follows the format – “8.0.0 –local-build”, “10.1.0 -staging”, etc. that is currently impacted.

More info on how to reproduce :

Set the versionName of the app in build.gradle as below –

android { ... defaultConfig { ... versionName "1.1-demo" }

Set up LoveDialog in apptentive with some conditions like min OS version, apptentive event, etc.
Satisfy those conditions in the app and notice that LoveDialog never appears. (Note: App does not crash, dialog simply won’t appear)
Check the logs to find the NumberFormatException in Apptentive.Version class, since the class expects version String to be in the format of “0”, “0.1”, “1.0.0”, etc. but the Version String returned by the PackageManager at the time of initialisation is “1.1-demo” (which is stored by apptentive in VersionHistory)

Thanks, Suraj

apptentive / apptentive-android