Skip to content

Commit 9fb2b87

Browse files
lfkellogglfkellogg
authored
Improve error handling around downloading and installing (#3263)
* Refactor tests around notifications to be more focused to the classes under test * Improve error handling around downloading and installing * Import ErrorMessages consistently, rather than use Constants.ErrorMessages * Fix lint errors * Some linting and test cleanup
1 parent 931fb8d commit 9fb2b87

File tree

8 files changed

+373
-289
lines changed

8 files changed

+373
-289
lines changed

‎firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/AabUpdater.java

Lines changed: 69 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.app.distribution;
1616

17+
import static com.google.firebase.app.distribution.FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE;
1718
import static com.google.firebase.app.distribution.FirebaseAppDistributionException.Status.NETWORK_FAILURE;
1819
import static com.google.firebase.app.distribution.TaskUtils.safeSetTaskException;
1920

@@ -23,11 +24,12 @@
2324
import androidx.annotation.GuardedBy;
2425
import androidx.annotation.NonNull;
2526
import androidx.annotation.VisibleForTesting;
27+
import com.google.firebase.app.distribution.Constants.ErrorMessages;
2628
import com.google.firebase.app.distribution.internal.InstallActivity;
2729
import com.google.firebase.app.distribution.internal.LogWrapper;
2830
import com.google.firebase.app.distribution.internal.SignInResultActivity;
2931
import java.io.IOException;
30-
import java.net.URL;
32+
import java.util.concurrent.Executor;
3133
import java.util.concurrent.Executors;
3234
import javax.net.ssl.HttpsURLConnection;
3335

@@ -36,6 +38,8 @@ class AabUpdater {
3638
private static final String TAG = "UpdateAabClient:";
3739

3840
private final FirebaseAppDistributionLifecycleNotifier lifecycleNotifier;
41+
private final HttpsUrlConnectionFactory httpsUrlConnectionFactory;
42+
private final Executor executor;
3943

4044
@GuardedBy("updateAabLock")
4145
private UpdateTaskImpl cachedUpdateTask;
@@ -46,12 +50,20 @@ class AabUpdater {
4650
private final Object updateAabLock = new Object();
4751

4852
AabUpdater() {
49-
this(FirebaseAppDistributionLifecycleNotifier.getInstance());
53+
this(
54+
FirebaseAppDistributionLifecycleNotifier.getInstance(),
55+
new HttpsUrlConnectionFactory(),
56+
Executors.newSingleThreadExecutor());
5057
}
5158

52-
AabUpdater(@NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier) {
59+
AabUpdater(
60+
@NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
61+
@NonNull HttpsUrlConnectionFactory httpsUrlConnectionFactory,
62+
@NonNull Executor executor) {
5363
this.lifecycleNotifier = lifecycleNotifier;
64+
this.httpsUrlConnectionFactory = httpsUrlConnectionFactory;
5465
lifecycleNotifier.addOnActivityStartedListener(this::onActivityStarted);
66+
this.executor = executor;
5567
}
5668

5769
@VisibleForTesting
@@ -78,77 +90,78 @@ UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
7890
}
7991
}
8092

81-
HttpsURLConnection openHttpsUrlConnection(String downloadUrl)
93+
private String fetchDownloadRedirectUrl(String downloadUrl)
8294
throws FirebaseAppDistributionException {
8395
HttpsURLConnection httpsURLConnection;
96+
int responseCode;
8497

8598
try {
86-
URL url = new URL(downloadUrl);
87-
httpsURLConnection = (HttpsURLConnection) url.openConnection();
99+
httpsURLConnection = httpsUrlConnectionFactory.openConnection(downloadUrl);
100+
httpsURLConnection.setInstanceFollowRedirects(false);
101+
responseCode = httpsURLConnection.getResponseCode();
88102
} catch (IOException e) {
89103
throw new FirebaseAppDistributionException(
90-
Constants.ErrorMessages.NETWORK_ERROR, NETWORK_FAILURE, e);
104+
"Failed to open connection to: " + downloadUrl, NETWORK_FAILURE, e);
91105
}
92-
return httpsURLConnection;
106+
107+
if (!isRedirectResponse(responseCode)) {
108+
throw new FirebaseAppDistributionException(
109+
"Expected redirect response code, but got: " + responseCode, DOWNLOAD_FAILURE);
110+
}
111+
String redirect = httpsURLConnection.getHeaderField("Location");
112+
httpsURLConnection.disconnect();
113+
114+
if (redirect == null) {
115+
throw new FirebaseAppDistributionException(
116+
"No Location header found in response from: " + downloadUrl, DOWNLOAD_FAILURE);
117+
} else if (redirect.isEmpty()) {
118+
throw new FirebaseAppDistributionException(
119+
"Empty Location header found in response from: " + downloadUrl, DOWNLOAD_FAILURE);
120+
}
121+
122+
return redirect;
123+
}
124+
125+
private static boolean isRedirectResponse(int responseCode) {
126+
return responseCode >= 300 && responseCode < 400;
93127
}
94128

95129
private void redirectToPlayForAabUpdate(String downloadUrl) {
96130
synchronized (updateAabLock) {
97131
if (lifecycleNotifier.getCurrentActivity() == null) {
98132
safeSetTaskException(
99133
cachedUpdateTask,
100-
new FirebaseAppDistributionException(
101-
Constants.ErrorMessages.APP_BACKGROUNDED,
102-
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
134+
new FirebaseAppDistributionException(ErrorMessages.APP_BACKGROUNDED, DOWNLOAD_FAILURE));
103135
return;
104136
}
105137
}
106138

107139
// The 302 redirect is obtained here to open the play store directly and avoid opening chrome
108-
Executors.newSingleThreadExecutor()
109-
.execute( // Execute the network calls on a background thread
110-
() -> {
111-
HttpsURLConnection connection;
112-
String redirect;
113-
try {
114-
connection = openHttpsUrlConnection(downloadUrl);
115-
116-
// To get url to play without redirect we do this connection
117-
connection.setInstanceFollowRedirects(false);
118-
redirect = connection.getHeaderField("Location");
119-
connection.disconnect();
120-
connection.getInputStream().close();
121-
} catch (FirebaseAppDistributionException | IOException e) {
122-
setUpdateTaskCompletionErrorWithDefault(
123-
e,
124-
new FirebaseAppDistributionException(
125-
Constants.ErrorMessages.NETWORK_ERROR,
126-
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
127-
return;
128-
}
129-
130-
if (!redirect.isEmpty()) {
131-
Intent updateIntent = new Intent(Intent.ACTION_VIEW);
132-
updateIntent.setData(Uri.parse(redirect));
133-
updateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
134-
LogWrapper.getInstance().v(TAG + "Redirecting to play");
135-
136-
synchronized (updateAabLock) {
137-
lifecycleNotifier.getCurrentActivity().startActivity(updateIntent);
138-
cachedUpdateTask.updateProgress(
139-
UpdateProgress.builder()
140-
.setApkBytesDownloaded(-1)
141-
.setApkFileTotalBytes(-1)
142-
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
143-
.build());
144-
}
145-
} else {
146-
setUpdateTaskCompletionError(
147-
new FirebaseAppDistributionException(
148-
Constants.ErrorMessages.NETWORK_ERROR,
149-
FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
150-
}
151-
});
140+
executor.execute( // Execute the network calls on a background thread
141+
() -> {
142+
String redirectUrl;
143+
try {
144+
redirectUrl = fetchDownloadRedirectUrl(downloadUrl);
145+
} catch (FirebaseAppDistributionException e) {
146+
setUpdateTaskCompletionError(e);
147+
return;
148+
}
149+
150+
Intent updateIntent = new Intent(Intent.ACTION_VIEW);
151+
updateIntent.setData(Uri.parse(redirectUrl));
152+
updateIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
153+
LogWrapper.getInstance().v(TAG + "Redirecting to play");
154+
155+
synchronized (updateAabLock) {
156+
lifecycleNotifier.getCurrentActivity().startActivity(updateIntent);
157+
cachedUpdateTask.updateProgress(
158+
UpdateProgress.builder()
159+
.setApkBytesDownloaded(-1)
160+
.setApkFileTotalBytes(-1)
161+
.setUpdateStatus(UpdateStatus.REDIRECTED_TO_PLAY)
162+
.build());
163+
}
164+
});
152165
}
153166

154167
private void setUpdateTaskCompletionError(FirebaseAppDistributionException e) {
@@ -157,21 +170,12 @@ private void setUpdateTaskCompletionError(FirebaseAppDistributionException e) {
157170
}
158171
}
159172

160-
private void setUpdateTaskCompletionErrorWithDefault(
161-
Exception e, FirebaseAppDistributionException defaultFirebaseException) {
162-
if (e instanceof FirebaseAppDistributionException) {
163-
setUpdateTaskCompletionError((FirebaseAppDistributionException) e);
164-
} else {
165-
setUpdateTaskCompletionError(defaultFirebaseException);
166-
}
167-
}
168-
169173
void tryCancelAabUpdateTask() {
170174
synchronized (updateAabLock) {
171175
safeSetTaskException(
172176
cachedUpdateTask,
173177
new FirebaseAppDistributionException(
174-
Constants.ErrorMessages.UPDATE_CANCELED,
178+
ErrorMessages.UPDATE_CANCELED,
175179
FirebaseAppDistributionException.Status.INSTALLATION_CANCELED,
176180
ReleaseUtils.convertToAppDistributionRelease(aabReleaseInProgress)));
177181
}

0 commit comments

Comments
 (0)