Skip to content

Realtime RC - Check HTTP status before listening and retrying #4121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 21, 2022
Merged

Realtime RC - Check HTTP status before listening and retrying #4121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
90ee6a1
Update client to check Http status codes before listening or connecting
qdpham13 Sep 20, 2022
c0c1c5c
Fix tests and add more tests.
qdpham13 Sep 20, 2022
f18fdf4
Format files.
qdpham13 Sep 20, 2022
59be1eb
Close error stream
qdpham13 Sep 21, 2022
55742a8
Update status code check and initialize response code to 0
qdpham13 Sep 21, 2022
0c96b00
Change responseCode var to Integer and initialize to null
qdpham13 Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigAutoFetch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,9 @@ private String parseAndValidateConfigUpdateMessage(String message) {
public void listenForNotifications() {
if (httpURLConnection != null) {
try {
int responseCode = httpURLConnection.getResponseCode();
if (responseCode == 200) {
InputStream inputStream = httpURLConnection.getInputStream();
handleNotifications(inputStream);
inputStream.close();
} else {
propagateErrors(
new FirebaseRemoteConfigRealtimeUpdateStreamException(
"Http connection responded with error: " + responseCode));
}
InputStream inputStream = httpURLConnection.getInputStream();
handleNotifications(inputStream);
inputStream.close();
} catch (IOException ex) {
propagateErrors(
new FirebaseRemoteConfigRealtimeUpdateFetchException(
Expand Down
60 changes: 47 additions & 13 deletions ...fig/src/main/java/com/google/firebase/remoteconfig/internal/ConfigRealtimeHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.TAG;
import static com.google.firebase.remoteconfig.RemoteConfigConstants.REALTIME_REGEX_URL;
import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.HTTP_TOO_MANY_REQUESTS;

import android.annotation.SuppressLint;
import android.content.Context;
Expand Down Expand Up @@ -236,7 +237,9 @@ private URL getUrl() {
return realtimeURL;
}

private HttpURLConnection createRealtimeConnection() throws IOException {
/** Create HTTP connection and set headers. */
@SuppressLint("VisibleForTests")
public HttpURLConnection createRealtimeConnection() throws IOException {
URL realtimeUrl = getUrl();
HttpURLConnection httpURLConnection = (HttpURLConnection) realtimeUrl.openConnection();
setCommonRequestHeaders(httpURLConnection);
Expand All @@ -245,8 +248,9 @@ private HttpURLConnection createRealtimeConnection() throws IOException {
return httpURLConnection;
}

// Try to reopen HTTP connection after a random amount of time
private synchronized void retryHTTPConnection() {
/** Retries HTTP stream connection asyncly in random time intervals. */
@SuppressLint("VisibleForTests")
public synchronized void retryHTTPConnection() {
if (canMakeHttpStreamConnection() && httpRetriesRemaining > 0) {
if (httpRetriesRemaining < ORIGINAL_RETRIES) {
httpRetrySeconds *= getRetryMultiplier();
Expand All @@ -273,7 +277,12 @@ synchronized void stopRealtime() {
scheduledExecutorService.shutdownNow();
}

private synchronized ConfigAutoFetch startAutoFetch(HttpURLConnection httpURLConnection) {
/**
* Create Autofetch class that listens on HTTP stream for ConfigUpdate messages and calls Fetch
* accordingly.
*/
@SuppressLint("VisibleForTests")
public synchronized ConfigAutoFetch startAutoFetch(HttpURLConnection httpURLConnection) {
ConfigUpdateListener retryCallback =
new ConfigUpdateListener() {
@Override
Expand All @@ -298,6 +307,15 @@ public void onError(Exception error) {
return new ConfigAutoFetch(httpURLConnection, configFetchHandler, listeners, retryCallback);
}

// HTTP status code that the Realtime client should retry on.
private boolean isStatusCodeRetryable(int statusCode) {
return statusCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT
|| statusCode == HTTP_TOO_MANY_REQUESTS
|| statusCode == HttpURLConnection.HTTP_BAD_GATEWAY
|| statusCode == HttpURLConnection.HTTP_UNAVAILABLE
|| statusCode == HttpURLConnection.HTTP_GATEWAY_TIMEOUT;
}

/**
* Open the realtime connection, begin listening for updates, and auto-fetch when an update is
* received.
Expand All @@ -306,39 +324,55 @@ public void onError(Exception error) {
* chunk-encoded HTTP body. When the connection closes, it attempts to reestablish the stream.
*/
@SuppressLint("VisibleForTests")
synchronized void beginRealtimeHttpStream() {
public synchronized void beginRealtimeHttpStream() {
if (!canMakeHttpStreamConnection()) {
return;
}

Integer responseCode = null;
try {
// Create the open the connection.
httpURLConnection = createRealtimeConnection();
httpURLConnection.connect();
responseCode = httpURLConnection.getResponseCode();

// Reset the retries remaining if we opened the connection without an exception.
resetRetryParameters();
// If the connection returned a 200 response code, start listening for messages.
if (responseCode == HttpURLConnection.HTTP_OK) {
// Reset the retries remaining if we opened the connection without an exception.
resetRetryParameters();

// Start listening for realtime notifications.
ConfigAutoFetch configAutoFetch = startAutoFetch(httpURLConnection);
configAutoFetch.listenForNotifications();
// Start listening for realtime notifications.
ConfigAutoFetch configAutoFetch = startAutoFetch(httpURLConnection);
configAutoFetch.listenForNotifications();
}
} catch (IOException e) {
Log.d(TAG, "Exception connecting to realtime stream. Retrying the connection...");
} finally {
closeRealtimeHttpStream();
retryHTTPConnection();

// If responseCode is null then no connection was made to server and the SDK should still
// retry.
if (responseCode == null
|| responseCode == HttpURLConnection.HTTP_OK
|| isStatusCodeRetryable(responseCode)) {
retryHTTPConnection();
} else {
propagateErrors(
new FirebaseRemoteConfigRealtimeUpdateStreamException(
"The server returned a status code that is not retryable. Realtime is shutting down."));
}
}
}

// Pauses Http stream listening
synchronized void closeRealtimeHttpStream() {
public synchronized void closeRealtimeHttpStream() {
if (httpURLConnection != null) {
this.httpURLConnection.disconnect();

// Explicitly close the input stream due to a bug in the Android okhttp implementation.
// See github.com/firebase/firebase-android-sdk/pull/808.
try {
this.httpURLConnection.getInputStream().close();
this.httpURLConnection.getErrorStream().close();
} catch (IOException e) {
}
this.httpURLConnection = null;
Expand Down
76 changes: 66 additions & 10 deletions firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -65,6 +68,7 @@
import com.google.firebase.remoteconfig.internal.ConfigGetParameterHandler;
import com.google.firebase.remoteconfig.internal.ConfigMetadataClient;
import com.google.firebase.remoteconfig.internal.ConfigRealtimeHandler;
import com.google.firebase.remoteconfig.internal.ConfigRealtimeHttpClient;
import com.google.firebase.remoteconfig.internal.Personalization;
import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -140,6 +144,7 @@ public final class FirebaseRemoteConfigTest {
@Mock private ConfigMetadataClient metadataClient;

@Mock private ConfigRealtimeHandler mockConfigRealtimeHandler;
@Mock private ConfigAutoFetch mockConfigAutoFetch;
@Mock private ConfigUpdateListenerRegistration mockRealtimeRegistration;
@Mock private HttpURLConnection mockHttpURLConnection;
@Mock private ConfigUpdateListener mockListener;
Expand All @@ -165,6 +170,7 @@ public final class FirebaseRemoteConfigTest {
private FetchResponse realtimeFetchedContainerResponse;
private ConfigContainer realtimeFetchedContainer;
private ConfigAutoFetch configAutoFetch;
private ConfigRealtimeHttpClient configRealtimeHttpClient;

private FetchResponse firstFetchedContainerResponse;

Expand Down Expand Up @@ -269,6 +275,14 @@ public void setUp() throws Exception {
listeners.add(mockListener);
configAutoFetch =
new ConfigAutoFetch(mockHttpURLConnection, mockFetchHandler, listeners, mockRetryListener);
configRealtimeHttpClient =
new ConfigRealtimeHttpClient(
firebaseApp,
mockFirebaseInstallations,
mockFetchHandler,
context,
"firebase",
listeners);
}

@Test
Expand Down Expand Up @@ -1107,7 +1121,6 @@ public void realtime_client_removeListener_success() {

@Test
public void realtime_stream_listen_and_retry_success() throws Exception {
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);
when(mockHttpURLConnection.getInputStream())
.thenReturn(
new ByteArrayInputStream(
Expand All @@ -1121,17 +1134,60 @@ public void realtime_stream_listen_and_retry_success() throws Exception {

@Test
public void realtime_stream_listen_fail() throws Exception {
when(mockHttpURLConnection.getResponseCode()).thenReturn(400);
when(mockHttpURLConnection.getInputStream())
.thenReturn(
new ByteArrayInputStream(
"{\\r\\n \\\"latestTemplateVersionNumber\\\": 1\\r\\n}"
.getBytes(StandardCharsets.UTF_8)));
when(mockFetchHandler.getTemplateVersionNumber()).thenReturn(1L);
when(mockFetchHandler.fetch(0)).thenReturn(Tasks.forResult(realtimeFetchedContainerResponse));
when(mockHttpURLConnection.getInputStream()).thenThrow(IOException.class);
configAutoFetch.listenForNotifications();

verify(mockListener).onError(any(FirebaseRemoteConfigRealtimeUpdateStreamException.class));
verify(mockListener).onError(any(FirebaseRemoteConfigRealtimeUpdateFetchException.class));
}

@Test
public void realtime_redirectStatusCode_noRetries() throws Exception {
ConfigRealtimeHttpClient configRealtimeHttpClientSpy = spy(configRealtimeHttpClient);
doReturn(mockHttpURLConnection).when(configRealtimeHttpClientSpy).createRealtimeConnection();
doNothing().when(configRealtimeHttpClientSpy).closeRealtimeHttpStream();
when(mockHttpURLConnection.getResponseCode()).thenReturn(301);
configRealtimeHttpClientSpy.beginRealtimeHttpStream();
verify(configRealtimeHttpClientSpy, never()).startAutoFetch(any());
verify(configRealtimeHttpClientSpy, never()).retryHTTPConnection();
}

@Test
public void realtime_okStatusCode_startAutofetchAndRetries() throws Exception {
ConfigRealtimeHttpClient configRealtimeHttpClientSpy = spy(configRealtimeHttpClient);
doReturn(mockHttpURLConnection).when(configRealtimeHttpClientSpy).createRealtimeConnection();
doReturn(mockConfigAutoFetch).when(configRealtimeHttpClientSpy).startAutoFetch(any());
doNothing().when(configRealtimeHttpClientSpy).retryHTTPConnection();
doNothing().when(configRealtimeHttpClientSpy).closeRealtimeHttpStream();
when(mockHttpURLConnection.getResponseCode()).thenReturn(200);

configRealtimeHttpClientSpy.beginRealtimeHttpStream();
verify(mockConfigAutoFetch).listenForNotifications();
verify(configRealtimeHttpClientSpy).retryHTTPConnection();
}

@Test
public void realtime_badGatewayStatusCode_noAutofetchButRetries() throws Exception {
ConfigRealtimeHttpClient configRealtimeHttpClientSpy = spy(configRealtimeHttpClient);
doReturn(mockHttpURLConnection).when(configRealtimeHttpClientSpy).createRealtimeConnection();
doNothing().when(configRealtimeHttpClientSpy).retryHTTPConnection();
doNothing().when(configRealtimeHttpClientSpy).closeRealtimeHttpStream();
when(mockHttpURLConnection.getResponseCode()).thenReturn(502);

configRealtimeHttpClientSpy.beginRealtimeHttpStream();
verify(configRealtimeHttpClientSpy, never()).startAutoFetch(any());
verify(configRealtimeHttpClientSpy).retryHTTPConnection();
}

@Test
public void realtime_exceptionThrown_noAutofetchButRetries() throws Exception {
ConfigRealtimeHttpClient configRealtimeHttpClientSpy = spy(configRealtimeHttpClient);
doThrow(IOException.class).when(configRealtimeHttpClientSpy).createRealtimeConnection();
doNothing().when(configRealtimeHttpClientSpy).retryHTTPConnection();
doNothing().when(configRealtimeHttpClientSpy).closeRealtimeHttpStream();

configRealtimeHttpClientSpy.beginRealtimeHttpStream();
verify(configRealtimeHttpClientSpy, never()).startAutoFetch(any());
verify(configRealtimeHttpClientSpy).retryHTTPConnection();
}

@Test
Expand Down