Skip to content

Commit

Permalink
Fix wrapping exceptions without String constructors
Browse files Browse the repository at this point in the history
Before this change, bankdroidifyException() failed if the exception
to wrap didn't come with a String constructor.

Now it works, with tests and all!
  • Loading branch information
walles authored and goober committed Nov 5, 2016
1 parent 0423092 commit d64ede1
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 17 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.liato.bankdroid.utils;

import android.support.annotation.Nullable;

import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;

Expand All @@ -23,21 +25,10 @@ public static <T extends Throwable> T bankdroidifyException(T exception) {
return exception;
}

T returnMe;
try {
returnMe = (T)exception.getClass().getConstructor(String.class)
.newInstance(exception.getMessage());
} catch (InstantiationException e) {
Timber.e(e, "Unable to Bankdroidify exception of type %s", exception.getClass());
return exception;
} catch (InvocationTargetException e) {
Timber.e(e, "Unable to Bankdroidify exception of type %s", exception.getClass());
return exception;
} catch (IllegalAccessException e) {
Timber.e(e, "Unable to Bankdroidify exception of type %s", exception.getClass());
return exception;
} catch (NoSuchMethodException e) {
Timber.e(e, "Unable to Bankdroidify exception of type %s", exception.getClass());
T returnMe = createWrapperException(exception);
if (returnMe == null) {
Timber.w(new RuntimeException(
"Unable to bankdroidify exception of class: " + exception.getClass()));
return exception;
}

Expand All @@ -48,6 +39,27 @@ public static <T extends Throwable> T bankdroidifyException(T exception) {
return returnMe;
}

@Nullable
private static <T extends Throwable> T createWrapperException(T wrapMe) {
Class<?> newClass = wrapMe.getClass();
while (newClass != null) {
try {
return (T) newClass.getConstructor(String.class)
.newInstance(wrapMe.getMessage());
} catch (InvocationTargetException e) {
newClass = newClass.getSuperclass();
} catch (NoSuchMethodException e) {
newClass = newClass.getSuperclass();
} catch (InstantiationException e) {
newClass = newClass.getSuperclass();
} catch (IllegalAccessException e) {
newClass = newClass.getSuperclass();
}
}

return null;
}

/**
* Remove all initial non-Bankdroid frames from a stack.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@
import org.junit.Assert;
import org.junit.Test;

import java.net.ConnectException;

import eu.nullbyte.android.urllib.Urllib;
import not.bankdroid.at.all.ExceptionThrower;

@SuppressWarnings("CallToPrintStackTrace")
public class ExceptionUtilsTest {
@Test
@SuppressWarnings("PMD") // This is for the stack trace printing, we really want to do it here
public void bankdroidifyException() throws Exception {
@SuppressWarnings({"PMD.AvoidPrintStackTrace", "PMD.AvoidCatchingNPE", "PMD.SystemPrintln"})
public void testBankdroidifyException() throws Exception {
Exception raw = null;
try {
//noinspection ConstantConditions
new Urllib(null);
Assert.fail("Exception expected");
} catch (NullPointerException e) {
Expand Down Expand Up @@ -38,4 +43,45 @@ public void bankdroidifyException() throws Exception {
// Verify that re-bankdroidifying is a no-op
Assert.assertSame(bankdroidified, ExceptionUtils.bankdroidifyException(bankdroidified));
}

/**
* Test that we can wrap exceptions without (String) constructors.
*/
@Test
@SuppressWarnings({"PMD.AvoidPrintStackTrace", "PMD.SystemPrintln"})
public void testBankdroidifyWonkyException() {
ExceptionThrower.WonkyException raw = null;
try {
ExceptionThrower.throwWonkyException();
Assert.fail("Exception expected");
} catch (ExceptionThrower.WonkyException e) {
raw = e;
}

// Print stack traces, useful if the tests fail
System.err.println("Before:");
raw.printStackTrace();

// Since bankdroidify() won't be able to create a WonkyException, it
// should fall back to creating something it extends
ConnectException bankdroidified = ExceptionUtils.bankdroidifyException(raw);

System.err.println();
System.err.println("After:");
bankdroidified.printStackTrace();

Assert.assertFalse("Test setup: Top frame of initial exception shouldn't be in Bankdroid",
raw.getStackTrace()[0].getClassName().startsWith("com.liato.bankdroid."));

Assert.assertTrue("Top frame of bankdroidified exception should be in Bankdroid",
bankdroidified.getStackTrace()[0].getClassName().startsWith("com.liato.bankdroid."));

Assert.assertEquals(raw.getMessage(), bankdroidified.getMessage());

// Verify that e is the cause of bankdroidified
Assert.assertSame(raw, bankdroidified.getCause());

// Verify that re-bankdroidifying is a no-op
Assert.assertSame(bankdroidified, ExceptionUtils.bankdroidifyException(bankdroidified));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package not.bankdroid.at.all;

import java.net.ConnectException;

/**
* For the test in {@link com.liato.bankdroid.utils.ExceptionUtilsTest}
*/
public class ExceptionThrower {
public static class WonkyException extends ConnectException {
public WonkyException(int wonky) {
super("Wonky: " + wonky);
}
}

public static void throwWonkyException() throws WonkyException {
throw new WonkyException(5);
}
}

0 comments on commit d64ede1

Please sign in to comment.