New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 810430 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 781754



Sign in to add a comment

Find a generic way to unwrap exceptions thrown across the WebView support library boundary

Project Member Reported by gsennton@chromium.org, Feb 8 2018

Issue description

Whenever an InvocationHandler used in a Proxy implementation throws an exception that wasn't declared in the corresponding method signature, the Proxy throws an UndeclaredThrowableException wrapping the original exception.

We should unwrap the original exception and rethrow that.

I'm not entirely sure how to do this in a generic way, UndeclaredThrowableException.getUndeclaredThrowable() just returns a Throwable so we can't just rethrow that (since it would require all our method signatures to declare a Throwable to be thrown).
 

Comment 1 by torne@chromium.org, Feb 8 2018

I'm not sure why we should?

It shouldn't be possible for the InvocationHandler to end up throwing a checked exception that isn't declared in the original method signature - that would be a bug in the InvocationHandler if so. Do you have a specific example of this happening?
I think what's happening is that because we are throwing an exception within method.invoke(), method.invoke() is wrapping the original exception in a  
java.lang.reflect.InvocationTargetException (which is not declared to be thrown in the method we want to implement), so this is probably just a question of catching InvocationTargetException and unwrapping the original exception from within InvocationHandler.invoke().

Comment 3 by torne@chromium.org, Feb 8 2018

Yeah, you need to do that (as my recent CL for the font preloading does). You also probably need to catch all the *other* checked reflection exceptions (which you can't do with ReflectiveOperationException pre-L, I think?) and wrap those in RuntimeException.
Awesome, thanks Torne!
For my own future reference, when you're talking about catching reflection exceptions you're referring to
https://chromium-review.googlesource.com/c/chromium/src/+/905106/3/android_webview/glue/java/src/com/android/webview/chromium/FontPreloadingWorkaround.java

Comment 5 by torne@chromium.org, Feb 8 2018

Yeah. There I'm just catching ReflectiveOperationException which is the superclass of all the reflection exceptions, but that was new in Java 7 so can only be used on L and up. So, any code in the support library that needs to be usable pre-L can't do that and will need to catch either Exception, or all the types separately. I guess most things in the support library will only be invoked on L+, though..
Status: Fixed (was: Assigned)
We should be handling this correctly as per CL
https://chromium-review.googlesource.com/c/chromium/src/+/919062

And yeah, we shouldn't be using InvocationHandlers anyway pre-L (that's a code-path we just shouldn't get into).

Sign in to add a comment