On 11 August 2011 23:24, Tim King <timk@jtimothyking.com> wrote:
Hi, Damien. I have lots of comments on this issue, as I've just implemented an exception-handling mechanism for the project I'm working on. Some of the features you're considering could have made my job a lot easier.
Hi, First of all, thanks for taking time to read and reply. And sorry to hear that timing could have been better. However, now we can use your experience to refine the implementation, which is a great benefit :) Before I answer more precisely, I can tell you that the power-of-2 was an implementation idea thought as an alternative of OO inheritance, and it solved the "catch_exception should block other handlers" issue. However, after discussion with bigpresh and others, it occured to me that it was difficult to explain how to use these power-of-2 flags, and if it's troublesome for seasoned Dancer users, I guss it'll be unusable for newcomers. So I'll probably switch back to a more standard mechanism, like Exception::Base. But read below for the details.
CATCHING EXCEPTIONS
In the project that I'm working on, it would have been useful from a plugin to catch an exception thrown in a route handler. A "catch_exception" handler might have worked. But this isn't really a hook, because if a "catch_exception" handler consumes an exception, none on the other handlers in the chain should be called. It seems to me, what we're talking about here is a new concept: a "catch-point." It's like a hook in that it calls back to a handler sub, but it more closely mirrors try-catch semantics.
Hmm, I think I see what you mean, however you may want the catching code to not stop execution of other handlers. So for me, your "catch_exception" handler can be implemented with a on_exception hook. The code in the hook closure can raise a new exception or a halt (which raises an exception too). Exceptions in hooks will break other handlers execution (see code in Dancer::Hook), and will be caught in Dancer::Handler::render_request. The D::Exception PR https://github.com/sukria/Dancer/pull/587 first purpose was to fix the fact that halt didn't break handlers chain. It now does. The "power-of-2" mechanism would allow a catching code to simply catch the exception, add E_HALT to it, and rethrow it. Thus the request would be immediately rendered as it were with halt, and the template would be able to use the original exception code and message. That's were it somehow replace the inheritance need. But as I said earlier, this mechanism is probably too complex to use. So, my opinion is that your catch_exception is implementable with a after_exception hook, so it could fit nicely in the existing hook system. Please, if you see something that would make that statement invalid, let me know, as I'd rather tackle problems down now, rather than in 2 months :)
As you suggested, someone might also want to catch exceptions from other parts of the process, e.g., exceptions thrown during deserialization or serialization, or from within a hook. So it seems that there are *at least* two distinct points at which applications and plugins might want to catch exceptions:
* One is in $route->execute. A handler hooked into this catch-point would execute in the context of the handler, and could do anything the handler could do, including modify the response, send errors, return serializable data, and throw the same or different exceptions.
* The other is in Dancer::Handler::render_request. A handler hooked into this catch-point would be able to modify the response, including sending errors, but would not have access to the serializer or router. This handler should also be able to throw the same or other exceptions, to be caught by later handlers in the chain.
And they may want to catch non-Dancer exceptions, too. Just as Dancer has default handling in Dancer::Handler::render_request for non-Dancer exceptions, so also should any catch-point mechanism treat both Dancer and non-Dancer exceptions equally.
I don't see clearly why you'd need to catch exception at the two places. What is it you'd want to do in the execute catch_code that you couldn't do in the render_request catch_code ? Why couldn't render_request catch_code not change the serializer? Also, if render_request (re-)throw exceptions, I'm wondering where/by who it would be caught ? except handle_request that is in the caller stack, above that you're pretty much out of Dancer's code. Maybe I don't understand what you mean ?
OTHER IDEAS
Idea 2, try-catch route syntax, doesn't add any capability that wasn't implementable before. Might be nice for pretty syntax, but I don't particularly care for it.
Agreed, that's what I stated in my email. So as soon as we show in the doc how to catch using eval {}; and suggest Try::Tiny, there is no point having a special syntax.
I'd rather just use Try::Tiny, then in my route handler:
try { do_some_stuff; } catch { if (my $e_value = is_dancer_exception(my $e = $_)) { handle_dancer_exception($e, $e_value); } else { die $e; # rethrow exception } };
Idea 3, using the before_error hook, doesn't really seem like the right solution, because this is about catching exceptions, not about generating errors.
Agreed, I was referring at it in my email as a solution to avoid having an additional hook, by using before_error and shared data. But it's a hack and a bad idea.
Yes, by default, Dancer instantiates and renders an error on a caught exception, but there's no particular reason why that's the only way to handle an exception. In some cases, for example, the right way to respond to a particular exception might be to forward to a different path. Or to generate a non-error response.
And using shared data to communicate the exception is so very, very wrong.
Completely agree. You'd be happy that the situation you describe is currently being taken care of, and someone is going to devote a lot of work on it very soon. I won't say more to avoid spoiling :)
THE SESSION SYSTEM EXAMPLE
I haven't used the Dancer session system, and I'm using the latest released Dancer (not the devel branch). So I can't really knowledgeably comment on "Real life example 3." But it seems to me that this is a situation in which you'd want to use a meta-session class, analogous to Dancer::Serializer::Mutable.
Ok yes :) But I was examinating the situation as "let's see how a user could implement this behaviour with just exception", so no additional module That is, the mutable serializer chooses which serializer to use— in that
case, based on the content types that the request and the serializers support. Then Dancer::Serializer::Mutable proxies the API to that chosen serializer.
One might visualize a session meta-engine that tries each configured session engine, handing off control to that engine. As each engine threw E_SESSION, the meta-engine would catch it, trying each additional engine in turn, until it found one that accepted the request. The meta-engine itself would only raise E_SESSION when none of the configured engines could handle the request.
The meta-engine (if possible) could even cache the knowledge it learned from these trials, so that it wouldn't have to go through multiple trials and failures for each and every request. That sounds very inefficient, multiple trials and failures every single request, for normal, expected requests. It should have some way of knowing which engine is to handle a given request, and funnel that request to that engine. Only when an engine becomes inoperative (e.g., because a database has gone down) should it search for an alternative.
In particular, you probably don't want to implement this functionality by catching the E_SESSION exceptions at a higher level. Because its logic is local to the session subsystem. Keep things that work together close together; keep things that work separately apart.
That's what I would do if I were to write a Dancer::MetaSession class indeed.
However, if there is a compelling reason to catch exceptions at some other point in the Dancer framework, you could define additional catch-points.
Additional thinking : there has been some proposals made that more of the Dancer DSL should use exceptions as workflow, for instance, 'forward', 'redirect', etc, things that should immediately stop the route code execution should raise an exception. And be caught by the immediate route code caller, so that hooks and the rest of handler can continue to be properly executed. That btw would be a strong point in favor of having a catch-point around route execution. Implementation-wise, using the "power-of-2" system, it could be done by raising exceptions of type E_ROUTE & E_FORWARD, or E_ROUTE & E_REDIRECT for instance. And only E_ROUTE exceptions would be caught in the route catch-point.
DANCER EXCEPTIONS
Apps may want to throw other exception objects, not just internal Dancer exceptions. And it's important to keep that in mind. Because Dancer::Exception is of limited usefulness to many apps.
I agree with that, that's why render_response catches all exceptions, not only Dancer's. An a on_exception hook would also catch
Frankly, I'm still stymied by the choice to use a power-of-2 integer as an exception code in Dancer::Exception. It's no more lightweight than using an arbitrary integer—in fact, it's slightly heavier, as seen below—and it restricts the usefulness of the class.
The reason to use a power-of-2 integer is if you have a small, limited number of defined conditions, and you want to communicate two or more of them simultaneously in the same value. So for example:
use Fcntl ':mode'; $mode = (stat($filename))[2]; print "$filename is readable\n" if ($mode & S_IREAD); print "$filename is a directory\n" if ($mode & S_IFDIR);
In this example, the mode returned by stat is a bit-mask simultaneously communicating the state of all the different file permissions that stat knows about, as they pertain to the $filename being examined.
But exceptions are a horse of a different color... or maybe an animal of a different species. You only raise one exception at a time, and there can be an arbitrarily large set of possible exceptions that an app might throw. Looking at the latest released Dancer source, I see that Dancer::Exception will refuse even to register more than 16 exceptions— on a system with 64-bit integers. That means you're limited to a maximum of 16 different exception types, and of the 64 bits carried around with each exception object, 48 of them will always go to waste. That's heavier (or less useful) than using an arbitrary integer value.
While Dancer::Exception might still be useable for internal Dancer exceptions, it fails to provide the minimum feature-set needed for most application code. In general, a lightweight exception class should encompass a simple integer value and a string, and it should be subclassable for apps that need to extend it... Of course, what I've just described is Exception::Base, which is why I've been throwing Exception::Base objects in my code.
Here is the (maybe flawed) reasoning I had. Traditionally, exceptions are implemented as you mentioned, as classes, like Exception::Base. Object inheritance is used to derive exceptions from a base one. But in cases of exceptions, inheritance is (99% of the time) only used to specify that an exception Foo is also from type Bar. Inheritance is not used for overloading, adding, removing / changing methods, or attributes. So from my point of view, inheritance does the job of allowing mixing/subclassing exception, but it's like using a very big and complex tool to do soemthing relatively simple. So the "flag" oriented implementation was a attempt to allow subclassing / mixing exceptions without the burden of OO. For example, in OO, if you'd want to create an exception thrown by redirect, and that should be caught just around a route, you'd have to implement : Dancer::Exception (base class, at least caught at render_response ) Dancer::Exception::Route (exceptions that have to be caught around a route code execuction) Dancer::Exception::Route::Redirect That's a lot of files and modules to write. Using "power-of-2" stuff, you just' just have E_ROUTE, E_REDIRECT. That said, it seems that the current system is too complicated to grasp and use, and the 16 exception types may be a problem (I don't think it is, but it's a limitation nevertheless). So, I think I'll rewrite the whole, with Dancer::Exception as base class, and classic OO inheritance. No special catch syntax, we let users use eval {} or Try::Tiny. I'm not sure on the hooks yet. There is a need to provide users with to possibility to define hooks to catch exceptions from a route, and continue the workflow, and hooks to catch exceptions thrown from outside routes. But that makes 2 hook types, which I'm not very happy with. Any idea ?
Anyhow, you asked for feedback. Hope you weren't disappointed. :)
I'm not :) Thanks
Cheers, -TimK
On 8/8/11 11:05 AM, damien krotkine wrote:
Hi,
I'm in the midle of doing the second round coding for Dancer::Exception (it's in topic/exceptions2 on github). I have few design questions, and I'd like some feedback from the community.
The plan is that with Dancer::Exception : - 1/ an exception has a value ( combination of power of 2 integers), and a message (string) - 2/ all exceptions raised by the Dancer core will be Dancer::Exception (instead of normal die / croak) - 3/ a user will be able to raise exceptions in the routes - 4/ a user will be able to create new exception types - 5/ when raised, exceptions are transmitted to the templating system when rendering a Dancer::Error - 6/ exceptions should properly stringify and numerify for backward compatibility and ease of use. - 7/ the user should have the opportunity to catch any Dancer exception and perform an appropriate action
Currently, in the topic/exceptions2 branch, all items are implemented, except item 7. More precisely, it's possible to catch Dancer::Exception using standard eval {...}. But what if a user wants to catch an exception raised from a distant route, or from Dancer Core itself ?
Real life example 1 : in some routes, the developer does some 'raise' of exceptions (internal or custom). The developer wants to run specific codes when these exception are raised. For now, he can wrap his route code with eval {} and use is_dancer_exception, as stated in Dancer::Exception.
Real life example 2 : in some routes, the developper wants to catch core exceptions that could occur.
Real life example 3 : if something wrong happens in the Dancer::Session::Yaml modules, it raises a E_SESSION exception. When this exception occurs, it'll render a Dancer::Error, stating that thet exception were a E_SESSION. However what would be cool is that you could catch the exceptions, see that it's of type SESSION, and in this case try a different Dancer::Session system. In this case the exception is a core one.
example 3 is not related to routes, so it's a bit difficult to know where to put a try/catch scope.
I have some ideas, but I'm not really convinced by them : - idea 1 : add a new hook, that would be called 'after_exception' - idea 2 : when an exception is raised, it's stored as Shared Data, and thus can be detected with a before_error hook - idea 3 : add a try_catch syntax when creating route, like :
get '/foo' => sub { ... raise E_LOGIN, "bad credential" }, catch E_REQUEST | E_GENERIC => sub { ... } catch E_LOGIN => sub { ... }
Currently, Dancer::Exception is not too bad : it's simple, efficient and fast. I don't want it to become a bloated system. But we need to provide a bit more feature for it to be really useful.
Please share your thoughts :)
dams.
_______________________________________________ Dancer-users mailing listDancer-users@perldancer.orghttp://www.backup-manager.org/cgi-bin/listinfo/dancer-users
_______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users