[Dancer-users] Dancer::Exception

[damien krotkine]

On 11 August 2011 23:24, Tim King <timk at 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
>
> http://www.JTimothyKing.com/
>
>
> 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 at perldancer.orghttp://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>
>
> _______________________________________________
> Dancer-users mailing list
> Dancer-users at perldancer.org
> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.backup-manager.org/pipermail/dancer-users/attachments/20110812/511510b2/attachment-0001.htm>