Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distribute.setEnabled(false) has no effect for non debuggable builds #1208

Closed
luboganev opened this issue Jul 8, 2019 · 7 comments
Closed
Labels

Comments

@luboganev
Copy link

luboganev commented Jul 8, 2019

Description

I'm running the AppCenter Distribute normally only for non-production builds to distribute internally and I want it disabled when I upload to Google Play. I have setup the setEnabled and setEnabledForDebuggableBuild both to false in this order. Despite of that my Google Play Live Production app suddenly started opening the AppCenter in the browser every time I start it.

Repro Steps

  1. Setup the AppCenter Distribute SDK in the main Application class of a project like this:
    Distribute.setEnabled(false); Distribute.setEnabledForDebuggableBuild(false); AppCenter.start(this, "<my appcenter id>", Crashes.class, Analytics.class, Distribute.class);
  2. Build and run a non-debuggable build for your application. (For example a production release)
  3. The SDK opens AppCenter website in an external browser although it shouldn't.

Details

  1. Which SDK version are you using?
    • e.g. 2.1.0
  2. Which OS version did you experience the issue on?
    • any Android OS
  3. What device version did you see this error on? Were you using an emulator or a physical device?
    • Google Pixel 2 , Pixel XL, Emulator
  4. What third party libraries are you using?
    • Unrelated. Appcenter gets initialized before any other libraries in the very beginning of the main application file.
  5. Please enable verbose logging for your app using AppCenter.setLogLevel(Log.VERBOSE) before your call to AppCenter.start(...) and include the logs here:
    I cannot do that, the issue happens on non-debuggable builds, so I cannot see any log messages.

My currrent workaround:
The only way to make sure I don't get this wrong behavior is by no including the Distribute.class at all when starting AppCenter SDK.

@guperrot
Copy link
Member

guperrot commented Jul 8, 2019

Hi, Distribute.setEnabledForDebuggableBuild(false) has effect only on debug builds.

For release builds from the store we use an automatic store detection, that's probably what failed in your case and to troubleshoot that we will need the verbose logs of the SDK. For example have 1 of the release build use AppCenter.setLogLevel(Log.VERBOSE) before start then use the logcat to retrieve logs (that also works in release). I recommend alpha or beta channel for this build or another way to avoid everyone seeing that test update.

As for why Distribute.setEnabled(false) didn't work: this API requires storage to persist the setting and is thus allowed only being called after AppCenter.start, you can move the call right after it, it will be taken in account before the in-app updates process starts.

@luboganev
Copy link
Author

luboganev commented Jul 9, 2019

Hi @guperrot ,

Thank you for the answer. My question refers mainly to the Distribute.setEnabled(false). I just copy pasted the entire code for completeness. I think I have understood how the API works. Please check those two points and confirm or comment on them:

  1. For debuggable builds Distribute.setEnabledForDebuggableBuild(false) kicks in and although the Distribute.setEnabled(false) has no effect before start, the first one disables the functionality.
  2. For non-debuggable builds Distribute.setEnabledForDebuggableBuild(false) is ignored, since the build is non-debuggable. The Distribute.setEnabled(false) still has no effect, thus the functionality is not disabled, and the popup is shown.

It is clearly a case of wrong usage from my side. However, there is absolutely no mention in any documentation that order of operations is important. The examples for Distribute.setEnabled(false) and AppCenter.start in the Java docs or on the
Documentation website exist in total isolation from each other. Please update it in a future release, otherwise this "wrong usage" might be something many devs would do by default.

I would like to give you an extra feedback from my side. In my opinion, everything related to debuggable or non-debuggable builds should not be of any concern to the any SDK. If such information is needed, it should be just configurable through flags upfront. Any decision depending on the type of the build should be made exclusively by the developer of the app and not hidden inside the code of any third party SDK. Otherwise our builds become extremely unpredictable.

@Jamminroot
Copy link
Contributor

Hey, @luboganev, you are correct.
Using Distribute.setEnabledForDebuggableBuild is only valid for debug builds, and would prevent Distibute module from functioning even if called before AppCenter.start (but again, only for debug builds).
For setEnabled it is different - it should only be called in runtime to alter current state of previously launched module.
If you would like this to be clarified in documentation, you can open a separate issue from the documentation page, Feedback > This page, so that PM can review your request and we can make docs better.
I am closing this issue for now since the original problem seem to be resolved, but feel free to contact me here.

@guperrot
Copy link
Member

guperrot commented Jul 9, 2019

We will need to update the docs for how to use those APIs.

As for some context, Distribute.setEnabledForDebuggableBuild didn't exist at first. The original specification was to make the SDK convenient to use and avoid opening the browser while working on the debug build since you are using Android Studio to code and are not distributing so the browser would be disruptive. The original concept of the SDK was to avoid as much configuration as possible so we opted to check for debug automatically.

Then recently we had the community contribution Distribute.setEnabledForDebuggableBuild to disable this automatic check, not changing the default behavior to remain backward compatible.

Though the methods have similar names they are indeed very different in nature and sorry for the confusion, I agree that the docs need to be improved.

As for the original issue it does not seem to be resolved as we are detecting the store automatically and should not have open the browser if installed from Google Play. I'm still curious about that one if you have time to test on an alpha build with debug logging (from AppCenter SDK) enabled.

@luboganev
Copy link
Author

Hi,

Thank you for the detailed information. I understand the big picture now. The goals of the SDK make sense, but most of the times one needs to do complete configuration themselves. I guess that is something we are used to, that's why I expected I need to do the configuration.

About the original issue, I haven't installed the app straight from Google Play. I just tested locally the APK we have prepared for the release by sideloading it on our test devices. We don't use the Alpha and Beta channels of Google Play, that's what we used Hockey and now AppCenter for. So I'm certain that the behavior when installing from Google Play works the way you describe it. Unfortunately I don't have time now to test it but it is good to know that the SDK behaves is like that. However, the fact I had no clue about that shows that maybe this nice feature should be highlighted a bit more in the documentation as well.

@guperrot
Copy link
Member

Ok, so I was confused all along by the fact that you said that opening the app from Google Play opened the browser, but actually it was sideloaded. Then it's expected behavior to open browser if not installed from Google Play and then no need for extra testing.

So basically we need to clarify the documentation.

@Jamminroot
Copy link
Contributor

Small update here - docs are updated now, and info about those 2 added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants