apptentive / apptentive-kit-ios

ApptentiveKit SDK for iOS and iPadOS
https://www.apptentive.com
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

AssertionFailures in SDK 6.0.x #18

Closed jaishankar closed 1 year ago

jaishankar commented 2 years ago

Team - With the SDK 6.0.x, we notice that Apptentive SDK uses assertion failure through out in many places,

when the registration key is not of valid format, when there is a registration failure when there is a new credential being used without deleting the app This breaks our app as we are providing an option in debug mode to switch between environments and on switching we will register with the appropriate credentials for the environment and restart the app flow. With 6.0.x this is breaking because of the assertion failures and there is no way for us to overcome this.

SDK should ideally throw an error which the app integrating the SDK should decide on how to deal with the error instead of SDK crashing the app with assertionFailure. Could you please consider this?

Also the isTesting check doesn't capture all different tests like UITests etc.. it works for unit tests only. We might need this Environment being configurable or a key like SkipApptentive as a launch argument when added to specific Test targets should not throw assertionFailures.

frankus commented 2 years ago

The issue we're trying to surface with the assertion failure is that changing the app credentials in an installed app makes the Apptentive instance no longer capable of communicating with the server, which leads to bugs whose source isn't immediately obvious.

In the long run we're looking at ways to support changing the app credentials on launch, but in the short run, we could surround the assertion with a #if, where if a developer needs to change credentials without reinstalling and is aware of the consequences, they can add a preprocessor setting to their target. Would that work with your setup?

jaishankar commented 2 years ago

Hi @frankus, Thanks for quick response As a short term solution if we could get,

As a long term approach if can we get

Appreciate quick resolution for the same, so we could get using this latest SDK. Thanks again!

jaishankar commented 2 years ago

Hi @frankus any updates on this?

GitTheCode commented 1 year ago

I'm also running into this - and it is making various testing paths impossible using a debug build. As an example, testing upgrade from app store version to the new version built locally in Xcode. I've run into this twice on two different apps - it's unrealistic (and impolite?) as a framework/sdk to require deleting/reinstalling an app when something goes wrong. Please remove the assertions until you have a better solution - one that allows clearing or resetting credentials for instance, or throws an error that can be handled by the calling code. Thanks -

frankus commented 1 year ago

We have a plan to allow changing the key/signature between launches, with each set of credentials having its own isolated data store on the device (our backend isn't set up to move/share data between multiple apps in the dashboard). We hope to get this implemented in the next sprint or two.

jaishankar commented 1 year ago

Could we expect any update on this?

frankus commented 1 year ago

Sorry for the radio silence on our part. The next release (should be out within hours to days) will have a public apptentiveAssertionHandler global variable that you can set to a function/closure that accepts the same arguments as assertionFailure.

I'll update back here once the release is out.

In terms of the credential issue, this is an interim band-aid, and we're still working on a way to change credentials without uninstalling/reinstalling. It just touches quite a bit of low-level code and we want to have the time to test it thoroughly.

frankus commented 1 year ago

This should be addressed in https://github.com/apptentive/apptentive-kit-ios/releases/tag/v6.0.4.

CaseyApptentive commented 1 year ago

@jaishankar and @GitTheCode, let us know if that frees you up!

jaishankar commented 1 year ago

Thanks yes this helps us to start using this SDK.

Hope the other part related to AppCredentiials validation will be addressed in a future release.

AppCredentials constructor validate the key and signature and return an optional AppCredentials object, consuming app can proceed to register only when valid

frankus commented 1 year ago

The root cause fix is finally released in v6.2.0.

Each key/signature pair operates with its own persistent storage on the device that shouldn't interfere with the files from any other key/signature pair.