Re: [cgiapp] Exception handling in C::A modules

[Steve Hay <suppressed>]

Steve Hay wrote:

> [snip]
>
> 2.  It executes the run-mode within an eval {} block of its own, 
> catches any exceptions raised and then simply throws them again after 
> having added a bit to them ("Error executing run mode...").
>
> This makes the gross presumption that the run-mode didn't raise an 
> exception *object*.  If it did, then it just gets stringified into 
> C::A's string error before getting thrown again.  Thus, every 
> exception that I ever raise myself gets caught, stringified and 
> rethrown.  My catcher() method catches it, of course, but I now find 
> that *all* my exception objects have become "uncaught [string] 
> exceptions" that my catcher() has caught.  There is no point in any of 
> the Exception::Classes that I've been carefully building!
>
> Why bother catching the exception at all, just to throw it again?  
> There's no point unless you're going to *do* something with it in some 
> circumstances.  Simply prepending a string to it just screws it up 
> when it's an object.  You could catch the exception and prepend a 
> string only if it is already a string (not an object), but even then 
> you're screwing up Exception::Class' record of where the exception was 
> thrown from. 

I've been reading the mail archive for other people's comments on 
exception handling problems with C::A and found this thread:

http://www.mail-archive.com/suppressed/msg00196.html

Jesse, I understand that you have some objections to doing anything very 
specific with exceptions in C::A, but the above thread brought to my 
attention the fact that earlier versions of C::A (e.g. 2.4) didn't used 
to do what the current version (3.1) does.  Version 2.4 has this:

=====
   # Process run mode!
   my $body;
   if (ref($rmeth) eq 'CODE') {
       if ($autoload_mode) {
           $body = $rmeth->($self, $rm);
       } else {
           $body = $rmeth->($self);
       }
   } else {
       my $meth_call = '$self->' . $rmeth;
       $meth_call .= '($rm)' if ($autoload_mode);
       $body = eval($meth_call);
       die ("Error executing run mode '$rm'.  Eval of code '$meth_call' 
resulted in error: " . $@) if ($@);
   }
=====

whereas version 3.1 now has this:

=====
   # Process run mode!
       my $body = eval { $autoload_mode ? $self->$rmeth($rm) : 
$self->$rmeth() };
       die "Error executing run mode '$rm': $@" if $@;
=====

So the earlier version just used to check for exceptions when running a 
run-mode specified by a string, presumably because it would be less sure 
that the string could even be resolved to a method call.

The new version is clearly much cleaner, but has the unfortunate effect 
of extending the exception handling to cover all cases.

Couldn't you at least revert it to its old behaviour, seeing as you're 
not doing anything special with it?, e.g.

       my $body = eval { $autoload_mode ? $self->$rmeth($rm) : 
$self->$rmeth() };
       die "Error executing run mode '$rm': $@" if $@ and ref($meth) eq 
'CODE';

Your previous reply on the subject, 
(http://www.mail-archive.com/suppressed/msg00198.html), 
speaks of putting a my_die() method into the application superclass.  
That might be tolerable for when *I* want to raise exceptions, but, of 
course, much (most?) of the code running within the run-modes it 
actually third-party code (CPAN modules), and I want to catch all the 
exceptions raised from within them as well.  They won't be using 
my_die() :-)

You also say, "CGI-App was built to allow you to implement project-wide 
or company-wide policies through sub-classing, and exception handling 
might be a good candidate for this type of structure.  [I agree 
completely!]  I am reluctant to add special exception handling to 
CGI::Application...", so why add anything at all, then?  In fact, what 
you've added makes it impossible for me to implement my project-wide 
exception handling policy because it stringifies all the exception 
objects that I was throwing!

Can you explain or justify why you have the offending eval { ... } / 
die() in C::'A's run()?

There is no point in it at all as far as I can tell.  If no exception is 
raised then it doesn't do anything.  If an exception is raised then you 
catch it in your eval { ... }, but then just throw it again!

WHY?!!!

And worse than that, you've even gratuitously munged it about a bit.  
The only thing that your eval { ... } / die() does is to make the 
admirable aim of implementing project-wide exception handling 
impossible.  Surely it's not your intention for C::A to do that?

Please can we at least have an explanation?

Regards,
- Steve


---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
             http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed