particle-iot / spark-setup-android

Former home of the Particle Device Setup library for Android
Apache License 2.0
23 stars 30 forks source link

Issue with spark login when compiling the library with ProGuard enabled in release config #41

Closed MicheleAuthometion closed 7 years ago

MicheleAuthometion commented 7 years ago

Hello,

I am developing an Android application which interacts with a Photon board for a prototype project, in several ways that I will not mention here since they are not relevant to the problem I am experiencing.

The point is that the Photon's device ID is required at several stages of this interaction, so I have integrated your "Device Setup Library" inside my app as a locally compiled library.

In a first development phase, I was deploying the app to my smartphone in debug mode, and everything was working fine: I could reach the "Setup complete" screen and receive the Photon's device ID through an appropriate BroadcastReceiver in my starting Activity. So far so good.

Then I switched to a release configuration with the following gradle code:

release {
    shrinkResources true
    minifyEnabled true
    proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'
    signingConfig signingConfigs.config
}

Just by doing this, the release app would not be able to proceed to the "device discovery" screen anymore. After some fiddling with the code (since it is not particularly practical to debug an app in release mode), I could realize that the problem was arising from a sparkCloud.logIn() function call (in GetReadyActivity) which does not lead to the onSuccess() method, but rather to the onFailure() one.

If I disable ProGuard (setting minifyEnabled to false), which I do NOT want to do for a release app, everything works just as fine as in the debug app scenario. This is puzzling me a lot as I really do not understand how ProGuard optimizations could cause such a thing as a logIn() function call to fail.

More interestingly, if I try-catch the sparkCloud.logIn() call, the onSuccess() method DOES actually get called! But soon after, the same problem arises as it gets stuck in the onFailure() callback of the sparkCloud.generateClaimCode() method. So still no solution after all.

Could you please provide any solution to avoid this problem without having to disable the ProGuard optimizations?

CityVibes commented 7 years ago

Try adding these to your proguard rules file:

-dontwarn rx.**
-dontwarn java.lang.invoke**
-dontwarn com.google.appengine.api.**
-dontwarn retrofit.**
-keep class retrofit.** { *; }
MicheleAuthometion commented 7 years ago

EDIT: Sorry if I marked it as solved, but actually I had not realised that I still had the minifyEnabled set to false somehow. That's why I thought it worked.

Thanks for pointing out that the retrofit classes should be kept, Julius, but unfortunately that did not fix the issue, and I still experience the same behaviour as before.

CityVibes commented 7 years ago

Could you provide more information: 1) Did you test it on multiple devices and Android versions? 2) Do you have logs, onFailure() should be logging an exception.

MicheleAuthometion commented 7 years ago

Sure, here you are:

  1. Yes, this behaviour was experienced on 2 different devices with 2 different Android versions (4.2.2 and 5.1) so far, in exactly the same way. Anyway it really seems to be related to ProGuard somehow, as I experience it only with minifyEnabled set to true, and never with minifyEnabled set to false (regardless of other factors like network conditions, etc., which are obviously kept the same in both the two test scenarios).

I am starting to wonder whether the Particle store app has minifyEnabled set to false as well, since I think that if the code in the Device Setup Library is the same as that used for the Particle store app, the same problem should arise when using that app as well, but it does not.

  1. Yes, this is what onFailure() does:
@Override
public void onFailure(ParticleCloudException error) {
    log.d("onFailed(): " + error.getMessage());
    dismissProgressDialog();
    // FIXME: check specifically for 401 errors
    // and set a better error message?  (Seems like
    // this works fine already...)

    // TODO: notify connection error
}

Now, at first it was giving an empty error message (error.getMessage() -> empty string), so I went into deeper analysis and found out that the ParticleCloudException class has a getBestMessage() function. Using that, it gives the following: "Unknown error communicating with server.". Not very useful anyways, I am afraid.

CityVibes commented 7 years ago

I haven't been able to reproduce bug yet. Particle Tinker app has minifyEnabled set to false. You are using locally compiled library, right? Are you using latest code and did you make any big changes to code? Would it be possible for you to try and use latest library release compile 'io.particle:devicesetup:0.4.1'? Also could you share (if possible) your proguard rules file and locally compiled library?

MicheleAuthometion commented 7 years ago

Sorry, I have forgotten to mention something quite important. I have made big changes to the original library and the original navigation flow quite a lot, and I even forgot about it - actually I did not remember how the original navigation flow was like, and now that I tried to see what happens compiling the latest version I remembered.

What I have done is basically the following: get rid of the LoginActivity completely, perform login directly through code inside GetReadyActivity using the same call that is done eventually by LoginActivity:

loginTask = Async.executeAsync(sparkCloud, new Async.ApiWork<ParticleCloud, Void>() {
    @Override
    public Void callApi(@NonNull ParticleCloud sparkCloud) throws ParticleCloudException {
        sparkCloud.logIn(email, password);
        return null;
    }

    @Override
    public void onTaskFinished() {
        loginTask = null;
    }

    @Override
    public void onSuccess(@NonNull Void result) {
        SEGAnalytics.identify(email);
        SEGAnalytics.track("Auth: Login success");
        log.d("Logged in...");
        if (isFinishing()) {
            return;
        }
        startActivity(NextActivitySelector.getNextActivityIntent(
                LoginActivity.this,
                sparkCloud,
                SDKGlobals.getSensitiveDataStorage(),
                null));
        finish();
    }

    @Override
    public void onFailure(@NonNull ParticleCloudException error) {
        log.d("onFailed(): " + error.getMessage());
        SEGAnalytics.track("Auth: Login failure");
        ParticleUi.showParticleButtonProgress(LoginActivity.this,
                R.id.action_log_in, false);
        // FIXME: check specifically for 401 errors
        // and set a better error message?  (Seems like
        // this works fine already...)
        passwordView.setError(error.getBestMessage());
        passwordView.requestFocus();
    }
});

In this code (taken from latest version, but still practically the same as the one in 0.3.6 which is the version of the library I am using), I simply hard-coded the "email" and "password" fields. Then in onSuccess() (which, I repeat, gets ALWAYS reached when I build with minifyEnabled set to false) instead of changing activity, since I am already in GetReadyActivity and I have just correctly logged in, I do directly:

@Override
public void onSuccess(Void result) {
    log.d("Logged in...");
    onReadyButtonClicked(false);
}

And that's the difference basically. Essentially that I skip user login screen as it is not something useful to have in my app, and then when the login call succeeds I click the "READY" button programmatically (the user does not even see this happening of course).

Finally, as for the test you asked me to do, with the latest version and with the unmodified navigation flow it does work, but:

  1. It is not a matter of version. With the unmodified navigation flow version 0.3.6 of the library works too. (I tested that too, just to be sure that I did not have to update to the latest version at this point of the project to solve the issue, which is something that would be pretty troublesome)

  2. That customized navigation flow is of course something I can not turn down.

EDIT: Sorry, forgot about the ProGuard file. It basically contains only what we mentioned before:

-dontwarn com.google.common.**
-dontwarn okio.**
-dontwarn retrofit.**
-keep class retrofit.** { *; }

This is because it takes the default ProGuard file:

proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro'

( https://android.googlesource.com/platform/sdk/+/master/files/proguard-android-optimize.txt )

And, before you ask, I have tried also with the other default ProGuard file, proguard-android.txt ( https://android.googlesource.com/platform/sdk/+/master/files/proguard-android.txt ), to no avail.

CityVibes commented 7 years ago

If unmodified versions are working with proguard then there isn't much I can do. You could try to write Proguard rules excluding any code from particle libraries thus minifying your app only.