[android] - include request urls when logging out failed http requests

mapbox · Jan 5, 2018 · 8a55f2b · 8a55f2b
1 parent ad28542
commit 8a55f2b
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 23 deletions.
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java
@@ -30,10 +30,16 @@
 import okhttp3.internal.Util;
 import timber.log.Timber;
 
+import static android.util.Log.DEBUG;
+import static android.util.Log.INFO;
+import static android.util.Log.WARN;
+
 class HTTPRequest implements Callback {
 
   private static OkHttpClient mClient = new OkHttpClient.Builder().dispatcher(getDispatcher()).build();
   private static boolean logEnabled = true;
+  private static boolean logRequestUrl = false;
+
   private String USER_AGENT_STRING = null;
 
   private static final int CONNECTION_ERROR = 0;
@@ -97,7 +103,7 @@ private HTTPRequest(long nativePtr, String resourceUrl, String etag, String modi
       mCall = mClient.newCall(mRequest);
       mCall.enqueue(this);
     } catch (Exception exception) {
-      onFailure(exception);
+      handleFailure(mCall, exception);
     }
   }
 
@@ -134,7 +140,7 @@ public void onResponse(Call call, Response response) throws IOException {
     try {
       body = response.body().bytes();
     } catch (IOException ioException) {
-      onFailure(ioException);
+      onFailure(call, ioException);
       // throw ioException;
       return;
     } finally {
@@ -157,29 +163,16 @@ public void onResponse(Call call, Response response) throws IOException {
 
   @Override
   public void onFailure(Call call, IOException e) {
-    onFailure(e);
+    handleFailure(call, e);
   }
 
-  private void onFailure(Exception e) {
-    int type = PERMANENT_ERROR;
-    if ((e instanceof NoRouteToHostException) || (e instanceof UnknownHostException) || (e instanceof SocketException)
-      || (e instanceof ProtocolException) || (e instanceof SSLException)) {
-      type = CONNECTION_ERROR;
-    } else if ((e instanceof InterruptedIOException)) {
-      type = TEMPORARY_ERROR;
-    }
-
+  private void handleFailure(Call call, Exception e) {
     String errorMessage = e.getMessage() != null ? e.getMessage() : "Error processing the request";
+    int type = getFailureType(e);
 
     if (logEnabled) {
-      if (type == TEMPORARY_ERROR) {
-        Timber.d("Request failed due to a temporary error: %s", errorMessage);
-      } else if (type == CONNECTION_ERROR) {
-        Timber.i("Request failed due to a connection error: %s", errorMessage);
-      } else {
-        // PERMANENT_ERROR
-        Timber.w("Request failed due to a permanent error: %s", errorMessage);
-      }
+      String requestUrl = call.request().url().toString();
+      logFailure(type, errorMessage, requestUrl);
     }
 
     mLock.lock();
@@ -189,6 +182,26 @@ private void onFailure(Exception e) {
     mLock.unlock();
   }
 
+  private int getFailureType(Exception e){
+    if ((e instanceof NoRouteToHostException) || (e instanceof UnknownHostException) || (e instanceof SocketException)
+      || (e instanceof ProtocolException) || (e instanceof SSLException)) {
+      return CONNECTION_ERROR;
+    } else if ((e instanceof InterruptedIOException)) {
+      return TEMPORARY_ERROR;
+    }
+    return PERMANENT_ERROR;
+  }
+
+  private void logFailure(int type, String errorMessage, String requestUrl) {
+    Timber.log(
+      type == TEMPORARY_ERROR ? DEBUG : type == CONNECTION_ERROR ? INFO : WARN,
+      "Request failed due to a %s error: %s %s",
+      type == TEMPORARY_ERROR ? "temporary" : type == CONNECTION_ERROR ? "connection" : "permanent",
+      errorMessage,
+      logRequestUrl ? requestUrl : ""
+    );
+  }
+
   private String getUserAgent() {
     if (USER_AGENT_STRING == null) {
       return USER_AGENT_STRING = Util.toHumanReadableAscii(
@@ -217,4 +230,8 @@ private String getApplicationIdentifier() {
   static void enableLog(boolean enabled) {
     logEnabled = enabled;
   }
+
+  static void enablePrintRequestUrlOnFailure(boolean enabled) {
+    logRequestUrl = enabled;
+  }
 }
diff --git a/...m/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java b/...m/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HttpRequestUtil.java
@@ -6,7 +6,7 @@
 public class HttpRequestUtil {
 
   /**
-   * Set the log state of HttpRequest.
+   * Set the log state of HttpRequest. Default value is true.
    * <p>
    * This configuration will outlast the lifecycle of the Map.
    * </p>
@@ -16,4 +16,20 @@ public class HttpRequestUtil {
   public static void setLogEnabled(boolean enabled) {
     HTTPRequest.enableLog(enabled);
   }
+
+  /**
+   * Enable printing of the request url when an error occurred. Default value is false.
+   * <p>
+   * Requires {@link #setLogEnabled(boolean)} to be activated.
+   * </p>
+   * <p>
+   * This configuration will outlast the lifecycle of the Map.
+   * </p>
+   *
+   * @param enabled True will print urls, false will disable
+   */
+  public static void setPrintRequestUrlOnFaillure(boolean enabled) {
+    HTTPRequest.enablePrintRequestUrlOnFailure(enabled);
+  }
+
 }
diff --git a/...tApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/LatLngBoundsActivity.java b/...tApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/LatLngBoundsActivity.java
@@ -10,6 +10,7 @@
 import com.mapbox.mapboxsdk.camera.CameraUpdateFactory;
 import com.mapbox.mapboxsdk.geometry.LatLng;
 import com.mapbox.mapboxsdk.geometry.LatLngBounds;
+import com.mapbox.mapboxsdk.http.HttpRequestUtil;
 import com.mapbox.mapboxsdk.maps.MapView;
 import com.mapbox.mapboxsdk.maps.MapboxMap;
 import com.mapbox.mapboxsdk.testapp.R;
@@ -51,6 +52,7 @@ public class LatLngBoundsActivity extends AppCompatActivity implements View.OnCl
   @Override
   protected void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
+    HttpRequestUtil.setLogEnabled(false);
     setContentView(R.layout.activity_latlngbounds);
     mapView = (MapView) findViewById(R.id.mapView);
     mapView.onCreate(savedInstanceState);
@@ -149,6 +151,7 @@ public void onLowMemory() {
   protected void onDestroy() {
     super.onDestroy();
     mapView.onDestroy();
+    HttpRequestUtil.setLogEnabled(true);
   }
 
   @Override

diff --git a/...tApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/maplayout/DebugModeActivity.java b/...tApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/maplayout/DebugModeActivity.java
@@ -55,7 +55,7 @@ public class DebugModeActivity extends AppCompatActivity implements OnMapReadyCa
   @Override
   protected void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
-    HttpRequestUtil.setLogEnabled(false);
+    HttpRequestUtil.setPrintRequestUrlOnFaillure(true);
     setContentView(R.layout.activity_debug_mode);
     setupToolbar();
     setupMapView(savedInstanceState);
@@ -206,7 +206,7 @@ protected void onSaveInstanceState(Bundle outState) {
   protected void onDestroy() {
     super.onDestroy();
     mapView.onDestroy();
-    HttpRequestUtil.setLogEnabled(true);
+    HttpRequestUtil.setPrintRequestUrlOnFaillure(false);
   }
 
   @Override