Skip to content

Commit c104db6

Browse files
hiranya911hiranya911
authored
Error handling revamp for FirebaseInstanceId API (#359)
* Delete instance ID API error handling revamp * Added tests for IO and parse errors
1 parent 34a6b0a commit c104db6

File tree

4 files changed

+186
-86
lines changed

4 files changed

+186
-86
lines changed

‎src/main/java/com/google/firebase/iid/FirebaseInstanceId.java

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,27 @@
1717
package com.google.firebase.iid;
1818

1919
import static com.google.common.base.Preconditions.checkArgument;
20+
import static com.google.common.base.Preconditions.checkNotNull;
2021

21-
import com.google.api.client.http.GenericUrl;
22-
import com.google.api.client.http.HttpRequest;
2322
import com.google.api.client.http.HttpRequestFactory;
24-
import com.google.api.client.http.HttpResponse;
25-
import com.google.api.client.http.HttpResponseException;
2623
import com.google.api.client.http.HttpResponseInterceptor;
27-
import com.google.api.client.http.HttpTransport;
28-
import com.google.api.client.json.JsonFactory;
29-
import com.google.api.client.json.JsonObjectParser;
3024
import com.google.api.core.ApiFuture;
3125
import com.google.common.annotations.VisibleForTesting;
3226
import com.google.common.base.Strings;
3327
import com.google.common.collect.ImmutableMap;
34-
import com.google.common.io.ByteStreams;
3528
import com.google.firebase.FirebaseApp;
29+
import com.google.firebase.FirebaseException;
3630
import com.google.firebase.ImplFirebaseTrampolines;
31+
import com.google.firebase.IncomingHttpResponse;
32+
import com.google.firebase.database.annotations.Nullable;
33+
import com.google.firebase.internal.AbstractHttpErrorHandler;
34+
import com.google.firebase.internal.ApiClientUtils;
3735
import com.google.firebase.internal.CallableOperation;
38-
import com.google.firebase.internal.FirebaseRequestInitializer;
36+
import com.google.firebase.internal.ErrorHandlingHttpClient;
3937
import com.google.firebase.internal.FirebaseService;
38+
import com.google.firebase.internal.HttpRequestInfo;
4039
import com.google.firebase.internal.NonNull;
4140

42-
import java.io.IOException;
4341
import java.util.Map;
4442

4543
/**
@@ -64,22 +62,32 @@ public class FirebaseInstanceId {
6462
.build();
6563

6664
private final FirebaseApp app;
67-
private final HttpRequestFactory requestFactory;
68-
private final JsonFactory jsonFactory;
6965
private final String projectId;
66+
private final ErrorHandlingHttpClient<FirebaseInstanceIdException> httpClient;
7067

7168
private HttpResponseInterceptor interceptor;
7269

7370
private FirebaseInstanceId(FirebaseApp app) {
74-
HttpTransport httpTransport = app.getOptions().getHttpTransport();
75-
this.app = app;
76-
this.requestFactory = httpTransport.createRequestFactory(new FirebaseRequestInitializer(app));
77-
this.jsonFactory = app.getOptions().getJsonFactory();
78-
this.projectId = ImplFirebaseTrampolines.getProjectId(app);
71+
this(app, null);
72+
}
73+
74+
@VisibleForTesting
75+
FirebaseInstanceId(FirebaseApp app, @Nullable HttpRequestFactory requestFactory) {
76+
this.app = checkNotNull(app, "app must not be null");
77+
String projectId = ImplFirebaseTrampolines.getProjectId(app);
7978
checkArgument(!Strings.isNullOrEmpty(projectId),
8079
"Project ID is required to access instance ID service. Use a service account credential or "
8180
+ "set the project ID explicitly via FirebaseOptions. Alternatively you can also "
8281
+ "set the project ID via the GOOGLE_CLOUD_PROJECT environment variable.");
82+
this.projectId = projectId;
83+
if (requestFactory == null) {
84+
requestFactory = ApiClientUtils.newAuthorizedRequestFactory(app);
85+
}
86+
87+
this.httpClient = new ErrorHandlingHttpClient<>(
88+
requestFactory,
89+
app.getOptions().getJsonFactory(),
90+
new InstanceIdErrorHandler());
8391
}
8492

8593
/**
@@ -146,42 +154,46 @@ private CallableOperation<Void, FirebaseInstanceIdException> deleteInstanceIdOp(
146154
protected Void execute() throws FirebaseInstanceIdException {
147155
String url = String.format(
148156
"%s/project/%s/instanceId/%s", IID_SERVICE_URL, projectId, instanceId);
149-
HttpResponse response = null;
150-
try {
151-
HttpRequest request = requestFactory.buildDeleteRequest(new GenericUrl(url));
152-
request.setParser(new JsonObjectParser(jsonFactory));
153-
request.setResponseInterceptor(interceptor);
154-
response = request.execute();
155-
ByteStreams.exhaust(response.getContent());
156-
} catch (Exception e) {
157-
handleError(instanceId, e);
158-
} finally {
159-
disconnectQuietly(response);
160-
}
157+
HttpRequestInfo request = HttpRequestInfo.buildDeleteRequest(url)
158+
.setResponseInterceptor(interceptor);
159+
httpClient.send(request);
161160
return null;
162161
}
163162
};
164163
}
165164

166-
private static void disconnectQuietly(HttpResponse response) {
167-
if (response != null) {
168-
try {
169-
response.disconnect();
170-
} catch (IOException ignored) {
171-
// ignored
165+
private static class InstanceIdErrorHandler
166+
extends AbstractHttpErrorHandler<FirebaseInstanceIdException> {
167+
168+
@Override
169+
protected FirebaseInstanceIdException createException(FirebaseException base) {
170+
String message = base.getMessage();
171+
String customMessage = getCustomMessage(base);
172+
if (!Strings.isNullOrEmpty(customMessage)) {
173+
message = customMessage;
172174
}
175+
176+
return new FirebaseInstanceIdException(base, message);
173177
}
174-
}
175178

176-
private void handleError(String instanceId, Exception e) throws FirebaseInstanceIdException {
177-
String msg = "Error while invoking instance ID service.";
178-
if (e instanceof HttpResponseException) {
179-
int statusCode = ((HttpResponseException) e).getStatusCode();
180-
if (ERROR_CODES.containsKey(statusCode)) {
181-
msg = String.format("Instance ID \"%s\": %s", instanceId, ERROR_CODES.get(statusCode));
179+
private String getCustomMessage(FirebaseException base) {
180+
IncomingHttpResponse response = base.getHttpResponse();
181+
if (response != null) {
182+
String instanceId = extractInstanceId(response);
183+
String description = ERROR_CODES.get(response.getStatusCode());
184+
if (description != null) {
185+
return String.format("Instance ID \"%s\": %s", instanceId, description);
186+
}
182187
}
188+
189+
return null;
190+
}
191+
192+
private String extractInstanceId(IncomingHttpResponse response) {
193+
String url = response.getRequest().getUrl();
194+
int index = url.lastIndexOf('/');
195+
return url.substring(index + 1);
183196
}
184-
throw new FirebaseInstanceIdException(msg, e);
185197
}
186198

187199
private static final String SERVICE_ID = FirebaseInstanceId.class.getName();

‎src/main/java/com/google/firebase/iid/FirebaseInstanceIdException.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
/**
2222
* Represents an exception encountered while interacting with the Firebase instance ID service.
2323
*/
24-
public class FirebaseInstanceIdException extends FirebaseException {
24+
public final class FirebaseInstanceIdException extends FirebaseException {
2525

26-
FirebaseInstanceIdException(String detailMessage, Throwable cause) {
27-
super(detailMessage, cause);
26+
FirebaseInstanceIdException(FirebaseException base, String message) {
27+
super(base.getErrorCodeNew(), message, base.getCause(), base.getHttpResponse());
2828
}
2929
}

‎src/main/java/com/google/firebase/internal/HttpRequestInfo.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ public static HttpRequestInfo buildGetRequest(String url) {
6363
return new HttpRequestInfo(HttpMethods.GET, url, null);
6464
}
6565

66+
public static HttpRequestInfo buildDeleteRequest(String url) {
67+
return new HttpRequestInfo(HttpMethods.DELETE, url, null);
68+
}
69+
6670
public static HttpRequestInfo buildPostRequest(String url, HttpContent content) {
6771
return new HttpRequestInfo(HttpMethods.POST, url, content);
6872
}

‎src/test/java/com/google/firebase/iid/FirebaseInstanceIdTest.java

Lines changed: 124 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,25 @@
2323
import static org.junit.Assert.assertTrue;
2424
import static org.junit.Assert.fail;
2525

26+
import com.google.api.client.http.HttpMethods;
2627
import com.google.api.client.http.HttpRequest;
2728
import com.google.api.client.http.HttpResponseException;
29+
import com.google.api.client.http.HttpTransport;
2830
import com.google.api.client.testing.http.MockHttpTransport;
2931
import com.google.api.client.testing.http.MockLowLevelHttpResponse;
3032
import com.google.common.collect.ImmutableList;
3133
import com.google.common.collect.ImmutableMap;
34+
import com.google.firebase.ErrorCode;
3235
import com.google.firebase.FirebaseApp;
3336
import com.google.firebase.FirebaseOptions;
37+
import com.google.firebase.IncomingHttpResponse;
38+
import com.google.firebase.OutgoingHttpRequest;
3439
import com.google.firebase.TestOnlyImplFirebaseTrampolines;
3540
import com.google.firebase.auth.MockGoogleCredentials;
3641
import com.google.firebase.testing.GenericFunction;
3742
import com.google.firebase.testing.TestResponseInterceptor;
43+
import com.google.firebase.testing.TestUtils;
44+
import java.io.IOException;
3845
import java.util.List;
3946
import java.util.Map;
4047
import java.util.concurrent.ExecutionException;
@@ -43,6 +50,25 @@
4350

4451
public class FirebaseInstanceIdTest {
4552

53+
private static final Map<Integer, String> ERROR_MESSAGES = ImmutableMap.of(
54+
404, "Instance ID \"test-iid\": Failed to find the instance ID.",
55+
409, "Instance ID \"test-iid\": Already deleted.",
56+
429, "Instance ID \"test-iid\": Request throttled out by the backend server.",
57+
500, "Instance ID \"test-iid\": Internal server error.",
58+
501, "Unexpected HTTP response with status: 501\ntest error"
59+
);
60+
61+
private static final Map<Integer, ErrorCode> ERROR_CODES = ImmutableMap.of(
62+
404, ErrorCode.NOT_FOUND,
63+
409, ErrorCode.CONFLICT,
64+
429, ErrorCode.RESOURCE_EXHAUSTED,
65+
500, ErrorCode.INTERNAL,
66+
501, ErrorCode.UNKNOWN
67+
);
68+
69+
private static final String TEST_URL =
70+
"https://console.firebase.google.com/v1/project/test-project/instanceId/test-iid";
71+
4672
@After
4773
public void tearDown() {
4874
TestOnlyImplFirebaseTrampolines.clearInstancesForTest();
@@ -123,62 +149,120 @@ public Void call(Object... args) throws Exception {
123149
}
124150
);
125151

126-
String url = "https://console.firebase.google.com/v1/project/test-project/instanceId/test-iid";
127152
for (GenericFunction<Void> fn : functions) {
128153
TestResponseInterceptor interceptor = new TestResponseInterceptor();
129154
instanceId.setInterceptor(interceptor);
130155
fn.call();
131156

132157
assertNotNull(interceptor.getResponse());
133158
HttpRequest request = interceptor.getResponse().getRequest();
134-
assertEquals("DELETE", request.getRequestMethod());
135-
assertEquals(url, request.getUrl().toString());
159+
assertEquals(HttpMethods.DELETE, request.getRequestMethod());
160+
assertEquals(TEST_URL, request.getUrl().toString());
136161
assertEquals("Bearer test-token", request.getHeaders().getAuthorization());
137162
}
138163
}
139164

140165
@Test
141166
public void testDeleteInstanceIdError() throws Exception {
142-
Map<Integer, String> errors = ImmutableMap.of(
143-
404, "Instance ID \"test-iid\": Failed to find the instance ID.",
144-
429, "Instance ID \"test-iid\": Request throttled out by the backend server.",
145-
500, "Instance ID \"test-iid\": Internal server error.",
146-
501, "Error while invoking instance ID service."
147-
);
167+
final MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
168+
MockHttpTransport transport = new MockHttpTransport.Builder()
169+
.setLowLevelHttpResponse(response)
170+
.build();
171+
FirebaseOptions options = new FirebaseOptions.Builder()
172+
.setCredentials(new MockGoogleCredentials("test-token"))
173+
.setProjectId("test-project")
174+
.setHttpTransport(transport)
175+
.build();
176+
FirebaseApp app = FirebaseApp.initializeApp(options);
148177

149-
String url = "https://console.firebase.google.com/v1/project/test-project/instanceId/test-iid";
150-
for (Map.Entry<Integer, String> entry : errors.entrySet()) {
151-
MockLowLevelHttpResponse response = new MockLowLevelHttpResponse()
152-
.setStatusCode(entry.getKey())
153-
.setContent("test error");
154-
MockHttpTransport transport = new MockHttpTransport.Builder()
155-
.setLowLevelHttpResponse(response)
156-
.build();
157-
FirebaseOptions options = new FirebaseOptions.Builder()
158-
.setCredentials(new MockGoogleCredentials("test-token"))
159-
.setProjectId("test-project")
160-
.setHttpTransport(transport)
161-
.build();
162-
final FirebaseApp app = FirebaseApp.initializeApp(options);
163-
164-
FirebaseInstanceId instanceId = FirebaseInstanceId.getInstance();
165-
TestResponseInterceptor interceptor = new TestResponseInterceptor();
166-
instanceId.setInterceptor(interceptor);
167-
try {
168-
instanceId.deleteInstanceIdAsync("test-iid").get();
169-
fail("No error thrown for HTTP error");
170-
} catch (ExecutionException e) {
171-
assertTrue(e.getCause() instanceof FirebaseInstanceIdException);
172-
assertEquals(entry.getValue(), e.getCause().getMessage());
173-
assertTrue(e.getCause().getCause() instanceof HttpResponseException);
174-
}
178+
// Disable retries by passing a regular HttpRequestFactory.
179+
FirebaseInstanceId instanceId = new FirebaseInstanceId(app, transport.createRequestFactory());
180+
TestResponseInterceptor interceptor = new TestResponseInterceptor();
181+
instanceId.setInterceptor(interceptor);
175182

176-
assertNotNull(interceptor.getResponse());
177-
HttpRequest request = interceptor.getResponse().getRequest();
178-
assertEquals("DELETE", request.getRequestMethod());
179-
assertEquals(url, request.getUrl().toString());
180-
assertEquals("Bearer test-token", request.getHeaders().getAuthorization());
183+
try {
184+
for (int statusCode : ERROR_CODES.keySet()) {
185+
response.setStatusCode(statusCode).setContent("test error");
186+
187+
try {
188+
instanceId.deleteInstanceIdAsync("test-iid").get();
189+
fail("No error thrown for HTTP error");
190+
} catch (ExecutionException e) {
191+
assertTrue(e.getCause() instanceof FirebaseInstanceIdException);
192+
checkFirebaseInstanceIdException((FirebaseInstanceIdException) e.getCause(), statusCode);
193+
}
194+
195+
assertNotNull(interceptor.getResponse());
196+
HttpRequest request = interceptor.getResponse().getRequest();
197+
assertEquals(HttpMethods.DELETE, request.getRequestMethod());
198+
assertEquals(TEST_URL, request.getUrl().toString());
199+
}
200+
} finally {
181201
app.delete();
182202
}
183203
}
204+
205+
@Test
206+
public void testDeleteInstanceIdTransportError() throws Exception {
207+
HttpTransport transport = TestUtils.createFaultyHttpTransport();
208+
FirebaseOptions options = new FirebaseOptions.Builder()
209+
.setCredentials(new MockGoogleCredentials("test-token"))
210+
.setProjectId("test-project")
211+
.setHttpTransport(transport)
212+
.build();
213+
FirebaseApp app = FirebaseApp.initializeApp(options);
214+
// Disable retries by passing a regular HttpRequestFactory.
215+
FirebaseInstanceId instanceId = new FirebaseInstanceId(app, transport.createRequestFactory());
216+
217+
try {
218+
instanceId.deleteInstanceIdAsync("test-iid").get();
219+
fail("No error thrown for HTTP error");
220+
} catch (ExecutionException e) {
221+
assertTrue(e.getCause() instanceof FirebaseInstanceIdException);
222+
FirebaseInstanceIdException error = (FirebaseInstanceIdException) e.getCause();
223+
assertEquals(ErrorCode.UNKNOWN, error.getErrorCodeNew());
224+
assertEquals(
225+
"Unknown error while making a remote service call: transport error",
226+
error.getMessage());
227+
assertTrue(error.getCause() instanceof IOException);
228+
assertNull(error.getHttpResponse());
229+
}
230+
}
231+
232+
@Test
233+
public void testDeleteInstanceIdInvalidJsonIgnored() throws Exception {
234+
final MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
235+
MockHttpTransport transport = new MockHttpTransport.Builder()
236+
.setLowLevelHttpResponse(response)
237+
.build();
238+
FirebaseOptions options = new FirebaseOptions.Builder()
239+
.setCredentials(new MockGoogleCredentials("test-token"))
240+
.setProjectId("test-project")
241+
.setHttpTransport(transport)
242+
.build();
243+
FirebaseApp app = FirebaseApp.initializeApp(options);
244+
245+
// Disable retries by passing a regular HttpRequestFactory.
246+
FirebaseInstanceId instanceId = new FirebaseInstanceId(app, transport.createRequestFactory());
247+
TestResponseInterceptor interceptor = new TestResponseInterceptor();
248+
instanceId.setInterceptor(interceptor);
249+
response.setContent("not json");
250+
251+
instanceId.deleteInstanceIdAsync("test-iid").get();
252+
253+
assertNotNull(interceptor.getResponse());
254+
}
255+
256+
private void checkFirebaseInstanceIdException(FirebaseInstanceIdException error, int statusCode) {
257+
assertEquals(ERROR_CODES.get(statusCode), error.getErrorCodeNew());
258+
assertEquals(ERROR_MESSAGES.get(statusCode), error.getMessage());
259+
assertTrue(error.getCause() instanceof HttpResponseException);
260+
261+
IncomingHttpResponse httpResponse = error.getHttpResponse();
262+
assertNotNull(httpResponse);
263+
assertEquals(statusCode, httpResponse.getStatusCode());
264+
OutgoingHttpRequest request = httpResponse.getRequest();
265+
assertEquals(HttpMethods.DELETE, request.getMethod());
266+
assertEquals(TEST_URL, request.getUrl());
267+
}
184268
}

0 commit comments

Comments
 (0)