cujojs / meld

AOP for JS with before, around, on, afterReturning, afterThrowing, after advice, and pointcuts
Other
644 stars 65 forks source link

Feature: around/after/afterReturning could auto-return the original return value #24

Closed jasonkarns closed 11 years ago

jasonkarns commented 11 years ago

It would be nice if there were a flavor of the around/after/afterReturning advice that didn't alter the return value. One that saved the 'proceed' return value and returned it automatically, without requiring the advice to do it manually.

I don't have any good ideas on how that would look in the API, though. I don't think it warrants its own pointcut method, but perhaps an option passed during setup?

briancavalier commented 11 years ago

@jasonkarns That's actually how after and afterThrowing work now. They don't alter the outcome or result of the original method (unless they themselves throw). Around is a more involved thing--it provides the most power, but also carries the most responsibility, and it stacks differently than other advices. It stacks like onion layers, completely enveloping either the original function or the previously added around advice. It would be tricky (but potentially doable) to implement a variant of around that preserved the original.

It's actually not quite clear what "original" would mean in that scenario. Consider this:

var myThing = new Thing();
// Inner around advice
meld.around(myThing, 'doSomething', function(joinpoint) {
    var originalResult = joinpoint.proceed();
    return somethingElse;
});
// Outer around advice
meld.aroundThatPreservesTheOriginalReturnValue(myThing, 'doSomething', function(joinpoint) {
    joinpoint.proceed();
    return yetAnotherValue;
});

// What should result be?
var result = myThing.doSomething();

The simplest thing I suppose would be that result would always be somethingElse. IOW, aroundThatPreservesTheOriginalReturnValue might simply return the result of the previous layer of around advice, discarding its own return value.

I think I'd need to see some compelling use cases for that kind of auto-passthrough around advice, though. Did you have any in mind?

jasonkarns commented 11 years ago

Ah, I didn't realize that after already did that! (will have to add that to the documentation!)

My current use case is profiling, though logging would be in the same situation. In both of these cases, the advice is only there to gather metrics and not to modify behavior. (Neither the ingoing arguments nor the outgoing result values should be modified.)

briancavalier commented 11 years ago

will have to add that to the documentation!

Sorry! It is indeed missing from the docs. Thanks, I'll open another issue for that.

My current use case is profiling, though logging would be in the same situation

The example trace aspect shows how you can use before, afterReturning, and afterThrowing advice to do this kind of thing. Also, wire's tracer aspect uses around advice to do something similar. Neither alters the parameters, return value, or thrown exception of the original method (in the case of wire's tracer, like we discussed above, it uses around, so simply doesn't alter the anything that passes through it's around "layer")

Hopefully those will give you some ideas for your own profiling/logging aspects.

briancavalier commented 11 years ago

Created #25 for updating the after* docs.

scothis commented 11 years ago

@jasonkarns you may be interested in checking out https://github.com/s2js/probes as a profiler based on meld

briancavalier commented 11 years ago

Closing this, see #25