Skip to content

Commit f2fed45

Browse files
annzimmerannzimmer
committed
adding more FirebaseMlException handling and logging
1 parent c8da8b0 commit f2fed45

File tree

10 files changed

+328
-56
lines changed

10 files changed

+328
-56
lines changed

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloader.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public class FirebaseModelDownloader {
5959
this.eventLogger = FirebaseMlLogger.getInstance();
6060
this.fileDownloadService = new ModelFileDownloadService(firebaseApp, transportFactory);
6161
this.modelDownloadService =
62-
new CustomModelDownloadService(firebaseApp, firebaseInstallationsApi, transportFactory);
62+
new CustomModelDownloadService(firebaseApp, firebaseInstallationsApi);
6363

6464
this.executor = Executors.newSingleThreadExecutor();
6565
fileManager = ModelFileManager.getInstance();
@@ -273,7 +273,7 @@ private Task<CustomModel> getCustomModelTask(
273273
return getCompletedLocalCustomModelTask(updatedModel);
274274
}
275275
// clean up model internally
276-
deleteModelDetails(currentModel.getName());
276+
deleteModelDetails(modelName);
277277
return Tasks.forException(
278278
new FirebaseMlException(
279279
"Possible caching issues: no model associated with " + modelName + ".",
@@ -369,13 +369,18 @@ private Task<CustomModel> retryExpiredUrlDownload(
369369
modelName, conditions, downloadTask, retryCounter - 1);
370370
}
371371
return Tasks.forException(
372-
new Exception("File download failed. Too many attempts."));
372+
new FirebaseMlException(
373+
"File download failed after multiple attempts, possible expired url.",
374+
FirebaseMlException.DOWNLOAD_URL_EXPIRED));
373375
});
374376
}
375377
return Tasks.forException(retryModelDetailTask.getException());
376378
});
379+
} else if (downloadTask.getException() instanceof FirebaseMlException) {
380+
return Tasks.forException(downloadTask.getException());
377381
}
378-
return Tasks.forException(new Exception("File download failed."));
382+
return Tasks.forException(
383+
new FirebaseMlException("File download failed.", FirebaseMlException.INTERNAL));
379384
}
380385

381386
private Task<CustomModel> finishModelDownload(@NonNull String modelName) {

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloaderRegistrar.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,11 @@ public List<Component<?>> getComponents() {
8585
.build(),
8686
Component.builder(CustomModelDownloadService.class)
8787
.add(Dependency.required(FirebaseApp.class))
88-
.add(Dependency.required(TransportFactory.class))
8988
.add(Dependency.required(FirebaseInstallationsApi.class))
9089
.factory(
9190
c ->
9291
new CustomModelDownloadService(
93-
c.get(FirebaseApp.class),
94-
c.get(FirebaseInstallationsApi.class),
95-
c.get(TransportFactory.class)))
92+
c.get(FirebaseApp.class), c.get(FirebaseInstallationsApi.class)))
9693
.build(),
9794
LibraryVersionComponent.create("firebase-ml-modeldownloader", BuildConfig.VERSION_NAME));
9895
}

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/CustomModelDownloadService.java

Lines changed: 92 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import android.util.JsonReader;
1818
import android.util.Log;
1919
import androidx.annotation.NonNull;
20-
import com.google.android.datatransport.TransportFactory;
2120
import com.google.android.gms.common.util.VisibleForTesting;
2221
import com.google.android.gms.tasks.Task;
2322
import com.google.android.gms.tasks.Tasks;
@@ -26,6 +25,7 @@
2625
import com.google.firebase.installations.InstallationTokenResult;
2726
import com.google.firebase.ml.modeldownloader.CustomModel;
2827
import com.google.firebase.ml.modeldownloader.FirebaseMlException;
28+
import com.google.firebase.ml.modeldownloader.FirebaseMlException.Code;
2929
import com.google.firebase.ml.modeldownloader.internal.FirebaseMlLogEvent.ModelDownloadLogEvent.ErrorCode;
3030
import java.io.BufferedReader;
3131
import java.io.IOException;
@@ -80,9 +80,7 @@ public class CustomModelDownloadService {
8080
private String downloadHost = FIREBASE_DOWNLOAD_HOST;
8181

8282
public CustomModelDownloadService(
83-
FirebaseApp firebaseApp,
84-
FirebaseInstallationsApi installationsApi,
85-
TransportFactory transportFactory) {
83+
FirebaseApp firebaseApp, FirebaseInstallationsApi installationsApi) {
8684
firebaseInstallations = installationsApi;
8785
apiKey = firebaseApp.getOptions().getApiKey();
8886
executorService = Executors.newCachedThreadPool();
@@ -133,7 +131,6 @@ public Task<CustomModel> getCustomModelDetails(
133131
try {
134132
URL url =
135133
new URL(String.format(DOWNLOAD_MODEL_REGEX, downloadHost, projectNumber, modelName));
136-
137134
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
138135
connection.setConnectTimeout(CONNECTION_TIME_OUT_MS);
139136
connection.setRequestProperty(ACCEPT_ENCODING_HEADER_KEY, GZIP_CONTENT_ENCODING);
@@ -171,10 +168,16 @@ public Task<CustomModel> getCustomModelDetails(
171168
return fetchDownloadDetails(modelName, connection);
172169
});
173170

174-
} catch (Exception e) {
175-
// TODO(annz) update to better error handling (use FirebaseMLExceptions)
171+
} catch (IOException e) {
172+
eventLogger.logDownloadFailureWithReason(
173+
new CustomModel(modelName, modelHash, 0, 0L),
174+
false,
175+
ErrorCode.MODEL_INFO_DOWNLOAD_CONNECTION_FAILED.getValue());
176+
176177
return Tasks.forException(
177-
new Exception("Error reading custom model from download service: " + e.getMessage(), e));
178+
new FirebaseMlException(
179+
"Error reading custom model from download service: " + e.getMessage(),
180+
FirebaseMlException.INVALID_ARGUMENT));
178181
}
179182
}
180183

@@ -196,29 +199,89 @@ static long parseTokenExpirationTimestamp(String expiresIn) {
196199
}
197200
}
198201

199-
private Task<CustomModel> fetchDownloadDetails(String modelName, HttpURLConnection connection)
200-
throws Exception {
201-
// todo(annz) add try catch for IOException specific errors.
202-
connection.connect();
203-
int httpResponseCode = connection.getResponseCode();
204-
205-
if (httpResponseCode == HttpURLConnection.HTTP_NOT_MODIFIED) {
206-
return Tasks.forResult(null);
207-
} else if (httpResponseCode == HttpURLConnection.HTTP_OK) {
208-
return Tasks.forResult(readCustomModelResponse(modelName, connection));
202+
private Task<CustomModel> fetchDownloadDetails(String modelName, HttpURLConnection connection) {
203+
try {
204+
connection.connect();
205+
206+
int httpResponseCode = connection.getResponseCode();
207+
String errorMessage = getErrorStream(connection);
208+
209+
switch (httpResponseCode) {
210+
case HttpURLConnection.HTTP_OK:
211+
return Tasks.forResult(readCustomModelResponse(modelName, connection));
212+
case HttpURLConnection.HTTP_NOT_MODIFIED:
213+
return Tasks.forResult(null);
214+
case HttpURLConnection.HTTP_NOT_FOUND:
215+
return Tasks.forException(
216+
new FirebaseMlException(
217+
String.format(Locale.getDefault(), "No model found with name: %s", modelName),
218+
FirebaseMlException.NOT_FOUND));
219+
case HttpURLConnection.HTTP_BAD_REQUEST:
220+
return setAndLogException(
221+
modelName,
222+
httpResponseCode,
223+
errorMessage,
224+
String.format(
225+
Locale.getDefault(),
226+
"Bad http request for model (%s); error message: %s",
227+
modelName,
228+
errorMessage),
229+
FirebaseMlException.INVALID_ARGUMENT);
230+
case 429: // too many requests
231+
return setAndLogException(
232+
modelName,
233+
httpResponseCode,
234+
errorMessage,
235+
"Too many requests to server please wait before trying again.",
236+
FirebaseMlException.INVALID_ARGUMENT);
237+
case HttpURLConnection.HTTP_SERVER_ERROR:
238+
return setAndLogException(
239+
modelName,
240+
httpResponseCode,
241+
errorMessage,
242+
String.format(
243+
Locale.getDefault(),
244+
"Server issue while fetching model (%s); error message: %s",
245+
modelName,
246+
errorMessage),
247+
FirebaseMlException.INTERNAL);
248+
default:
249+
return setAndLogException(
250+
modelName,
251+
httpResponseCode,
252+
errorMessage,
253+
String.format(
254+
Locale.getDefault(),
255+
"Failed to connect to Firebase ML download server with HTTP status code: %d"
256+
+ " and error message: %s",
257+
httpResponseCode,
258+
errorMessage),
259+
FirebaseMlException.INTERNAL);
260+
}
261+
} catch (IOException e) {
262+
ErrorCode errorCode = ErrorCode.MODEL_INFO_DOWNLOAD_CONNECTION_FAILED;
263+
String errorMessage = "Failed to get model URL";
264+
if (e instanceof UnknownHostException) {
265+
errorCode = ErrorCode.NO_NETWORK_CONNECTION;
266+
errorMessage = "Failed to retrieve model info due to no internet connection.";
267+
}
268+
eventLogger.logModelInfoRetrieverFailure(new CustomModel(modelName, "", 0, 0), errorCode);
269+
return Tasks.forException(
270+
new FirebaseMlException(errorMessage, FirebaseMlException.INTERNAL));
209271
}
272+
}
210273

211-
String errorMessage = getErrorStream(connection);
212-
213-
// todo(annz) add more specific error handling. NOT_FOUND, etc.
214-
return Tasks.forException(
215-
new Exception(
216-
String.format(
217-
Locale.getDefault(),
218-
"Failed to connect to Firebase ML download server with HTTP status code: %d"
219-
+ " and error message: %s",
220-
connection.getResponseCode(),
221-
errorMessage)));
274+
private Task<CustomModel> setAndLogException(
275+
String modelName,
276+
int httpResponseCode,
277+
String errorMessage,
278+
String s,
279+
@Code int invalidArgument) {
280+
eventLogger.logModelInfoRetrieverFailure(
281+
new CustomModel(modelName, "", 0, 0),
282+
ErrorCode.MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS,
283+
httpResponseCode);
284+
return Tasks.forException(new FirebaseMlException(errorMessage, invalidArgument));
222285
}
223286

224287
private CustomModel readCustomModelResponse(

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/FirebaseMlLogEvent.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,10 @@ public static Builder builder() {
143143
// range of error codes is 100 to 199.
144144
public enum ErrorCode {
145145
NO_ERROR(0),
146+
TIME_OUT_FETCHING_MODEL_METADATA(5),
146147
NO_NETWORK_CONNECTION(102),
147148
DOWNLOAD_FAILED(104),
149+
MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS(105),
148150
MODEL_INFO_DOWNLOAD_CONNECTION_FAILED(107),
149151
UNKNOWN_ERROR(9999);
150152
private static final SparseArray<ErrorCode> valueMap = new SparseArray<>();
@@ -153,8 +155,10 @@ public enum ErrorCode {
153155

154156
static {
155157
valueMap.put(0, NO_ERROR);
158+
valueMap.put(5, TIME_OUT_FETCHING_MODEL_METADATA);
156159
valueMap.put(102, NO_NETWORK_CONNECTION);
157160
valueMap.put(104, DOWNLOAD_FAILED);
161+
valueMap.put(105, MODEL_INFO_DOWNLOAD_UNSUCCESSFUL_HTTP_STATUS);
158162
valueMap.put(107, MODEL_INFO_DOWNLOAD_CONNECTION_FAILED);
159163
valueMap.put(9999, UNKNOWN_ERROR);
160164
}

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/FirebaseMlLogger.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,20 @@ public static FirebaseMlLogger getInstance() {
9191
return FirebaseApp.getInstance().get(FirebaseMlLogger.class);
9292
}
9393

94+
void logModelInfoRetrieverFailure(CustomModel model, ErrorCode errorCode) {
95+
logModelInfoRetrieverFailure(model, errorCode, NO_FAILURE_VALUE);
96+
}
97+
98+
void logModelInfoRetrieverFailure(CustomModel model, ErrorCode errorCode, int httpResponseCode) {
99+
logDownloadEvent(
100+
model,
101+
errorCode,
102+
false,
103+
/* shouldLogExactDownloadTime= */ false,
104+
DownloadStatus.MODEL_INFO_RETRIEVAL_FAILED,
105+
httpResponseCode);
106+
}
107+
94108
public void logDownloadEventWithExactDownloadTime(
95109
@NonNull CustomModel customModel, ErrorCode errorCode, DownloadStatus status) {
96110
logDownloadEvent(

‎firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/ModelFileDownloadService.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,8 @@ public void onReceive(Context context, Intent intent) {
563563
eventLogger.logDownloadFailureWithReason(
564564
downloadingModel, false, FirebaseMlLogger.NO_FAILURE_VALUE);
565565
}
566-
taskCompletionSource.setException(new Exception("Model downloading failed"));
566+
taskCompletionSource.setException(
567+
new FirebaseMlException("Model downloading failed", FirebaseMlException.INTERNAL));
567568
}
568569

569570
private boolean checkErrorCausedByExpiry(long downloadUrlExpiry, int failureReason) {

‎firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/FirebaseModelDownloaderTest.java

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.truth.Truth.assertThat;
1818
import static org.junit.Assert.assertEquals;
1919
import static org.junit.Assert.assertTrue;
20+
import static org.junit.Assert.fail;
2021
import static org.mockito.ArgumentMatchers.any;
2122
import static org.mockito.ArgumentMatchers.anyBoolean;
2223
import static org.mockito.ArgumentMatchers.anyInt;
@@ -445,8 +446,11 @@ public void getModel_latestModel_noLocalModel_error() throws Exception {
445446
task.addOnCompleteListener(executor, onCompleteListener);
446447
try {
447448
onCompleteListener.await();
448-
} catch (Exception ex) {
449+
} catch (FirebaseMlException ex) {
450+
assertEquals(ex.getCode(), FirebaseMlException.INTERNAL);
449451
assertThat(ex.getMessage().contains("download failed")).isTrue();
452+
} catch (Exception ex) {
453+
fail("Unexpected error message: " + ex.getMessage());
450454
}
451455

452456
verify(mockPrefs, times(2)).getCustomModelDetails(eq(MODEL_NAME));
@@ -539,7 +543,7 @@ public void getModel_updateBackground_noLocalModel() throws Exception {
539543
}
540544

541545
@Test
542-
public void getModel_updateBackground_noLocalModel_error() throws Exception {
546+
public void getModel_updateBackground_noLocalModel_error() {
543547
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
544548
.thenReturn(null)
545549
.thenReturn(null)
@@ -556,8 +560,11 @@ public void getModel_updateBackground_noLocalModel_error() throws Exception {
556560
task.addOnCompleteListener(executor, onCompleteListener);
557561
try {
558562
onCompleteListener.await();
559-
} catch (Exception ex) {
563+
} catch (FirebaseMlException ex) {
564+
assertEquals(ex.getCode(), FirebaseMlException.INTERNAL);
560565
assertThat(ex.getMessage().contains("download failed")).isTrue();
566+
} catch (Exception ex) {
567+
fail("Unexpected error message: " + ex.getMessage());
561568
}
562569

563570
verify(mockPrefs, times(2)).getCustomModelDetails(eq(MODEL_NAME));
@@ -654,9 +661,11 @@ public void getModel_local_noLocalModel_urlRetry_maxTries() {
654661
task.addOnCompleteListener(executor, onCompleteListener);
655662
try {
656663
onCompleteListener.await();
664+
} catch (FirebaseMlException ex) {
665+
assertEquals(ex.getCode(), FirebaseMlException.DOWNLOAD_URL_EXPIRED);
666+
assertThat(ex.getMessage().contains("multiple attempts")).isTrue();
657667
} catch (Exception ex) {
658-
assertThat(ex.getMessage().contains("download failed")).isTrue();
659-
assertThat(ex.getMessage().contains("Too many attempts")).isTrue();
668+
fail("Unexpected error message: " + ex.getMessage());
660669
}
661670

662671
verify(mockPrefs, times(2)).getCustomModelDetails(eq(MODEL_NAME));
@@ -667,7 +676,7 @@ public void getModel_local_noLocalModel_urlRetry_maxTries() {
667676
}
668677

669678
@Test
670-
public void getModel_local_noLocalModel_error() throws Exception {
679+
public void getModel_local_noLocalModel_error() {
671680
when(mockPrefs.getCustomModelDetails(eq(MODEL_NAME)))
672681
.thenReturn(null)
673682
.thenReturn(null)
@@ -676,15 +685,20 @@ public void getModel_local_noLocalModel_error() throws Exception {
676685
eq(TEST_PROJECT_ID), eq(MODEL_NAME), eq(null)))
677686
.thenReturn(Tasks.forResult(ORIG_CUSTOM_MODEL_URL));
678687
when(mockFileDownloadService.download(any(), eq(DOWNLOAD_CONDITIONS)))
679-
.thenReturn(Tasks.forException(new Exception("bad download")));
688+
.thenReturn(
689+
Tasks.forException(
690+
new FirebaseMlException("bad download", FirebaseMlException.INVALID_ARGUMENT)));
680691
TestOnCompleteListener<CustomModel> onCompleteListener = new TestOnCompleteListener<>();
681692
Task<CustomModel> task =
682693
firebaseModelDownloader.getModel(MODEL_NAME, DownloadType.LOCAL_MODEL, DOWNLOAD_CONDITIONS);
683694
task.addOnCompleteListener(executor, onCompleteListener);
684695
try {
685696
onCompleteListener.await();
697+
} catch (FirebaseMlException ex) {
698+
assertEquals(ex.getCode(), FirebaseMlException.INVALID_ARGUMENT);
699+
assertThat(ex.getMessage().contains("bad download")).isTrue();
686700
} catch (Exception ex) {
687-
assertThat(ex.getMessage().contains("download failed")).isTrue();
701+
fail("Unexpected error message: " + ex.getMessage());
688702
}
689703

690704
verify(mockPrefs, times(2)).getCustomModelDetails(eq(MODEL_NAME));

‎firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/TestOnCompleteListener.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,8 @@ public TResult await() throws Exception {
5959
if (exception instanceof IOException) {
6060
throw new ExecutionException(exception);
6161
}
62-
// TODO(annz) replace with firebase ml exception handling.
63-
if (exception instanceof Exception) {
64-
throw exception;
62+
if (exception instanceof FirebaseMlException) {
63+
throw (FirebaseMlException) exception;
6564
}
6665
throw new IllegalStateException("got an unexpected exception type", exception);
6766
}

0 commit comments

Comments
 (0)