Fixed
Status Update
No update yet.
Comments
da...@google.com
Hey - In your example you are not returning any value from your transaction so instead of using runInTransaction that takes a Callable use the one that takes a Runnable: runInTransaction(@NonNull Runnable body). The version that takes a Runnable doesn't do any exception transformation and is equivalent to just calling begin-try-finally-end code. See: https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/room/runtime/src/main/java/androidx/room/RoomDatabase.java#355
For the Callable version, you are right that it might be bad for us to wrap checked exceptions in Runtime exceptions, this is unfortunate but its because the Callable interface declares that the method call() throws Exception. Other functional interfaces, like java.util.function.Supplier are not available in older APIs.
For the Callable version, you are right that it might be bad for us to wrap checked exceptions in Runtime exceptions, this is unfortunate but its because the Callable interface declares that the method call() throws Exception. Other functional interfaces, like java.util.function.Supplier are not available in older APIs.
kp...@gmail.com
Hello! Thanks for your ideas, but...
1. It was just a bad example (sorry) - I need return value in some cases too. So Runnable is sometimes not the option.
2. Example with Runnable won't compile with outer try...catch (close to the example above). The problem will be removed only when try...catch is moved inside of Runnable. And that way it would be a bit tricky to do some logic inside or rethrow a warning.
1. It was just a bad example (sorry) - I need return value in some cases too. So Runnable is sometimes not the option.
2. Example with Runnable won't compile with outer try...catch (close to the example above). The problem will be removed only when try...catch is moved inside of Runnable. And that way it would be a bit tricky to do some logic inside or rethrow a warning.
da...@google.com
Ah gotcha, I think I now understand further your issue.
I think we can agree this is not a problem for unchecked exceptions, those will simply propagate up the stack and you can add your try-catch anywhere in the trace.
For checked exceptions, you are limited by Java's exception handling in lambda expressions. That is, because the functional interface doesn't declare the exception, you'll need to handle it in the lambda itself, because in theory, the Runnable / Callable might execute in another thread (even though it doesn't) and then your try-catch doesn't correctly handle the checked exception. I agree, this is not great and if you look around online for this issue you'll find a few clever solutions and some explanations of this unfortunate combination of functional programming style and checked exceptions in Java.
Assuming you are using Java 8 features, one of the clever solutions is to create a ThrowableRunnable that extends Runnable and that uses the 'sneaky throw' technique. For example:
```
interface ThrowingRunnable extends Runnable {
@Override
default void run() {
try {
doRun();
} catch (Exception e) {
sneakyThrow(e);
}
}
void doRun() throws Exception;
}
@NonNull
@SuppressWarnings("unchecked")
static <E extends Throwable> void sneakyThrow(@NonNull Throwable e) throws E {
throw (E) e;
}
```
with usage:
```
mDatabase.runInTransaction((ThrowingRunnable)() -> {
// do things here that might throw checked exceptions
});
```
Note that this still means you can't wrap runInTransaction() in a try-catch using a checked exception, the compiler will complain that the checked exception is never thrown in the corresponding block. However if the method that call runInTransaction() declares that it throws a checked exception and that exception is indeed thrown from the lambda then you'll be able to catch it without unwrapping it.
Another (an even better) solution is to use Kotlin, since it doesn't have checked exceptions. :) (https://kotlinlang.org/docs/reference/exceptions.html#checked-exceptions )
```
try {
database.runInTransaction {
// Checked exceptions thrown here can be caught by type in the wrapping try-catch
}
} catch (ex: JSONException) {
// Yay Kotlin!
}
```
Otherwise feel free to simply use begin / end transaction methods and suppress the deprecated usage warning.
I still think its wrong on us to wrap the checked exception in a RuntimeException in the Callable version, I'm looking into how to fix this, if we can or at least document it. So in the mean time I'll keep this issue open.
I think we can agree this is not a problem for unchecked exceptions, those will simply propagate up the stack and you can add your try-catch anywhere in the trace.
For checked exceptions, you are limited by Java's exception handling in lambda expressions. That is, because the functional interface doesn't declare the exception, you'll need to handle it in the lambda itself, because in theory, the Runnable / Callable might execute in another thread (even though it doesn't) and then your try-catch doesn't correctly handle the checked exception. I agree, this is not great and if you look around online for this issue you'll find a few clever solutions and some explanations of this unfortunate combination of functional programming style and checked exceptions in Java.
Assuming you are using Java 8 features, one of the clever solutions is to create a ThrowableRunnable that extends Runnable and that uses the 'sneaky throw' technique. For example:
```
interface ThrowingRunnable extends Runnable {
@Override
default void run() {
try {
doRun();
} catch (Exception e) {
sneakyThrow(e);
}
}
void doRun() throws Exception;
}
@NonNull
@SuppressWarnings("unchecked")
static <E extends Throwable> void sneakyThrow(@NonNull Throwable e) throws E {
throw (E) e;
}
```
with usage:
```
mDatabase.runInTransaction((ThrowingRunnable)() -> {
// do things here that might throw checked exceptions
});
```
Note that this still means you can't wrap runInTransaction() in a try-catch using a checked exception, the compiler will complain that the checked exception is never thrown in the corresponding block. However if the method that call runInTransaction() declares that it throws a checked exception and that exception is indeed thrown from the lambda then you'll be able to catch it without unwrapping it.
Another (an even better) solution is to use Kotlin, since it doesn't have checked exceptions. :) (
```
try {
database.runInTransaction {
// Checked exceptions thrown here can be caught by type in the wrapping try-catch
}
} catch (ex: JSONException) {
// Yay Kotlin!
}
```
Otherwise feel free to simply use begin / end transaction methods and suppress the deprecated usage warning.
I still think its wrong on us to wrap the checked exception in a RuntimeException in the Callable version, I'm looking into how to fix this, if we can or at least document it. So in the mean time I'll keep this issue open.
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit ec088a43ebfc8c32abfc9e631f08a50d1d3085b0
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Tue Mar 19 08:51:53 2019
Use a sneaky throw in runInTransaction(Callable)
Using's Java8 sneaky throw technique allows us to not have to wrap
checked exceptions in RuntimeExceptions, meaning callers with exception
catches don't need to unwrap the exception.
Since this is only a Java8 compiler technique (bytecode doesn't
distinguish between unchecked and checked exceptions), then we
circumvent bumping Room's minSdk by simply jarring the unpublished
room-common-java8 library.
Bug: 128623748
Test: SneakyThrowTests in API 15
Change-Id: I1611bd776d1f3030b95ba57586d89a7d1742fbe8
M buildSrc/src/main/kotlin/androidx/build/PublishDocsRules.kt
A room/common-java8/build.gradle
A room/common-java8/src/main/java/androidx/room/util/SneakyThrow.java
A room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/SneakyThrowTest.kt
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/SneakyThrowTest.java
M room/runtime/build.gradle
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
M settings.gradle
https://android-review.googlesource.com/930325
https://goto.google.com/android-sha1/ec088a43ebfc8c32abfc9e631f08a50d1d3085b0
Branch: androidx-master-dev
commit ec088a43ebfc8c32abfc9e631f08a50d1d3085b0
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Tue Mar 19 08:51:53 2019
Use a sneaky throw in runInTransaction(Callable)
Using's Java8 sneaky throw technique allows us to not have to wrap
checked exceptions in RuntimeExceptions, meaning callers with exception
catches don't need to unwrap the exception.
Since this is only a Java8 compiler technique (bytecode doesn't
distinguish between unchecked and checked exceptions), then we
circumvent bumping Room's minSdk by simply jarring the unpublished
room-common-java8 library.
Bug: 128623748
Test: SneakyThrowTests in API 15
Change-Id: I1611bd776d1f3030b95ba57586d89a7d1742fbe8
M buildSrc/src/main/kotlin/androidx/build/PublishDocsRules.kt
A room/common-java8/build.gradle
A room/common-java8/src/main/java/androidx/room/util/SneakyThrow.java
A room/integration-tests/kotlintestapp/src/androidTest/java/androidx/room/integration/kotlintestapp/test/SneakyThrowTest.kt
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/SneakyThrowTest.java
M room/runtime/build.gradle
M room/runtime/src/main/java/androidx/room/RoomDatabase.java
M settings.gradle
Description
Version used: Room 2.0.0-alpha05
Devices/Android versions reproduced on: Any
Recent deprecations influence business logic flow a lot. Example: remote server's response parsing (I think, rather common case).
Old way:
// JSONException is thrown upper, so it can be managed alltogether with HTTP response errors (for example, just ignored to use old data on unexpected response format)
public void parseResponse(final JSONObject obj, final JSONArray subObjs) throws JSONException
{
db.beginTransaction();
try {
final DBItem item = db.getDaoItem().findItem(obj.getLong("id"));
item.parseSomehow(obj); // throws JSONException on failure
final DBSubItem[] subitems = db.getDaoSubItem().findSubItems(item.getId());
db.getDaoSubItem().replaceSubItems(item.getId(), DBSubItem.parseSomehow(subObjs)); // replaces, reusing subitems with updating inner order; throws JSONException on failure
db.setTransactionSuccessful();
} finally {
db.endTransaction();
}
}
New way (looks worse, no exception type control - so you can throw anything and check it only in runtime):
public void parseResponse(final JSONObject obj, final JSONArray subObjs) throws JSONException
{
try {
db.runInTransaction(() -> {
... // same code as in previous example inside try...catch
return null;
});
} catch (RuntimeException e) {
if (e.getCause() instanceof JSONException) {
throw (JSONException)e.getCause();
} else {
throw e;
}
}
}
Of course, I can try to separate parsing from database interaction, but it would require at least temporary objects with no more usage (in the project's code). Additionally, sometimes next parsing steps depend on previous / storage data.
Final thoughts. Looks like runInTransaction should be recommended method in most situations, but the old way is still convenient too.