Skip to content

Commit

Permalink
Reduce the amount of stack trace spam in error message. If all the er…
Browse files Browse the repository at this point in the history
…rors have

the same cause, don't list them individually and instead use as the Throwable's
cause.  If some (but not all) errors are duplicates, elide the duplicate stack
traces and point to the error # it's the same as.

"same" here is defined as the same exception class, same stack trace, and same causes (with causes using the same 'same' definition).
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=92389485
  • Loading branch information
sameb committed May 20, 2015
1 parent 86e8569 commit d53fe4b
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 20 deletions.
42 changes: 35 additions & 7 deletions core/src/com/google/inject/internal/Errors.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@

package com.google.inject.internal;

import com.google.common.base.Equivalence;
import com.google.common.base.Objects;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.inject.ConfigurationException;
Expand All @@ -43,18 +47,18 @@
import com.google.inject.spi.TypeConverterBinding;
import com.google.inject.spi.TypeListenerBinding;

import java.io.PrintWriter;
import java.io.Serializable;
import java.io.StringWriter;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Collection;
import java.util.Formatter;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.logging.Level;
Expand Down Expand Up @@ -575,8 +579,10 @@ public static String format(String heading, Collection<Message> errorMessages) {
int index = 1;
boolean displayCauses = getOnlyCause(errorMessages) == null;

Map<Equivalence.Wrapper<Throwable>, Integer> causes = Maps.newHashMap();
for (Message errorMessage : errorMessages) {
fmt.format("%s) %s%n", index++, errorMessage.getMessage());
int thisIdx = index++;
fmt.format("%s) %s%n", thisIdx, errorMessage.getMessage());

List<Object> dependencies = errorMessage.getSources();
for (int i = dependencies.size() - 1; i >= 0; i--) {
Expand All @@ -586,9 +592,15 @@ public static String format(String heading, Collection<Message> errorMessages) {

Throwable cause = errorMessage.getCause();
if (displayCauses && cause != null) {
StringWriter writer = new StringWriter();
cause.printStackTrace(new PrintWriter(writer));
fmt.format("Caused by: %s", writer.getBuffer());
Equivalence.Wrapper<Throwable> causeEquivalence = ThrowableEquivalence.INSTANCE.wrap(cause);
if (!causes.containsKey(causeEquivalence)) {
causes.put(causeEquivalence, thisIdx);
fmt.format("Caused by: %s", Throwables.getStackTraceAsString(cause));
} else {
int causeIdx = causes.get(causeEquivalence);
fmt.format("Caused by: %s (same stack trace as error #%s)",
cause.getClass().getName(), causeIdx);
}
}

fmt.format("%n");
Expand Down Expand Up @@ -662,7 +674,8 @@ public static Throwable getOnlyCause(Collection<Message> messages) {
continue;
}

if (onlyCause != null) {
if (onlyCause != null
&& !ThrowableEquivalence.INSTANCE.equivalent(onlyCause, messageCause)) {
return null;
}

Expand Down Expand Up @@ -839,4 +852,19 @@ public static void formatInjectionPoint(Formatter formatter, Dependency<?> depen
formatSource(formatter, injectionPoint.getMember());
}
}

static class ThrowableEquivalence extends Equivalence<Throwable> {
static final ThrowableEquivalence INSTANCE = new ThrowableEquivalence();

@Override protected boolean doEquivalent(Throwable a, Throwable b) {
return a.getClass().equals(b.getClass())
&& Objects.equal(a.getMessage(), b.getMessage())
&& Arrays.equals(a.getStackTrace(), b.getStackTrace())
&& equivalent(a.getCause(), b.getCause());
}

@Override protected int doHash(Throwable t) {
return Objects.hashCode(t.getClass().hashCode(), t.getMessage(), hash(t.getCause()));
}
}
}
132 changes: 125 additions & 7 deletions core/test/com/google/inject/ProvisionExceptionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import com.google.common.base.Throwables;
import com.google.inject.spi.Message;

import junit.framework.TestCase;

import java.io.IOException;
Expand Down Expand Up @@ -57,7 +60,7 @@ public void testExceptionsCollapsed() {
public void testExceptionsCollapsedWithScopes() {
try {
Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(B.class).in(Scopes.SINGLETON);
}
}).getInstance(A.class);
Expand Down Expand Up @@ -86,7 +89,7 @@ public void testMethodInjectionExceptions() {
public void testBindToProviderInstanceExceptions() {
try {
Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(D.class).toProvider(new DProvider());
}
}).getInstance(D.class);
Expand Down Expand Up @@ -116,7 +119,7 @@ public void testProvisionExceptionsAreWrappedForBindToType() {
public void testProvisionExceptionsAreWrappedForBindToProviderType() {
try {
Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(F.class).toProvider(FProvider.class);
}
}).getInstance(F.class);
Expand All @@ -131,7 +134,7 @@ protected void configure() {
public void testProvisionExceptionsAreWrappedForBindToProviderInstance() {
try {
Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(F.class).toProvider(new FProvider());
}
}).getInstance(F.class);
Expand Down Expand Up @@ -229,7 +232,7 @@ public void testBindingAnnotationsOnMethodsAndConstructors() {

public void testBindingAnnotationWarningForScala() {
Injector injector = Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(String.class).annotatedWith(Green.class).toInstance("lime!");
}
});
Expand All @@ -238,7 +241,7 @@ protected void configure() {

public void testLinkedBindings() {
Injector injector = Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(D.class).to(RealD.class);
}
});
Expand All @@ -256,7 +259,7 @@ protected void configure() {

public void testProviderKeyBindings() {
Injector injector = Guice.createInjector(new AbstractModule() {
protected void configure() {
@Override protected void configure() {
bind(D.class).toProvider(DProvider.class);
}
});
Expand All @@ -271,6 +274,121 @@ protected void configure() {
}
}

public void testDuplicateCausesCollapsed() {
final RuntimeException sharedException = new RuntimeException("fail");
Injector injector = Guice.createInjector(new AbstractModule() {
@Override protected void configure() {}

@Provides Integer i() { throw sharedException; }

@Provides Double d() { throw sharedException; }

});

try{
injector.getInstance(DoubleFailure.class);
fail();
} catch (ProvisionException pe) {
assertEquals(sharedException, pe.getCause());
assertEquals(2, pe.getErrorMessages().size());
for (Message message : pe.getErrorMessages()) {
assertEquals(sharedException, message.getCause());
}
}
}

public void testMultipleDuplicates() {
final RuntimeException exception1 = new RuntimeException("fail");
final RuntimeException exception2 = new RuntimeException("abort");
Injector injector = Guice.createInjector(new AbstractModule() {
@Override protected void configure() {}

@Provides Integer i() { throw exception1; }

@Provides Double d() { throw exception1; }

@Provides String s() { throw exception2; }

@Provides Number n() { throw exception2; }

});

try{
injector.getInstance(QuadrupleFailure.class);
fail();
} catch (ProvisionException pe) {
assertNull(pe.getCause());
assertEquals(4, pe.getErrorMessages().size());

String e1 = Throwables.getStackTraceAsString(exception1);
String e2 = Throwables.getStackTraceAsString(exception2);
assertContains(pe.getMessage(),
"\n1) ", e1, "\n2) ", "(same stack trace as error #1)",
"\n3) ", e2, "\n4) ", "(same stack trace as error #3)");
}
}

static class DoubleFailure {
@Inject DoubleFailure(Integer i, Double d) { }
}

static class QuadrupleFailure {
@Inject QuadrupleFailure(Integer i, Double d, String s, Number n) { }
}

public void testDuplicatesDifferentInstances() {
Injector injector = Guice.createInjector(new AbstractModule() {
@Override protected void configure() {}

@Provides Integer i() { throw new RuntimeException(); }

});

try{
injector.getInstance(DoubleSameFailure.class);
fail();
} catch (ProvisionException pe) {
assertNotNull(pe.toString(), pe.getCause());
assertEquals(2, pe.getErrorMessages().size());
for (Message message : pe.getErrorMessages()) {
assertNotNull(message.toString(), message.getCause());
}
}
}

public void testMultipleDuplicatesDifferentInstaces() {
Injector injector = Guice.createInjector(new AbstractModule() {
@Override protected void configure() {}

@Provides Integer i() { throw new RuntimeException("fail"); }

@Provides String s() { throw new RuntimeException("abort"); }

});

try{
injector.getInstance(QuadrupleSameFailure.class);
fail();
} catch (ProvisionException pe) {
assertNull(pe.getCause());
assertEquals(4, pe.getErrorMessages().size());

assertContains(pe.getMessage(),
"\n1) ", "Caused by: java.lang.RuntimeException: fail",
"\n2) ", "Caused by: java.lang.RuntimeException (same stack trace as error #1)",
"\n3) ", "Caused by: java.lang.RuntimeException: abort",
"\n4) ", "Caused by: java.lang.RuntimeException (same stack trace as error #3)");
}
}

static class DoubleSameFailure {
@Inject DoubleSameFailure(Integer i1, Integer i2) { }
}

static class QuadrupleSameFailure {
@Inject QuadrupleSameFailure(Integer i1, Integer i2, String s1, String s2) { }
}

private class InnerClass {}

static class A {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1480,11 +1480,11 @@ protected void configure() {
.providing(ProvisionExceptionFoo.class);
bindScope(BadScope.class, new Scope() {
@Override
public <T> Provider<T> scope(Key<T> key, Provider<T> unscoped) {
public <T> Provider<T> scope(final Key<T> key, Provider<T> unscoped) {
return new Provider<T>() {
@Override
public T get() {
throw new OutOfScopeException("failure");
throw new OutOfScopeException("failure: " + key.toString());
}
};
}
Expand All @@ -1498,10 +1498,10 @@ public T get() {
} catch(ProvisionException pe) {
assertEquals(2, pe.getErrorMessages().size());
List<Message> messages = Lists.newArrayList(pe.getErrorMessages());
assertEquals("Error in custom provider, com.google.inject.OutOfScopeException: failure",
messages.get(0).getMessage());
assertEquals("Error in custom provider, com.google.inject.OutOfScopeException: failure",
messages.get(1).getMessage());
assertEquals("Error in custom provider, com.google.inject.OutOfScopeException: failure: "
+ Key.get(Unscoped1.class), messages.get(0).getMessage());
assertEquals("Error in custom provider, com.google.inject.OutOfScopeException: failure: "
+ Key.get(Unscoped2.class), messages.get(1).getMessage());
}
}

Expand Down

0 comments on commit d53fe4b

Please sign in to comment.