Error handling. Now if InjectionListeners or InjectableTypeListeners fail, we include those errors in our pretty error report.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@910 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/ConstructorInjector.java b/src/com/google/inject/ConstructorInjector.java
index 7176474..13ff143 100644
--- a/src/com/google/inject/ConstructorInjector.java
+++ b/src/com/google/inject/ConstructorInjector.java
@@ -95,7 +95,12 @@
}
for (InjectionListener<? super T> injectionListener : injectionListeners) {
- injectionListener.afterInjection(t); // TODO: handle user exceptions here
+ try {
+ injectionListener.afterInjection(t);
+ } catch (RuntimeException e) {
+ throw errors.withSource(constructionProxy.getInjectionPoint())
+ .errorNotifyingInjectionListener(injectionListener, injectableType, e).toException();
+ }
}
return t;
diff --git a/src/com/google/inject/ConstructorInjectorStore.java b/src/com/google/inject/ConstructorInjectorStore.java
index ff78e6d..147362c 100644
--- a/src/com/google/inject/ConstructorInjectorStore.java
+++ b/src/com/google/inject/ConstructorInjectorStore.java
@@ -64,8 +64,16 @@
private <T> ConstructorInjector<T> createConstructor(TypeLiteral<T> type, Errors errors)
throws ErrorsException {
+ int numErrorsBefore = errors.size();
- InjectionPoint injectionPoint = InjectionPoint.forConstructorOf(type);
+ InjectionPoint injectionPoint;
+ try {
+ injectionPoint = InjectionPoint.forConstructorOf(type);
+ } catch (ConfigurationException e) {
+ errors.merge(e.getErrorMessages());
+ throw errors.toException();
+ }
+
ImmutableList<SingleParameterInjector<?>> constructorParameterInjectors
= injector.getParametersInjectors(injectionPoint.getDependencies(), errors);
ImmutableList<SingleMemberInjector> memberInjectors = injector.injectors.get(type, errors);
@@ -83,8 +91,11 @@
for (InjectableTypeListenerBinding typeListener : injectableTypeListenerBindings) {
if (typeListener.getTypeMatcher().matches(type)) {
- // TODO: wrap this user code in a better try/catch block
- typeListener.getListener().hear(injectableType, encounter);
+ try {
+ typeListener.getListener().hear(injectableType, encounter);
+ } catch (RuntimeException e) {
+ errors.errorNotifyingTypeListener(typeListener, injectableType, e);
+ }
}
}
@@ -96,6 +107,8 @@
injectionPoint, type, injectableMembers, proxyFactory.getInterceptors());
}
+ errors.throwIfNewErrors(numErrorsBefore);
+
return new ConstructorInjector<T>(proxyFactory.create(), constructorParameterInjectors,
memberInjectors, encounter.getInjectionListeners(), injectableType);
}
@@ -193,5 +206,9 @@
return methodInterceptors;
}
/*end[AOP]*/
+
+ @Override public String toString() {
+ return type.toString();
+ }
}
}
diff --git a/src/com/google/inject/InjectorImpl.java b/src/com/google/inject/InjectorImpl.java
index a17fb40..db644e8 100644
--- a/src/com/google/inject/InjectorImpl.java
+++ b/src/com/google/inject/InjectorImpl.java
@@ -284,7 +284,7 @@
return new ConvertedConstantBindingImpl<T>(this, key, converted, stringBinding);
} catch (ErrorsException e) {
throw e;
- } catch (Exception e) {
+ } catch (RuntimeException e) {
throw errors.conversionError(stringValue, source, type, matchingConverter, e)
.toException();
}
diff --git a/src/com/google/inject/internal/Errors.java b/src/com/google/inject/internal/Errors.java
index ed781d5..130c087 100644
--- a/src/com/google/inject/internal/Errors.java
+++ b/src/com/google/inject/internal/Errors.java
@@ -24,6 +24,9 @@
import com.google.inject.Scope;
import com.google.inject.TypeLiteral;
import com.google.inject.spi.Dependency;
+import com.google.inject.spi.InjectableType;
+import com.google.inject.spi.InjectableTypeListenerBinding;
+import com.google.inject.spi.InjectionListener;
import com.google.inject.spi.InjectionPoint;
import com.google.inject.spi.Message;
import java.io.PrintWriter;
@@ -125,8 +128,8 @@
}
public Errors conversionError(String stringValue, Object source,
- TypeLiteral<?> type, MatcherAndConverter matchingConverter, Exception cause) {
- return addMessage(cause, "Error converting '%s' (bound at %s) to %s%n"
+ TypeLiteral<?> type, MatcherAndConverter matchingConverter, RuntimeException cause) {
+ return errorInUserCode(cause, "Error converting '%s' (bound at %s) to %s%n"
+ " using %s.%n"
+ " Reason: %s",
stringValue, convert(source), type, matchingConverter, cause);
@@ -248,15 +251,29 @@
}
public Errors errorInjectingMethod(Throwable cause) {
- return errorInUserCode("Error injecting method, %s", cause);
+ return errorInUserCode(cause, "Error injecting method, %s", cause);
+ }
+
+ public Errors errorNotifyingTypeListener(InjectableTypeListenerBinding listener,
+ InjectableType<?> injectableType, Throwable cause) {
+ return errorInUserCode(cause,
+ "Error notifying InjectableType.Listener %s (bound at %s) of %s.%n"
+ + " Reason: %s",
+ listener.getListener(), convert(listener.getSource()), injectableType, cause);
}
public Errors errorInjectingConstructor(Throwable cause) {
- return errorInUserCode("Error injecting constructor, %s", cause);
+ return errorInUserCode(cause, "Error injecting constructor, %s", cause);
}
public Errors errorInProvider(RuntimeException runtimeException) {
- return errorInUserCode("Error in custom provider, %s", runtimeException);
+ return errorInUserCode(runtimeException, "Error in custom provider, %s", runtimeException);
+ }
+
+ public Errors errorNotifyingInjectionListener(
+ InjectionListener listener, InjectableType<?> injectableType, RuntimeException cause) {
+ return errorInUserCode(cause, "Error notifying InjectionListener %s of %s.%n"
+ + " Reason: %s", listener, injectableType, cause);
}
public void exposedButNotBound(Key<?> key) {
@@ -275,13 +292,13 @@
}
}
- public Errors errorInUserCode(String message, Throwable cause) {
+ public Errors errorInUserCode(Throwable cause, String messageFormat, Object... arguments) {
Collection<Message> messages = getMessagesFromThrowable(cause);
if (!messages.isEmpty()) {
return merge(messages);
} else {
- return addMessage(cause, message, cause);
+ return addMessage(cause, messageFormat, arguments);
}
}
diff --git a/test/com/google/inject/AllTests.java b/test/com/google/inject/AllTests.java
index 3999405..d9d61d2 100644
--- a/test/com/google/inject/AllTests.java
+++ b/test/com/google/inject/AllTests.java
@@ -30,6 +30,7 @@
import com.google.inject.spi.ModuleWriterTest;
import com.google.inject.spi.ProviderMethodsTest;
import com.google.inject.spi.SpiBindingsTest;
+import com.google.inject.spi.BindingTargetVisitorTest;
import com.google.inject.util.ProvidersTest;
import com.google.inject.util.TypesTest;
import junit.framework.Test;
@@ -51,18 +52,18 @@
suite.addTestSuite(BoundInstanceInjectionTest.class);
suite.addTestSuite(BoundProviderTest.class);
suite.addTestSuite(CircularDependencyTest.class);
- suite.addTestSuite(TypeConversionTest.class);
- // suite.addTestSuite(ErrorHandlingTest.class); not a testcase
+ // ErrorHandlingTest.class is not a testcase
suite.addTestSuite(EagerSingletonTest.class);
suite.addTestSuite(GenericInjectionTest.class);
suite.addTestSuite(ImplicitBindingTest.class);
suite.addTestSuite(InjectableTypeListenerTest.class);
- suite.addTestSuite(InjectionPointTest.class);
suite.addTestSuite(InjectorTest.class);
+ // IntegrationTest is AOP-only
suite.addTestSuite(KeyTest.class);
suite.addTestSuite(LoggerInjectionTest.class);
- suite.addTestSuite(ModuleTest.class);
+ // MethodInterceptionTest is AOP-only
suite.addTestSuite(ModulesTest.class);
+ suite.addTestSuite(ModuleTest.class);
suite.addTestSuite(NullableInjectionPointTest.class);
suite.addTestSuite(OptionalBindingTest.class);
suite.addTestSuite(OverrideModuleTest.class);
@@ -70,29 +71,22 @@
suite.addTestSuite(PrivateModuleTest.class);
suite.addTestSuite(ProviderInjectionTest.class);
suite.addTestSuite(ProvisionExceptionTest.class);
+ // ProxyFactoryTest is AOP-only
suite.addTestSuite(ReflectionTest.class);
+ suite.addTestSuite(RequestInjectionTest.class);
suite.addTestSuite(ScopesTest.class);
suite.addTestSuite(SerializationTest.class);
- suite.addTestSuite(RequestInjectionTest.class);
suite.addTestSuite(SuperclassTest.class);
+ suite.addTestSuite(TypeConversionTest.class);
suite.addTestSuite(TypeLiteralInjectionTest.class);
suite.addTestSuite(TypeLiteralTest.class);
suite.addTestSuite(TypeLiteralTypeResolutionTest.class);
- // spi
- suite.addTestSuite(ElementsTest.class);
- suite.addTestSuite(HasDependenciesTest.class);
- suite.addTestSuite(ProviderMethodsTest.class);
- suite.addTestSuite(ModuleWriterTest.class);
- suite.addTestSuite(ModuleRewriterTest.class);
- suite.addTestSuite(SpiBindingsTest.class);
-
// internal
suite.addTestSuite(FinalizableReferenceQueueTest.class);
+ suite.addTestSuite(Jsr166HashMapTest.class);
suite.addTestSuite(LineNumbersTest.class);
suite.addTest(MapMakerTestSuite.suite());
- suite.addTestSuite(Jsr166HashMapTest.class);
- suite.addTestSuite(TypesTest.class);
suite.addTestSuite(UniqueAnnotationsTest.class);
// matcher
@@ -101,11 +95,22 @@
// names
suite.addTestSuite(NamesTest.class);
+ // spi
+ suite.addTestSuite(BindingTargetVisitorTest.class);
+ suite.addTestSuite(ElementsTest.class);
+ suite.addTestSuite(HasDependenciesTest.class);
+ suite.addTestSuite(InjectionPointTest.class);
+ suite.addTestSuite(ModuleRewriterTest.class);
+ suite.addTestSuite(ModuleWriterTest.class);
+ suite.addTestSuite(ProviderMethodsTest.class);
+ suite.addTestSuite(SpiBindingsTest.class);
+
// tools
// suite.addTestSuite(JmxTest.class); not a testcase
// util
suite.addTestSuite(ProvidersTest.class);
+ suite.addTestSuite(TypesTest.class);
/*if[AOP]*/
suite.addTestSuite(ProxyFactoryTest.class);
diff --git a/test/com/google/inject/InjectableTypeListenerTest.java b/test/com/google/inject/InjectableTypeListenerTest.java
index c5de01f..e178aa8 100644
--- a/test/com/google/inject/InjectableTypeListenerTest.java
+++ b/test/com/google/inject/InjectableTypeListenerTest.java
@@ -25,6 +25,7 @@
import com.google.inject.spi.InjectableType;
import com.google.inject.spi.InjectableType.Encounter;
import com.google.inject.spi.InjectionListener;
+import static com.google.inject.Asserts.assertContains;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.util.List;
@@ -131,9 +132,129 @@
assertEquals("felibeep", c.beep());
}
- // TODO(jessewilson): injectableType.Listener throws test
+ public void testInjectableTypeListenerThrows() {
+ final InjectableType.Listener clumsyListener = new InjectableType.Listener() {
+ int failures = 0;
- // TODO(jessewilson): injectionlistener throws test
+ public <I> void hear(InjectableType<I> injectableType, Encounter<I> encounter) {
+ throw new ClassCastException("whoops, failure #" + (++failures));
+ }
+
+ @Override public String toString() {
+ return "clumsy";
+ }
+ };
+
+ try {
+ Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bindListener(any(), clumsyListener);
+ bind(B.class);
+ bind(C.class);
+ }
+ });
+ fail();
+ } catch (CreationException expected) {
+ assertContains(expected.getMessage(),
+ "1) Error notifying InjectableType.Listener clumsy (bound at " + getClass().getName(),
+ ".configure(InjectableTypeListenerTest.java:",
+ "of " + B.class.getName(),
+ "Reason: java.lang.ClassCastException: whoops, failure #1",
+ "2) Error notifying InjectableType.Listener clumsy (bound at " + getClass().getName(),
+ ".configure(InjectableTypeListenerTest.java:",
+ "of " + C.class.getName(),
+ "Reason: java.lang.ClassCastException: whoops, failure #2");
+ }
+
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bindListener(any(), clumsyListener);
+ }
+ });
+ try {
+ injector.getProvider(B.class);
+ fail();
+ } catch (ConfigurationException expected) {
+ assertContains(expected.getMessage(),
+ "1) Error notifying InjectableType.Listener clumsy (bound at " + getClass().getName(),
+ ".configure(InjectableTypeListenerTest.java:",
+ "of " + B.class.getName(),
+ "Reason: java.lang.ClassCastException: whoops, failure #3");
+ }
+
+ // getting it again should yield the same exception #3
+ try {
+ injector.getInstance(B.class);
+ fail();
+ } catch (ConfigurationException expected) {
+ assertContains(expected.getMessage(),
+ "1) Error notifying InjectableType.Listener clumsy (bound at " + getClass().getName(),
+ ".configure(InjectableTypeListenerTest.java:",
+ "of " + B.class.getName(),
+ "Reason: java.lang.ClassCastException: whoops, failure #3");
+ }
+
+ // non-constructed types do not participate
+ assertSame(Stage.DEVELOPMENT, injector.getInstance(Stage.class));
+ }
+
+ public void testInjectionListenerThrows() {
+ final InjectionListener<Object> injectionListener = new InjectionListener<Object>() {
+ int failures = 0;
+
+ public void afterInjection(Object injectee) {
+ throw new ClassCastException("whoops, failure #" + (++failures));
+ }
+
+ @Override public String toString() {
+ return "goofy";
+ }
+ };
+
+ Injector injector = Guice.createInjector(new AbstractModule() {
+ protected void configure() {
+ bindListener(any(), new InjectableType.Listener() {
+ public <I> void hear(InjectableType<I> injectableType, Encounter<I> encounter) {
+ encounter.register(injectionListener);
+ }
+ });
+ bind(B.class);
+ }
+ });
+
+ try {
+ injector.getInstance(A.class);
+ fail();
+ } catch (ProvisionException e) {
+ assertContains(e.getMessage(),
+ "1) Error notifying InjectionListener goofy of " + A.class.getName(),
+ " Reason: java.lang.ClassCastException: whoops, failure #1");
+ }
+
+ // second time through should be a new cause (#2)
+ try {
+ injector.getInstance(A.class);
+ fail();
+ } catch (ProvisionException e) {
+ assertContains(e.getMessage(),
+ "1) Error notifying InjectionListener goofy of " + A.class.getName(),
+ " Reason: java.lang.ClassCastException: whoops, failure #2");
+ }
+
+ // we should get errors for all types, but only on getInstance()
+ Provider<B> bProvider = injector.getProvider(B.class);
+ try {
+ bProvider.get();
+ fail();
+ } catch (ProvisionException e) {
+ assertContains(e.getMessage(),
+ "1) Error notifying InjectionListener goofy of " + B.class.getName(),
+ " Reason: java.lang.ClassCastException: whoops, failure #3");
+ }
+
+ // non-constructed types do not participate
+ assertSame(Stage.DEVELOPMENT, injector.getInstance(Stage.class));
+ }
static class A {
@Inject Injector injector;
