Single.blockingGet() -> BlockingMultiObserver/ExceptionCatcher should not wrap Exceptions?

[marcottedan]

Version 2.1.9

Assuming this code:

return Single.error(new IllegalArgumentException("I'm the cause"));

I think there's a issue in the design of ExceptionCatcher.wrapOrThrow. When calling blockingGet(), it wraps any instance of Exception but leaves Runtime and Error as-is. So basically you throw an exception somewhere but the Rx framework bubbles it up with a wrapper you have to unwrap to get your initial cause.

/**
     * If the provided Throwable is an Error this method
     * throws it, otherwise returns a RuntimeException wrapping the error
     * if that error is a checked exception.
     * @param error the error to wrap or throw
     * @return the (wrapped) error
     */
    public static RuntimeException wrapOrThrow(Throwable error) {
        if (error instanceof Error) {
            throw (Error)error;
        }
        if (error instanceof RuntimeException) {
            return (RuntimeException)error;
        }
        return new RuntimeException(error);
    }

It's especially painful in unit tests, where you catch an Exception and have to sometime exception.getCause() and sometime only exception is good enough. So if your Single would be consumed by a Subscriber but you want to test it by blockingGet to ease your life, you end up with an extra coating of exception that won't be there in your real scenario, leading to glue code in tests that don't behave like the original code.

      Single<String> anySingle = Single.error(new IOException("I'm the cause"));
      try {
         anySingle.blockingGet();
      }
      // catch (RuntimeException e) {
      // Should fail, but will pass
      // Assert.assertTrue(e.getCause() instanceof IOException);
      // }
      catch (Exception e) {
         // Should pass, but will fail
         Assert.assertTrue(e instanceof IOException);
      }

The caller code that invokes the ExceptionCatcher is BlockingMultiObserver.

public T blockingGet() {
        if (getCount() != 0) {
            try {
                BlockingHelper.verifyNonBlocking();
                await();
            } catch (InterruptedException ex) {
                dispose();
                throw ExceptionHelper.wrapOrThrow(ex);
            }
        }
        Throwable ex = error;
        if (ex != null) {
            throw ExceptionHelper.wrapOrThrow(ex);
        }
        return value;
    }

I believe that the blockingGet() method should actually throws Exception

Is there any workaround for that?

Thank you very much

[marcottedan]

Google also deprecated the same exact behavior done here from Guava in Throwables.propagate: https://github.com/google/guava/wiki/Why-we-deprecated-Throwables.propagate

[akarnokd]

Having throws X where X has to be the widest exception type, Throwable, is equally painful. PR #5951 details the wrapping in these methods.

Is there any workaround for that?

• Don't use blockingGet for tests, use test() and TestObserver.assertXs.
• Have your own blocking consumer and declare the signature as you see fit.

Otherwise, the signature is now baked in and can't be changed as it breaks binary compatibility.

[JakeWharton]

It would only break source compatibility, not binary. I still wouldn't change it though.

[artem-zinnatullin]

Soo, I personally thank everyone (sarcasm) in this issue because you got me to interested and it's 5:34 am…

I've built 0-deps java library that allows throwing checked exceptions without wrapping and without declaring it in method description

https://github.com/artem-zinnatullin/java-uncheck-exceptions

It's not on maven central yet, but jar is available on releases page (or you can build it, see README)

TL;TR: yep, in Java source code you can't throw checked exception without wrapping or declaring it. But in Java bytecode you can, so I do it from bytecode (you can read more in README and check implementation).

If you like it, we can integrate that class into RxJava and maybe Guava later.

[JakeWharton]

You can use sneaky throws for this which is far simpler.

…

On Thu, Apr 19, 2018, 8:35 AM Artem Zinnatullin :slowpoke: < ***@***.***> wrote: Soo, I personally thank everyone (sarcasm) in this issue because you got me to interested and it's 5:34 am… I've built 0-deps java library that allows throwing checked exceptions *without wrapping* and *without declaring* it in method description https://github.com/artem-zinnatullin/java-uncheck-exceptions It's not on maven central yet, but jar is available on releases page <https://github.com/artem-zinnatullin/java-uncheck-exceptions/releases> (or you can build it, see README <https://github.com/artem-zinnatullin/java-uncheck-exceptions>) TL;TR: yep, in Java source code you can't throw checked exception without wrapping or declaring it. But in Java bytecode you can, so I do it from bytecode (you can read more in README <https://github.com/artem-zinnatullin/java-uncheck-exceptions> and check implementation). If you like it, we can integrate that class into RxJava and maybe Guava later. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5965 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEWhUeW-7x720Sjw55ChRteDvDqeWks5tqISJgaJpZM4TawSc> .

[artem-zinnatullin]

Yep, but it's Lombok we're talking about, I don't know many people who'd like to have it in the project

I should mention it in the readme tho 👍

[JakeWharton]

Sneaky throws is not lombok. It's a 10-year-old generic trick. I'll find it when I get to a computer.

…

On Thu, Apr 19, 2018, 8:44 AM Artem Zinnatullin :slowpoke: < ***@***.***> wrote: Yep, but it's Lombok we're talking about, I don't know many people who'd like to have it in the project I should mention it in the readme tho 👍 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5965 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEcd9jk9XmvfE58Wb2a2ZU0x8meX9ks5tqIaZgaJpZM4TawSc> .

[vanniktech]

OkIo is using it https://github.com/square/okio/blob/4ae65b82e301f958cd4f04ba20b964a694267f64/okio/src/main/java/okio/Util.java#L64

public static void sneakyRethrow(Throwable t) {
  Util.<Error>sneakyThrow2(t);
}

@SuppressWarnings("unchecked")
private static <T extends Throwable> void sneakyThrow2(Throwable t) throws T {
  throw (T) t;
}

[artem-zinnatullin]

Yep, Jesse sent it too, what a facepalm then 🤦‍♂️

It was even discussed before in RxJava

• Modify Consumer accept() throw Exception to throw Throwable instead. #5206
• 1.x: operator mapIO which is like map but allows checked exceptions #3499
• Throw the checked exception without declaring it. #644

Well, that was a fun exercise I guess…