Skip to content

Commit 585b93a

Browse files
cpovirkGoogle Java Core Libraries
authored andcommitted
Restore the behavior of throwing an IllegalArgumentException directly.
I'd undone this in cl/686728158 as part of addressing #7434, but I subsequently got a guilty conscience. It almost certainly doesn't matter, but it's a little weird. RELNOTES=n/a PiperOrigin-RevId: 686919814
1 parent dc423d2 commit 585b93a

File tree

4 files changed

+50
-14
lines changed

4 files changed

+50
-14
lines changed

android/guava-tests/test/com/google/common/eventbus/EventBusTest.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616

1717
package com.google.common.eventbus;
1818

19-
import static com.google.common.truth.Truth.assertThat;
2019
import static org.junit.Assert.assertThrows;
2120

2221
import com.google.common.collect.ImmutableList;
2322
import com.google.common.collect.Lists;
24-
import com.google.common.util.concurrent.UncheckedExecutionException;
2523
import java.util.List;
2624
import java.util.concurrent.ExecutorService;
2725
import java.util.concurrent.Executors;
@@ -283,10 +281,7 @@ class SubscribesToPrimitive {
283281
@Subscribe
284282
public void toInt(int i) {}
285283
}
286-
UncheckedExecutionException thrown =
287-
assertThrows(
288-
UncheckedExecutionException.class, () -> bus.register(new SubscribesToPrimitive()));
289-
assertThat(thrown).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
284+
assertThrows(IllegalArgumentException.class, () -> bus.register(new SubscribesToPrimitive()));
290285
}
291286

292287
/** Records thrown exception information. */

android/guava/src/com/google/common/eventbus/SubscriberRegistry.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.common.collect.Multimap;
3333
import com.google.common.primitives.Primitives;
3434
import com.google.common.reflect.TypeToken;
35+
import com.google.common.util.concurrent.UncheckedExecutionException;
3536
import com.google.j2objc.annotations.Weak;
3637
import java.lang.reflect.Method;
3738
import java.util.Arrays;
@@ -164,7 +165,29 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
164165
}
165166

166167
private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
167-
return subscriberMethodsCache.getUnchecked(clazz);
168+
try {
169+
return subscriberMethodsCache.getUnchecked(clazz);
170+
} catch (UncheckedExecutionException e) {
171+
if (e.getCause() instanceof IllegalArgumentException) {
172+
/*
173+
* IllegalArgumentException is the one unchecked exception that we know is likely to happen
174+
* (thanks to the checkArgument calls in getAnnotatedMethodsNotCached). If it happens, we'd
175+
* prefer to propagate an IllegalArgumentException to the caller. However, we don't want to
176+
* simply rethrow an exception (e.getCause()) that may in rare cases have come from another
177+
* thread. To accomplish both goals, we wrap that IllegalArgumentException in a new
178+
* instance.
179+
*/
180+
throw new IllegalArgumentException(e.getCause().getMessage(), e.getCause());
181+
}
182+
/*
183+
* If some other exception happened, we just propagate the wrapper
184+
* UncheckedExecutionException, which has the stack trace from this thread and which has its
185+
* cause set to the underlying exception (which may be from another thread). If we someday
186+
* learn that some other exception besides IllegalArgumentException is common, then we could
187+
* add another special case to throw an instance of it, too.
188+
*/
189+
throw e;
190+
}
168191
}
169192

170193
private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {

guava-tests/test/com/google/common/eventbus/EventBusTest.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616

1717
package com.google.common.eventbus;
1818

19-
import static com.google.common.truth.Truth.assertThat;
2019
import static org.junit.Assert.assertThrows;
2120

2221
import com.google.common.collect.ImmutableList;
2322
import com.google.common.collect.Lists;
24-
import com.google.common.util.concurrent.UncheckedExecutionException;
2523
import java.util.List;
2624
import java.util.concurrent.ExecutorService;
2725
import java.util.concurrent.Executors;
@@ -283,10 +281,7 @@ class SubscribesToPrimitive {
283281
@Subscribe
284282
public void toInt(int i) {}
285283
}
286-
UncheckedExecutionException thrown =
287-
assertThrows(
288-
UncheckedExecutionException.class, () -> bus.register(new SubscribesToPrimitive()));
289-
assertThat(thrown).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
284+
assertThrows(IllegalArgumentException.class, () -> bus.register(new SubscribesToPrimitive()));
290285
}
291286

292287
/** Records thrown exception information. */

guava/src/com/google/common/eventbus/SubscriberRegistry.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.common.collect.Multimap;
3333
import com.google.common.primitives.Primitives;
3434
import com.google.common.reflect.TypeToken;
35+
import com.google.common.util.concurrent.UncheckedExecutionException;
3536
import com.google.j2objc.annotations.Weak;
3637
import java.lang.reflect.Method;
3738
import java.util.Arrays;
@@ -164,7 +165,29 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
164165
}
165166

166167
private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
167-
return subscriberMethodsCache.getUnchecked(clazz);
168+
try {
169+
return subscriberMethodsCache.getUnchecked(clazz);
170+
} catch (UncheckedExecutionException e) {
171+
if (e.getCause() instanceof IllegalArgumentException) {
172+
/*
173+
* IllegalArgumentException is the one unchecked exception that we know is likely to happen
174+
* (thanks to the checkArgument calls in getAnnotatedMethodsNotCached). If it happens, we'd
175+
* prefer to propagate an IllegalArgumentException to the caller. However, we don't want to
176+
* simply rethrow an exception (e.getCause()) that may in rare cases have come from another
177+
* thread. To accomplish both goals, we wrap that IllegalArgumentException in a new
178+
* instance.
179+
*/
180+
throw new IllegalArgumentException(e.getCause().getMessage(), e.getCause());
181+
}
182+
/*
183+
* If some other exception happened, we just propagate the wrapper
184+
* UncheckedExecutionException, which has the stack trace from this thread and which has its
185+
* cause set to the underlying exception (which may be from another thread). If we someday
186+
* learn that some other exception besides IllegalArgumentException is common, then we could
187+
* add another special case to throw an instance of it, too.
188+
*/
189+
throw e;
190+
}
168191
}
169192

170193
private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {

0 commit comments

Comments
 (0)