[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


On September 16, 2003 11:20 am, Steve Hay wrote:
> 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.

As the originator of the above change, I feel obliged to comment.  As
long as you're perusing the old archives, you'll probably see why this
change was made: orders of magnitude faster.

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

Cleaner, but also faster.  Don't forget faster.  ;-)

> 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';

That seems reasonable.  However, I would suggest not going half-way
here:

  my $body = eval { $autoload_mode ? $self->$rmeth($rm) : $self->$rmeth() };
  if ( $@ )
  {
    $self->my_die($rm, $@);
    return; # in case my_die returns
  }

Then C::A could have a new method:

sub my_die
{
   my $self = shift;
   my ($rm, $@) = @_;
   die "Error executing run mode '$rm': $@";
}

This would probably give you exactly what you want: your subclass of
C::A could override my_die and you'd have exactly the exception object
that you threw elsewhere.  And you can handle it whatever way you want.

> 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() :-)

How does the above my_die handle your expectations?  It allows everyone
to use die normally, and gives you the exception handler you want. 
Meanwhile, it still gives everyone else a reasonable default handler. 
Or at least, the same default handler we have now ;-)

> 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()?

I only submitted it because it was already there - I was going for
speed, cleanliness and did I mention speed?

> 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?


---------------------------------------------------------------------
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


Mail converted by mhonarc 2.6.15
This archive provided courtesy of JSW4.NET, Internet Hosting Services for Small Business.