Skip to content

Commit

Permalink
Blame Urllib problems on the callers
Browse files Browse the repository at this point in the history
For Crashlytics.

The previous attempt at doing this failed, because it tried to modify
the *first* exception in the chain, but it turns out that it's the
*last* exception in the chain that Crashlytics looks at.

So given an Exception...
"
java.lang.Exception: This is a test Exception
    at not.bankdroid.at.all.ExceptionFactory.getException(ExceptionFactory.java:20)
    at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at ...
"
... we now report to Crashlytics:
"
java.lang.Exception: This is a test Exception
    at not.bankdroid.at.all.ExceptionFactory.getException(ExceptionFactory.java:20)
    at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at ...
Caused by: java.lang.Exception: This is a test Exception
    at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at ...
    ... 37 more
"
  • Loading branch information
walles authored and goober committed Nov 14, 2016
1 parent d64ede1 commit ef07d47
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 84 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.liato.bankdroid.utils;

import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;

import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;
Expand All @@ -11,41 +13,109 @@ public class ExceptionUtils {
private static final String PREFIX = "com.liato.bankdroid.";

/**
* Take an exception thrown and make it look like it came from Bankdroid.
* Modify an Exception to make it look like it was ultimately caused by Bankdroid.
* <p/>
* Specifically, if Urllib.java, called by Bankdroid code, throws an exception,
* rewrite the exception so that it appears as if it was thrown from the
* Bankdroid method calling Urllib, but caused by the original Exception.
* The purpose is to make Crashlytics report Urllib exceptions as coming from whatever
* bank Urllib is trying to access.
* <p/>
* For example, this exception:
* <pre>
* java.lang.Exception: This is a test Exception
* at not.bankdroid.at.all.ExceptionFactory.getException(ExceptionFactory.java:20)
* at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
* at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
* at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
* at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
* at java.lang.reflect.Method.invoke(Method.java:497)
* at ...
* </pre>
*
* Would be turned into this exception:
* <pre>
* java.lang.Exception: This is a test Exception
* at not.bankdroid.at.all.ExceptionFactory.getException(ExceptionFactory.java:20)
* at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
* at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
* at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
* at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
* at java.lang.reflect.Method.invoke(Method.java:497)
* at ...
* Caused by: java.lang.Exception: This is a test Exception
* at com.liato.bankdroid.utils.ExceptionUtilsTest.testBlameBankdroid(ExceptionUtilsTest.java:16)
* at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
* at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
* at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
* at java.lang.reflect.Method.invoke(Method.java:497)
* at ...
* ... 37 more
* </pre>
*/
public static <T extends Throwable> T bankdroidifyException(T exception) {
public static void blameBankdroid(Throwable exception) {
Throwable ultimateCause = getUltimateCause(exception);
if (ultimateCause == null) {
// Unable to find ultimate cause, never mind
return;
}

StackTraceElement[] bankdroidifiedStacktrace =
bankdroidifyStacktrace(exception.getStackTrace());
if (bankdroidifiedStacktrace.length == exception.getStackTrace().length) {
// Unable to bankdroidify stacktrace, never mind
return exception;
bankdroidifyStacktrace(ultimateCause.getStackTrace());
if (bankdroidifiedStacktrace.length == 0) {
// No Bankdroid stack frames found, never mind
return;
}
if (bankdroidifiedStacktrace.length == ultimateCause.getStackTrace().length) {
// Bankdroid already to blame, never mind
return;
}

T returnMe = createWrapperException(exception);
if (returnMe == null) {
Throwable fakeCause = cloneException(ultimateCause);
if (fakeCause == null) {
Timber.w(new RuntimeException(
"Unable to bankdroidify exception of class: " + exception.getClass()));
return exception;
"Unable to bankdroidify exception of class: " + ultimateCause.getClass()));
return;
}

returnMe.initCause(exception);
// Put the bankdroidified stack trace before the fakeCause's actual stack trace
fakeCause.setStackTrace(concatArrays(bankdroidifiedStacktrace, fakeCause.getStackTrace()));

returnMe.setStackTrace(bankdroidifiedStacktrace);
ultimateCause.initCause(fakeCause);
}

@VisibleForTesting
static StackTraceElement[] concatArrays(StackTraceElement[] a, StackTraceElement[] b) {
StackTraceElement[] returnMe = new StackTraceElement[a.length + b.length];
System.arraycopy(a, 0, returnMe, 0, a.length);
System.arraycopy(b, 0, returnMe, a.length, b.length);
return returnMe;
}

@VisibleForTesting
@Nullable
static Throwable getUltimateCause(Throwable t) {
int laps = 0;
Throwable ultimateCause = t;
while (ultimateCause.getCause() != null) {
ultimateCause = ultimateCause.getCause();
if (laps++ > 10) {
return null;
}
}
return ultimateCause;
}

/**
* Clone message and stacktrace but not the cause.
*/
@Nullable
private static <T extends Throwable> T createWrapperException(T wrapMe) {
@VisibleForTesting
static <T extends Throwable> T cloneException(T wrapMe) {
Class<?> newClass = wrapMe.getClass();
while (newClass != null) {
try {
return (T) newClass.getConstructor(String.class)
.newInstance(wrapMe.getMessage());
T returnMe =
(T) newClass.getConstructor(String.class).newInstance(wrapMe.getMessage());
returnMe.setStackTrace(wrapMe.getStackTrace());
return returnMe;
} catch (InvocationTargetException e) {
newClass = newClass.getSuperclass();
} catch (NoSuchMethodException e) {
Expand All @@ -63,9 +133,12 @@ private static <T extends Throwable> T createWrapperException(T wrapMe) {
/**
* Remove all initial non-Bankdroid frames from a stack.
*
* @return A copy of rawStack but with the initial non-Bankdroid frames removed
* @return A copy of rawStack but with the initial non-Bankdroid frames removed, or null
* if no sensible answer can be given.
*/
private static StackTraceElement[] bankdroidifyStacktrace(final StackTraceElement[] rawStack) {
@VisibleForTesting
@NonNull
static StackTraceElement[] bankdroidifyStacktrace(final StackTraceElement[] rawStack) {
for (int i = 0; i < rawStack.length; i++) {
StackTraceElement stackTraceElement = rawStack[i];
if (stackTraceElement.getClassName().startsWith(PREFIX)) {
Expand All @@ -74,6 +147,6 @@ private static StackTraceElement[] bankdroidifyStacktrace(final StackTraceElemen
}

// No Bankdroid stack frames found, never mind
return rawStack;
return new StackTraceElement[0];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ public String open(String url) throws ClientProtocolException, IOException {
try {
return this.open(url, new ArrayList<NameValuePair>());
} catch (IOException e) {
throw ExceptionUtils.bankdroidifyException(e);
ExceptionUtils.blameBankdroid(e);
throw e;
}
}

Expand All @@ -162,7 +163,8 @@ public String open(String url, List<NameValuePair> postData)
try {
return open(url, postData, false);
} catch (IOException e) {
throw ExceptionUtils.bankdroidifyException(e);
ExceptionUtils.blameBankdroid(e);
throw e;
}
}

Expand All @@ -182,7 +184,8 @@ public HttpResponse openAsHttpResponse(String url, List<NameValuePair> postData,
try {
return openAsHttpResponse(url, entity, forcePost);
} catch (IOException e) {
throw ExceptionUtils.bankdroidifyException(e);
ExceptionUtils.blameBankdroid(e);
throw e;
}
}

Expand All @@ -200,7 +203,8 @@ public HttpResponse openAsHttpResponse(String url, HttpEntity entity, boolean fo
return openAsHttpResponse(url, entity, HttpMethod.POST);
}
} catch (IOException e) {
throw ExceptionUtils.bankdroidifyException(e);
ExceptionUtils.blameBankdroid(e);
throw e;
}
}

Expand Down Expand Up @@ -293,7 +297,8 @@ public InputStream openStream(String url, String postData, boolean forcePost)
return openStream(url, postData != null ? new StringEntity(postData, this.charset) : null,
forcePost);
} catch (IOException e) {
throw ExceptionUtils.bankdroidifyException(e);
ExceptionUtils.blameBankdroid(e);
throw e;
}
}

Expand Down
Loading

0 comments on commit ef07d47

Please sign in to comment.