Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-dev] around advice & exceptions 
Wes Isberg wrote:


First, a side note: your proxy pattern invokes the method directly,
rather than using proceed(..). As a result, any less-precedent advice
will not run. It's better to close over the proceed(..) invocation
and pass that to the method, perhaps using a nested class that
permits the callee to modify the context used to proceed.  Doing
that will cause other advice to run properly and also works for
join points besides method-execution.
Aha. You helped me to realize that I didnt' need to use reflection alltogether! The following handles my use case (except the exceptions, so to speak) 

  Object around(Object o) : proxyOperations(o)
  {
    return proceed(delegate == null ? o : delegate);
  }

I dont know what you mean by "...close over the proceed(..) invocation
and pass that to the method, perhaps using a nested class that
permits the callee to modify the context used to proceed." Please explain if you have a moment to spare.
In theory, your proposal doesn't expand what the join point can throw, so any advice or code that gets control in an exception handler should be expecting the exception. I'd want to verify that.
This was exactly my intention: unwrap the exceptions that are expected in the join point context and nothing more. An attempt to unwrap unexpected exception raises an Error. 

> (I'm less happy about using "throws
UnwrappableException" to mean "this advice unwraps any UnwrappableException it can" but I think that's a separate issue.)
I'm not too happy about the name either. It was the first thing that came to my mind. I'll let native english speakers come up with a name that suits the idea better ;-)
Also, doing this exception macro thingy at each join point would entail every implementation of AspectJ doing the same thing, so we might 
be committing them to a particular implementation of around advice.
First, I think the contract could be expanded to before and after advice too. Second, I don't see obvious implemention constraints (maybe because I don't know the implementation). Of course it is an extension of the language, and if an implementation does not support it, the UnwrappedException will be treaded as an ordinary exception and the compiler will complain that the piece of advice declares an exception that is not excpected in the join point context (if I understand correctly). 


I'm not sure yet why you need to catch and rethrow the exceptions.
You seem to be using reflection (and having to unwrap things like
InvocationTargetException) rather than just changing the context
using proceed.  E.g.,

    Object around(Object original)  : proxied(original) {
        Object result = originalToProxy.get(original);
        return proceed(null == result ? original : result);
    }


Umm, that's exactly what I wrote after reading the first paragraph!
Well, maybe I was under a false impression that I need to catch/rethrow any checked exceptions that may be thrown in the proxied method. I would have to do that in reflection scenario, but you made me realize I don't need it.
I made a quick test on my modified code and I found out that around advice work seamlesly with the various checked exceptions that the advised mehtods are throwing! Yipee! I can go on with my toy project instead of commiting to AspectJ development ;-) 


(Note that you can get a lot of benefit from using the supertype of
some context object, because it enables you to pick out join points
whose context objects are different types but share the same supertype.
However, you will cause a ClassCastException if your replacement object
is not the right type for this join point.)
Understood. In my particular scenario, I will be using replacements that are always the same type as the original object, created using clone() method. 

> If your NPE is reproducible in the latest version of AspectJ,
> please submit it as a bug.

I'll whip up a minimal test case and submit it tomorrow morning.

Thanks for the explainations!

Rafal

Back to the top