Trying to preserve the getCause() of ProvisionException to be the user's exception
git-svn-id: https://google-guice.googlecode.com/svn/trunk@347 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/BoundProviderFactory.java b/src/com/google/inject/BoundProviderFactory.java
index 6d1c442..61046a1 100644
--- a/src/com/google/inject/BoundProviderFactory.java
+++ b/src/com/google/inject/BoundProviderFactory.java
@@ -57,6 +57,15 @@
}
public T get(InternalContext context) {
- return context.sanitize(providerFactory.get(context).get(), source);
+ Provider<? extends T> provider = providerFactory.get(context);
+ try {
+ return context.sanitize(provider.get(), source);
+ } catch(ProvisionException provisionException) {
+ provisionException.addContext(context.getExternalContext());
+ throw provisionException;
+ } catch(RuntimeException e) {
+ throw new ProvisionException(context.getExternalContext(), e,
+ ErrorMessages.ERROR_IN_PROVIDER);
+ }
}
}
diff --git a/src/com/google/inject/ConstructorInjector.java b/src/com/google/inject/ConstructorInjector.java
index cd87cf1..6660f3e 100644
--- a/src/com/google/inject/ConstructorInjector.java
+++ b/src/com/google/inject/ConstructorInjector.java
@@ -158,7 +158,9 @@
return t;
}
catch (InvocationTargetException e) {
- throw new RuntimeException(e);
+ Throwable cause = e.getCause() != null ? e.getCause() : e;
+ throw new ProvisionException(context.getExternalContext(), cause,
+ ErrorMessages.ERROR_INJECTING_CONSTRUCTOR);
}
finally {
constructionContext.removeCurrentReference();
diff --git a/src/com/google/inject/ErrorMessages.java b/src/com/google/inject/ErrorMessages.java
index f31fb31..9b8e641 100644
--- a/src/com/google/inject/ErrorMessages.java
+++ b/src/com/google/inject/ErrorMessages.java
@@ -155,14 +155,24 @@
static final String PRELOAD_NOT_ALLOWED = "Preloading is only supported for"
+ " singleton-scoped bindings.";
- static final String EXCEPTION_WHILE_CREATING = "Error while locating"
- + " instance%n bound to %s%n for member at %s";
-
- static final String CANNOT_INJECT_NULL_INTO_MEMBER =
- "Null value returned by binding at %s, but %s is not @Nullable";
+ static final String ERROR_INJECTING_FIELD = "Error injecting field";
+ static final String ERROR_INJECTING_METHOD = "Error injecting method";
+ static final String ERROR_INJECTING_CONSTRUCTOR =
+ "Error injecting constructor";
+ static final String ERROR_IN_PROVIDER = "Error in custom provider";
+ static final String ERROR_WHILE_LOCATING_FIELD =
+ " while locating %s%n for field at %s";
+ static final String ERROR_WHILE_LOCATING_PARAMETER =
+ " while locating %s%n for parameter %s at %s";
+ static final String ERROR_WHILE_LOCATING_VALUE =
+ " while locating %s";
static final String CANNOT_INJECT_NULL =
- "Null value returned by binding at %s";
+ "null returned by binding at %s";
+ static final String CANNOT_INJECT_NULL_INTO_MEMBER =
+ "null returned by binding at %s%n but %s is not @Nullable";
+
+
static String getRootMessage(Throwable t) {
Throwable cause = t.getCause();
diff --git a/src/com/google/inject/InjectorImpl.java b/src/com/google/inject/InjectorImpl.java
index 6909cd9..970c045 100644
--- a/src/com/google/inject/InjectorImpl.java
+++ b/src/com/google/inject/InjectorImpl.java
@@ -477,8 +477,13 @@
catch (ConfigurationException e) {
throw e;
}
- catch (Throwable throwable) {
- throw new ProvisionException(externalContext, throwable);
+ catch (ProvisionException provisionException) {
+ provisionException.addContext(externalContext);
+ throw provisionException;
+ }
+ catch (RuntimeException runtimeException) {
+ throw new ProvisionException(externalContext, runtimeException,
+ ErrorMessages.ERROR_INJECTING_FIELD);
}
finally {
context.setExternalContext(previous);
@@ -571,14 +576,17 @@
try {
methodInvoker.invoke(o, getParameters(context, parameterInjectors));
}
+ catch (IllegalAccessException e) {
+ throw new AssertionError(e);
+ }
catch (ProvisionException e) {
+ e.addContext(context.getExternalContext());
throw e;
}
- catch (IllegalAccessException e) {
- throw new ProvisionException(context.getExternalContext(), e);
- }
catch (InvocationTargetException e) {
- throw new ProvisionException(context.getExternalContext(), e);
+ Throwable cause = e.getCause() != null ? e.getCause() : e;
+ throw new ProvisionException(context.getExternalContext(), cause,
+ ErrorMessages.ERROR_INJECTING_METHOD);
}
}
}
@@ -655,8 +663,13 @@
catch (ConfigurationException e) {
throw e;
}
- catch (Throwable throwable) {
- throw new ProvisionException(externalContext, throwable);
+ catch (ProvisionException provisionException) {
+ provisionException.addContext(externalContext);
+ throw provisionException;
+ }
+ catch (RuntimeException runtimeException) {
+ throw new ProvisionException(externalContext, runtimeException,
+ ErrorMessages.ERROR_INJECTING_METHOD);
}
finally {
context.setExternalContext(previous);
diff --git a/src/com/google/inject/InternalContext.java b/src/com/google/inject/InternalContext.java
index bfb1301..6b2a447 100644
--- a/src/com/google/inject/InternalContext.java
+++ b/src/com/google/inject/InternalContext.java
@@ -86,8 +86,10 @@
? String.format(ErrorMessages.CANNOT_INJECT_NULL_INTO_MEMBER, source,
getExternalContext().getMember())
: String.format(ErrorMessages.CANNOT_INJECT_NULL, source);
+
throw new ProvisionException(externalContext,
- new NullPointerException(message));
+ new NullPointerException(message),
+ String.format(ErrorMessages.CANNOT_INJECT_NULL, source));
}
// TODO(kevinb): gee, ya think we might want to remove this?
diff --git a/src/com/google/inject/ProvisionException.java b/src/com/google/inject/ProvisionException.java
index 689db4d..fa0fa9e 100644
--- a/src/com/google/inject/ProvisionException.java
+++ b/src/com/google/inject/ProvisionException.java
@@ -16,24 +16,93 @@
package com.google.inject;
+import static com.google.inject.ErrorMessages.ERROR_WHILE_LOCATING_FIELD;
+import static com.google.inject.ErrorMessages.ERROR_WHILE_LOCATING_PARAMETER;
+import static com.google.inject.ErrorMessages.ERROR_WHILE_LOCATING_VALUE;
+import com.google.inject.internal.Objects;
import com.google.inject.internal.StackTraceElements;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
/**
* Used to rethrow exceptions that occur while providing instances, to add
* additional contextual details.
*/
-class ProvisionException extends RuntimeException {
- public ProvisionException(ExternalContext<?> externalContext,
- Throwable cause) {
- super(createMessage(externalContext), cause);
+public class ProvisionException extends RuntimeException {
+
+ private static final String NEWLINE = String.format("%n");
+
+ private final String errorMessage;
+ private final List<ExternalContext> contexts =
+ new ArrayList<ExternalContext>(4);
+
+ ProvisionException(ExternalContext<?> externalContext,
+ Throwable cause, String errorMessage) {
+ super(errorMessage, cause);
+ this.errorMessage = errorMessage;
+ contexts.add(externalContext);
}
- private static String createMessage(ExternalContext<?> externalContext) {
+ /**
+ * Add more context to this exception, to be included in the exception
+ * message. This allows nested contexts to be displayed more concisely than
+ * with exception chaining.
+ */
+ void addContext(ExternalContext<?> externalContext) {
+ // deduplicate contexts.
+ if (!contexts.isEmpty()) {
+ ExternalContext last = contexts.get(contexts.size() - 1);
+ if (Objects.equal(last.getKey(), externalContext.getKey())
+ && Objects.equal(last.getMember(), externalContext.getMember())) {
+ return;
+ }
+ }
+
+ contexts.add(externalContext);
+ }
+
+ @Override
+ public String getMessage() {
+ StringBuilder result = new StringBuilder();
+ result.append(errorMessage)
+ .append(NEWLINE);
+
+ for (Iterator<ExternalContext> e = contexts.iterator(); e.hasNext(); ) {
+ ExternalContext externalContext = e.next();
+ result.append(contextToSnippet(externalContext));
+ if (e.hasNext()) {
+ result.append(NEWLINE);
+ }
+ }
+
+ return result.toString();
+ }
+
+ /**
+ * Returns a snippet to include in the stacktrace message that describes the
+ * specified context.
+ */
+ private String contextToSnippet(ExternalContext externalContext) {
Key<?> key = externalContext.getKey();
+ Object keyDescription = ErrorMessages.convert(key);
Member member = externalContext.getMember();
- return String.format(ErrorMessages.EXCEPTION_WHILE_CREATING,
- ErrorMessages.convert(key),
- StackTraceElements.forMember(member));
+
+ if (member instanceof Field) {
+ return String.format(ERROR_WHILE_LOCATING_FIELD,
+ keyDescription, StackTraceElements.forMember(member));
+
+ } else if (member instanceof Method || member instanceof Constructor) {
+ return String.format(ERROR_WHILE_LOCATING_PARAMETER,
+ keyDescription, externalContext.getParameterIndex(),
+ StackTraceElements.forMember(member));
+
+ } else {
+ return String.format(ERROR_WHILE_LOCATING_VALUE, keyDescription);
+ }
}
}
diff --git a/test/com/google/inject/NullableInjectionPointTest.java b/test/com/google/inject/NullableInjectionPointTest.java
index 413b419..f2ccc32 100644
--- a/test/com/google/inject/NullableInjectionPointTest.java
+++ b/test/com/google/inject/NullableInjectionPointTest.java
@@ -13,10 +13,10 @@
fail("Injecting null should fail with an error");
}
catch (ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
- assertContains(rootCause.getMessage(), "FooConstructor");
- assertContains(rootCause.getMessage(), "is not @Nullable");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
+ assertContains(cause.getMessage(), "FooConstructor");
+ assertContains(cause.getMessage(), "is not @Nullable");
}
}
@@ -26,10 +26,10 @@
fail("Injecting null should fail with an error");
}
catch (ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
- assertContains(rootCause.getMessage(), "FooMethod.setFoo");
- assertContains(rootCause.getMessage(), "is not @Nullable");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
+ assertContains(cause.getMessage(), "FooMethod.setFoo");
+ assertContains(cause.getMessage(), "is not @Nullable");
}
}
@@ -39,10 +39,10 @@
fail("Injecting null should fail with an error");
}
catch (ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
- assertContains(rootCause.getMessage(), "FooField.foo");
- assertContains(rootCause.getMessage(), "is not @Nullable");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
+ assertContains(cause.getMessage(), "FooField.foo");
+ assertContains(cause.getMessage(), "is not @Nullable");
}
}
@@ -52,8 +52,8 @@
fail("Getting an instance of null should fail with an error");
}
catch (ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding "
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding "
+ "at com.google.inject.NullableInjectionPointTest");
}
}
@@ -97,8 +97,8 @@
injector.getInstance(FooField.class);
}
catch(ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
}
}
@@ -118,8 +118,8 @@
injector.getInstance(FooField.class);
}
catch(ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
}
}
@@ -139,8 +139,8 @@
injector.getInstance(FooField.class);
}
catch(ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding");
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding at");
}
}
@@ -160,19 +160,12 @@
injector.getInstance(FooField.class);
}
catch(ProvisionException expected) {
- Throwable rootCause = getRootCause(expected);
- assertContains(rootCause.getMessage(), "Null value returned by binding "
+ NullPointerException cause = (NullPointerException)expected.getCause();
+ assertContains(cause.getMessage(), "null returned by binding "
+ "at com.google.inject.NullableInjectionPointTest");
}
}
- private Throwable getRootCause(Throwable throwable) {
- while (throwable.getCause() != null) {
- throwable = throwable.getCause();
- }
- return throwable;
- }
-
private void assertContains(String text, String substring) {
assertTrue(String.format("Expected \"%s\" to contain substring \"%s\"",
text, substring), text.contains(substring));
diff --git a/test/com/google/inject/ProvisionExceptionTest.java b/test/com/google/inject/ProvisionExceptionTest.java
new file mode 100644
index 0000000..b1afc8f
--- /dev/null
+++ b/test/com/google/inject/ProvisionExceptionTest.java
@@ -0,0 +1,95 @@
+package com.google.inject;
+
+import junit.framework.TestCase;
+
+/**
+ * @author jessewilson
+ */
+public class ProvisionExceptionTest extends TestCase {
+
+ public void testExceptionsCollapsed() {
+ try {
+ Guice.createInjector().getInstance(A.class);
+ fail();
+ }
+ catch (ProvisionException e) {
+ assertTrue(e.getCause() instanceof UnsupportedOperationException);
+ assertContains(e.getMessage(), "Error injecting constructor");
+ assertContains(e.getMessage(), "while locating "
+ + "com.google.inject.ProvisionExceptionTest$D");
+ assertContains(e.getMessage(), "for parameter 0 at "
+ + "com.google.inject.ProvisionExceptionTest$C.setD");
+ assertContains(e.getMessage(), "while locating "
+ + "com.google.inject.ProvisionExceptionTest$C");
+ assertContains(e.getMessage(), "for field at "
+ + "com.google.inject.ProvisionExceptionTest$B.c");
+ assertContains(e.getMessage(), "while locating "
+ + "com.google.inject.ProvisionExceptionTest$B");
+ assertContains(e.getMessage(), "for parameter 0 at "
+ + "com.google.inject.ProvisionExceptionTest$A");
+ }
+ }
+
+ public void testMethodInjectionExceptions() {
+ try {
+ Guice.createInjector().getInstance(E.class);
+ fail();
+ }
+ catch (ProvisionException e) {
+ e.printStackTrace();
+ assertTrue(e.getCause() instanceof UnsupportedOperationException);
+ assertContains(e.getMessage(), "Error injecting method");
+ assertContains(e.getMessage(), "while locating "
+ + "com.google.inject.ProvisionExceptionTest$E");
+ }
+ }
+
+ public void testProviderExceptions() {
+ try {
+ Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bind(D.class).toProvider(DProvider.class);
+ }
+ }).getInstance(D.class);
+ fail();
+ }
+ catch (ProvisionException e) {
+ assertTrue(e.getCause() instanceof UnsupportedOperationException);
+ assertContains(e.getMessage(), "Error in custom provider");
+ assertContains(e.getMessage(), "while locating "
+ + "com.google.inject.ProvisionExceptionTest$D");
+ }
+ }
+
+ static class A {
+ @Inject
+ A(B b) { }
+ }
+ static class B {
+ @Inject C c;
+ }
+ static class C {
+ @Inject
+ void setD(D d) { }
+ }
+ static class D {
+ D() {
+ throw new UnsupportedOperationException();
+ }
+ }
+ static class E {
+ @Inject void setObject(Object o) {
+ throw new UnsupportedOperationException();
+ }
+ }
+ static class DProvider implements Provider<D> {
+ public D get() {
+ return new D();
+ }
+ }
+
+ private void assertContains(String text, String substring) {
+ assertTrue(String.format("Expected \"%s\" to contain substring \"%s\"",
+ text, substring), text.contains(substring));
+ }
+}
