Skip to content

Commit eb32c9c

Browse files
authored
Error handling revamp for the project management API (#367)
* Error handling revamp for the project management API * Minor code and test cleanup * Fixed some lint errors; Removed requestFactory reference from project mgt service impl
1 parent 5d9ae69 commit eb32c9c

File tree

8 files changed

+253
-164
lines changed

8 files changed

+253
-164
lines changed

src/main/java/com/google/firebase/internal/ErrorHandlingHttpClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public <V> V parse(IncomingHttpResponse response, Class<V> responseType) throws
116116
}
117117
}
118118

119-
private void parse(IncomingHttpResponse response, Object destination) throws T {
119+
public void parse(IncomingHttpResponse response, Object destination) throws T {
120120
try {
121121
JsonParser parser = jsonFactory.createJsonParser(response.getContent());
122122
parser.parse(destination);

src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementException.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,27 @@
1515

1616
package com.google.firebase.projectmanagement;
1717

18+
import com.google.firebase.ErrorCode;
1819
import com.google.firebase.FirebaseException;
19-
import com.google.firebase.internal.Nullable;
20+
import com.google.firebase.IncomingHttpResponse;
21+
import com.google.firebase.database.annotations.Nullable;
22+
import com.google.firebase.internal.NonNull;
2023

2124
/**
2225
* An exception encountered while interacting with the Firebase Project Management Service.
2326
*/
24-
public class FirebaseProjectManagementException extends FirebaseException {
25-
FirebaseProjectManagementException(String detailMessage) {
26-
this(detailMessage, null);
27+
public final class FirebaseProjectManagementException extends FirebaseException {
28+
29+
FirebaseProjectManagementException(@NonNull FirebaseException base) {
30+
this(base, base.getMessage());
31+
}
32+
33+
FirebaseProjectManagementException(@NonNull FirebaseException base, @NonNull String message) {
34+
super(base.getErrorCodeNew(), message, base.getCause(), base.getHttpResponse());
2735
}
2836

29-
FirebaseProjectManagementException(String detailMessage, @Nullable Throwable cause) {
30-
super(detailMessage, cause);
37+
FirebaseProjectManagementException(
38+
@NonNull ErrorCode code, @NonNull String message, @Nullable IncomingHttpResponse response) {
39+
super(code, message, null, response);
3140
}
3241
}

src/main/java/com/google/firebase/projectmanagement/FirebaseProjectManagementServiceImpl.java

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@
3131
import com.google.common.base.Strings;
3232
import com.google.common.collect.ImmutableList;
3333
import com.google.common.collect.ImmutableMap;
34+
import com.google.firebase.ErrorCode;
3435
import com.google.firebase.FirebaseApp;
3536
import com.google.firebase.ImplFirebaseTrampolines;
37+
import com.google.firebase.IncomingHttpResponse;
3638
import com.google.firebase.internal.ApiClientUtils;
3739
import com.google.firebase.internal.CallableOperation;
3840
import java.nio.charset.StandardCharsets;
@@ -56,7 +58,6 @@ class FirebaseProjectManagementServiceImpl implements AndroidAppService, IosAppS
5658
private final FirebaseApp app;
5759
private final Sleeper sleeper;
5860
private final Scheduler scheduler;
59-
private final HttpRequestFactory requestFactory;
6061
private final HttpHelper httpHelper;
6162

6263
private final CreateAndroidAppFromAppIdFunction createAndroidAppFromAppIdFunction =
@@ -78,15 +79,9 @@ class FirebaseProjectManagementServiceImpl implements AndroidAppService, IosAppS
7879
this.app = checkNotNull(app);
7980
this.sleeper = checkNotNull(sleeper);
8081
this.scheduler = checkNotNull(scheduler);
81-
this.requestFactory = checkNotNull(requestFactory);
8282
this.httpHelper = new HttpHelper(app.getOptions().getJsonFactory(), requestFactory);
8383
}
8484

85-
@VisibleForTesting
86-
HttpRequestFactory getRequestFactory() {
87-
return requestFactory;
88-
}
89-
9085
@VisibleForTesting
9186
void setInterceptor(HttpResponseInterceptor interceptor) {
9287
httpHelper.setInterceptor(interceptor);
@@ -318,14 +313,14 @@ protected String execute() throws FirebaseProjectManagementException {
318313
payloadBuilder.put("display_name", displayName);
319314
}
320315
OperationResponse operationResponseInstance = new OperationResponse();
321-
httpHelper.makePostRequest(
316+
IncomingHttpResponse response = httpHelper.makePostRequest(
322317
url, payloadBuilder.build(), operationResponseInstance, projectId, "Project ID");
323318
if (Strings.isNullOrEmpty(operationResponseInstance.name)) {
324-
throw HttpHelper.createFirebaseProjectManagementException(
319+
String message = buildMessage(
325320
namespace,
326321
"Bundle ID",
327-
"Unable to create App: server returned null operation name.",
328-
/* cause= */ null);
322+
"Unable to create App: server returned null operation name.");
323+
throw new FirebaseProjectManagementException(ErrorCode.INTERNAL, message, response);
329324
}
330325
return operationResponseInstance.name;
331326
}
@@ -341,27 +336,29 @@ private String pollOperation(String projectId, String operationName)
341336
* Math.pow(POLL_EXPONENTIAL_BACKOFF_FACTOR, currentAttempt));
342337
sleepOrThrow(projectId, delayMillis);
343338
OperationResponse operationResponseInstance = new OperationResponse();
344-
httpHelper.makeGetRequest(url, operationResponseInstance, projectId, "Project ID");
339+
IncomingHttpResponse response = httpHelper.makeGetRequest(
340+
url, operationResponseInstance, projectId, "Project ID");
345341
if (!operationResponseInstance.done) {
346342
continue;
347343
}
348344
// The Long Running Operation API guarantees that when done == true, exactly one of 'response'
349345
// or 'error' is set.
350346
if (operationResponseInstance.response == null
351347
|| Strings.isNullOrEmpty(operationResponseInstance.response.appId)) {
352-
throw HttpHelper.createFirebaseProjectManagementException(
348+
String message = buildMessage(
353349
projectId,
354350
"Project ID",
355-
"Unable to create App: internal server error.",
356-
/* cause= */ null);
351+
"Unable to create App: internal server error.");
352+
throw new FirebaseProjectManagementException(ErrorCode.INTERNAL, message, response);
357353
}
358354
return operationResponseInstance.response.appId;
359355
}
360-
throw HttpHelper.createFirebaseProjectManagementException(
356+
357+
String message = buildMessage(
361358
projectId,
362359
"Project ID",
363-
"Unable to create App: deadline exceeded.",
364-
/* cause= */ null);
360+
"Unable to create App: deadline exceeded.");
361+
throw new FirebaseProjectManagementException(ErrorCode.DEADLINE_EXCEEDED, message, null);
365362
}
366363

367364
/**
@@ -420,19 +417,22 @@ private WaitOperationRunnable(
420417
public void run() {
421418
String url = String.format("%s/v1/%s", FIREBASE_PROJECT_MANAGEMENT_URL, operationName);
422419
OperationResponse operationResponseInstance = new OperationResponse();
420+
IncomingHttpResponse httpResponse;
423421
try {
424-
httpHelper.makeGetRequest(url, operationResponseInstance, projectId, "Project ID");
422+
httpResponse = httpHelper.makeGetRequest(
423+
url, operationResponseInstance, projectId, "Project ID");
425424
} catch (FirebaseProjectManagementException e) {
426425
settableFuture.setException(e);
427426
return;
428427
}
429428
if (!operationResponseInstance.done) {
430429
if (numberOfPreviousPolls + 1 >= MAXIMUM_POLLING_ATTEMPTS) {
431-
settableFuture.setException(HttpHelper.createFirebaseProjectManagementException(
432-
projectId,
430+
String message = buildMessage(projectId,
433431
"Project ID",
434-
"Unable to create App: deadline exceeded.",
435-
/* cause= */ null));
432+
"Unable to create App: deadline exceeded.");
433+
FirebaseProjectManagementException exception = new FirebaseProjectManagementException(
434+
ErrorCode.DEADLINE_EXCEEDED, message, httpResponse);
435+
settableFuture.setException(exception);
436436
} else {
437437
long delayMillis = (long) (
438438
POLL_BASE_WAIT_TIME_MILLIS
@@ -451,11 +451,12 @@ public void run() {
451451
// or 'error' is set.
452452
if (operationResponseInstance.response == null
453453
|| Strings.isNullOrEmpty(operationResponseInstance.response.appId)) {
454-
settableFuture.setException(HttpHelper.createFirebaseProjectManagementException(
455-
projectId,
454+
String message = buildMessage(projectId,
456455
"Project ID",
457-
"Unable to create App: internal server error.",
458-
/* cause= */ null));
456+
"Unable to create App: internal server error.");
457+
FirebaseProjectManagementException exception = new FirebaseProjectManagementException(
458+
ErrorCode.INTERNAL, message, httpResponse);
459+
settableFuture.setException(exception);
459460
} else {
460461
settableFuture.set(operationResponseInstance.response.appId);
461462
}
@@ -765,14 +766,17 @@ private void sleepOrThrow(String projectId, long delayMillis)
765766
try {
766767
sleeper.sleep(delayMillis);
767768
} catch (InterruptedException e) {
768-
throw HttpHelper.createFirebaseProjectManagementException(
769-
projectId,
769+
String message = buildMessage(projectId,
770770
"Project ID",
771-
"Unable to create App: exponential backoff interrupted.",
772-
/* cause= */ null);
771+
"Unable to create App: exponential backoff interrupted.");
772+
throw new FirebaseProjectManagementException(ErrorCode.ABORTED, message, null);
773773
}
774774
}
775775

776+
private String buildMessage(String resourceId, String resourceIdName, String description) {
777+
return String.format("%s \"%s\": %s", resourceIdName, resourceId, description);
778+
}
779+
776780
/* Helper types. */
777781

778782
private interface CreateAppFromAppIdFunction<T> extends ApiFunction<String, T> {}

0 commit comments

Comments
 (0)