[cgiapp] Re: CGI::App 3.2 proposed release available

[Mark Stosberg]

On 2003-11-26, Terrence Brannon [EMAIL PROTECTED] wrote:

 Module::Build is stil rather new in my opinion. I cross-posted to dfv 
 and the mb lists about not being able to install Data::FormValidator in 
 my local tree as opposed to /usr (just try it at perlmonk.org sometime!) 
 and Ken hasn't been around to fix it. I am using CGI::Application at 
 perlmonk.org for a gaming site and don't want to have to hand-install it 
 because we switched over to M::B.

Using the 'traditional' compability mode, M::B would not even be
required to install the module-- MakeMaker could still be used. So this
would fix your issue above. The way DFV currently uses M::B does require
it for installation. I'm open to the idea of allowing MakeMaker
installation again. We could discuss that more on the DFV list.

However, this is the least of my concerns with getting 3.2 released. :) 


Mark

-- 
 . . . . . . . . . . . . . . . . . . . . . . . . . . . 
   Mark StosbergPrincipal Developer  
   [EMAIL PROTECTED] Summersault, LLC 
   765-939-9301 ext 202 database driven websites
 . . . . . http://www.summersault.com/ . . . . . . . .


-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
  http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [cgiapp] Re: CGI::App 3.2 proposed release available

[Steve Hay]

Mark Stosberg wrote:

The only other thing that I'd still really like is for something to be 
done about the problems with exception handling that the catch/rethrow 
(eval/die) in run() causes.  This has been discussed at length before in 
several threads (see the archives), but nothing has ever happened.  Is 
it ever going to?



This is not something I am fresh on or have an opinion on at the moment.
I know that if there is a complete patch, something is more likely to
happen, though. :)

I'll accept that invitation!  First, a quick refresher of what the 
problem is:

The code in C::A that causes problems for my exception handling is these 
two lines in the run() method:

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

In the case of exceptions thrown by the run mode method in question (as 
opposed to C::A itself having an error, e.g. the run mode method can't 
be dispatched), catching (eval) and then rethrowing (die) the exception 
in this way causes two problems for me:

Problem 1. It assumes that the exception in $@ is a string.  I'm trying 
to setup a system based around Exception::Class that uses Perl's 
exception objects.  The above code stringifies the exception object in 
[EMAIL PROTECTED]  (Exception::Class' objects overload stringification, so that at 
least a decent error message is still produced, but all of the other 
information on the object is lost.)

Problem 2. It adds an extra frame into the stack trace, which confuses 
modules like Exception::Class as to where the exception comes from.  
(That is, E::C now thinks that the error was in C::A::run() instead of 
somewhere inside the run mode method where it really was.)

There are various ways to improve things here.  Here's a couple that 
spring to mind (with patches against the pre-release 3.2):

Solution 1.

The whole catch/rethrow thing above is almost pointless.  It clearly 
makes no difference if there are no errors, and it doesn't stop the code 
from die()'ing if an error does occur either.

I believe the only point of the catch/rethrow is to give a nicer error 
message (Error executing run mode ...) than whatever Perl's default 
error message would be in the case where the run mode method can't be 
dispatched.

We could simply remove the catch/rethrow altogether, and leave it up to 
Perl to say that the method can't be dispatched if that's the case:

=
--- Application.pm.orig2003-11-25 14:01:29.053896600 +
+++ Application.pm2003-11-25 15:46:44.971140300 +
@@ -150,8 +150,7 @@
 }
 
 # Process run mode!
-my $body = eval { $autoload_mode ? $self-$rmeth($rm) : 
$self-$rmeth() };
-die Error executing run mode '$rm': $@ if $@;
+my $body = $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
 
 # Make sure that $body is not undefined (supress 'uninitialized 
value' warnings)
 $body =  unless defined $body;
=

Solution 2.

Slightly nicer than that:  Pick off those cases where the run mode 
method dispatch might fail and use the catch/rethrow to give a nice 
error in those cases.  Don't use it otherwise.

We can use the UNIVERSAL::can() method to see whether the run mode 
method dispatch should work.  If $self-can($rmeth) is true then the 
method definitely can be dispatched so any exception raised will be from 
within that method and we shouldn't mess with it.  If $self-can($rmeth) 
is false then we try the method dispatch anyway, just in case the run 
mode method is AUTOLOAD'ed (which UNIVERSAL::can() can't pick up on), 
but within the protection of an eval { ... } just in case the dispatch 
fails, in which case we throw a (nicer) error message ourselves as we 
currently do:

=
--- Application.pm.orig2003-11-25 14:01:29.053896600 +
+++ Application.pm2003-11-25 15:51:36.922114000 +
@@ -150,8 +150,14 @@
 }
 
 # Process run mode!
-my $body = eval { $autoload_mode ? $self-$rmeth($rm) : 
$self-$rmeth() };
-die Error executing run mode '$rm': $@ if $@;
+my $body;
+if ($self-can($rmeth)) {
+$body = $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
+}
+else {
+$body = eval { $autoload_mode ? $self-$rmeth($rm) : 
$self-$rmeth() };
+die Error executing run mode '$rm': $@ if $@;
+}
 
 # Make sure that $body is not undefined (supress 'uninitialized 
value' warnings)
 $body =  unless defined $body;
=

Solution 3.

Retain the exception catching in all circumstances, but provide (yet 
another) overridable method, cgiapp_exception(), to deal with the 
exception that we've just caught.  The default implementation of the new 
method would, of course, be the current behaviour.

In my case, I would override cgiapp_exception() to simply die with the 
exception (object) that is passed to it, without tampering with it in 
any way.

=
--- Application.pm.orig2003-11-25 14:01:29.053896600 +
+++ Application.pm2003-11-25 

[cgiapp] Re: CGI::App 3.2 proposed release available

[Mark Stosberg]

On 2003-11-25, Steve Hay [EMAIL PROTECTED] wrote:

 I'll accept that invitation!  First, a quick refresher of what the 
 problem is:

 The code in C::A that causes problems for my exception handling is these 
 two lines in the run() method:

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

 In the case of exceptions thrown by the run mode method in question (as 
 opposed to C::A itself having an error, e.g. the run mode method can't 
 be dispatched), catching (eval) and then rethrowing (die) the exception 
 in this way causes two problems for me:

 Problem 1. It assumes that the exception in $@ is a string.  I'm trying 
 to setup a system based around Exception::Class that uses Perl's 
 exception objects.  The above code stringifies the exception object in 
 [EMAIL PROTECTED]  (Exception::Class' objects overload stringification, so that at 
 least a decent error message is still produced, but all of the other 
 information on the object is lost.)

 Problem 2. It adds an extra frame into the stack trace, which confuses 
 modules like Exception::Class as to where the exception comes from.  
 (That is, E::C now thinks that the error was in C::A::run() instead of 
 somewhere inside the run mode method where it really was.)

Steve,

I agree this seems worthwhile to address. I like solutions 2 and 3 you
propose, with a preference for 2 now. We could always start there and
add the complexity of 3. later. It seems there are already some
built-in ways to override what die is doing if people want to use
them.

I'll wait for more feedback from others before applying the patch.

Mark

-- 
 . . . . . . . . . . . . . . . . . . . . . . . . . . . 
   Mark StosbergPrincipal Developer  
   [EMAIL PROTECTED] Summersault, LLC 
   765-939-9301 ext 202 database driven websites
 . . . . . http://www.summersault.com/ . . . . . . . .


-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
  http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

[cgiapp] Fwd: Re: [cgiapp] Re: CGI::App 3.2 proposed release available

[Darin McBride]

On November 25, 2003 09:19 am, Steve Hay wrote:
 Mark Stosberg wrote:
 The only other thing that I'd still really like is for something to be
 done about the problems with exception handling that the catch/rethrow
 (eval/die) in run() causes.  This has been discussed at length before in
 several threads (see the archives), but nothing has ever happened.  Is
 it ever going to?
 
 This is not something I am fresh on or have an opinion on at the moment.
 I know that if there is a complete patch, something is more likely to
 happen, though. :)

 I'll accept that invitation!  First, a quick refresher of what the
 problem is:

 The code in C::A that causes problems for my exception handling is these
 two lines in the run() method:

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

[snip]

 =
 --- Application.pm.orig2003-11-25 14:01:29.053896600 +
 +++ Application.pm2003-11-25 15:51:36.922114000 +
 @@ -150,8 +150,14 @@
  }

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

  # Make sure that $body is not undefined (supress 'uninitialized
 value' warnings)
  $body =  unless defined $body;
 =

I'll go with option #4.  Best of all worlds.  ;-

my $body = $self-execute_run_mode($rm, $rmeth, $autoload_mode);

[...]

sub execute_run_mode
{
   my ($self, $rm, $rmeth, $autoload_mode) = @_;
   my $body;
   if ($self-can($rmeth)) {
 $body = $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
   }
   else {
 $body = eval { $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth(); }
 die Error executing run mode '$rm': $@ if $@;
   }
   $body;
}


In your application, simply override execute_run_mode:

sub execute_run_mode
{
   my ($self, $rm, $rmeth, $autoload_mode) = @_;
   $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
}

Of course, we could take this a small point further, and make the
second execute_run_mode from above into C::A::execute_run_mode_no_catch
and then have an option we can set during setup whether we want C::A to
catch exceptions or not (default to true):

my $exec_func = $self-catch_exceptions() ? \execute_run_mode :
\execute_run_mode_no_catch;
my $body = $self-$exec_func($rm, $rmeth, $autoload_mode);

I'm leaving the creation of sub catch_exceptions as an excersise for
the reader. :-)

I think this allows the most flexibility for everyone.  And where the
above two solutions don't fit someone's desire, they can override
execute_run_mode still.


-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
  http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

[cgiapp] Re: CGI::App 3.2 proposed release available

[Mark Stosberg]

On 2003-11-25, Steve Hay [EMAIL PROTECTED] wrote:
 Mark Stosberg wrote:

Hello,

Here's a link to a updated CGI::App distribution that I'm proposing
become CGI::App 3.2: 

http://mark.stosberg.com/perl/CGI-Application-3.2_mls2.tar.gz

 Here's a tiny but important documentation patch that corrects a mistake 
 in the cgiapp_postrun() description.  This has cropped up a couple of 
 times on the mailing list:

=
 --- Application.pm.orig2003-11-25 14:01:29.053896600 +
 +++ Application.pm2003-11-25 16:39:04.450832900 +
 @@ -1121,7 +1121,7 @@
  $new_output .= /table;
  
  # Replace old output with new output
 -$output_ref = \$new_output;
 +$$output_ref = $new_output;
}

Applied. Thanks.

Mark

-- 
 . . . . . . . . . . . . . . . . . . . . . . . . . . . 
   Mark StosbergPrincipal Developer  
   [EMAIL PROTECTED] Summersault, LLC 
   765-939-9301 ext 202 database driven websites
 . . . . . http://www.summersault.com/ . . . . . . . .


-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
  http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

[cgiapp] Re: Fwd: Re: [cgiapp] Re: CGI::App 3.2 proposed release available

[Mark Stosberg]

On 2003-11-25, Darin McBride [EMAIL PROTECTED] wrote:

 I'll go with option #4.  Best of all worlds.  ;-

 my $body = $self-execute_run_mode($rm, $rmeth, $autoload_mode);

 [...]

 sub execute_run_mode
 {
my ($self, $rm, $rmeth, $autoload_mode) = @_;
my $body;
if ($self-can($rmeth)) {
  $body = $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
}
else {
  $body = eval { $autoload_mode ? $self-$rmeth($rm) : $self-$rmeth(); }
  die Error executing run mode '$rm': $@ if $@;
}
$body;
 }


 In your application, simply override execute_run_mode:

 sub execute_run_mode
 {
my ($self, $rm, $rmeth, $autoload_mode) = @_;
$autoload_mode ? $self-$rmeth($rm) : $self-$rmeth();
 }

I don't like this as well as #2, because like #3, it means adding
another public interface method to C::A. I'm not opposed to that, just
persistent-- I think part of the elegance of CGI::App is that it's fairly
simple to learn and use.

I believe with #2, people can still use the standard techniques for
overriding die. I don't think we need to re-invent that concept in
CGI::App. 

At least, I'd rather start out with a simpler solution now and see how
it serves us for a while. It's a lot easier to /add/ visible
functionality to a module than it is to remove a bad decision later that
some people may depending on the API for.

Mark

-- 
 . . . . . . . . . . . . . . . . . . . . . . . . . . . 
   Mark StosbergPrincipal Developer  
   [EMAIL PROTECTED] Summersault, LLC 
   765-939-9301 ext 202 database driven websites
 . . . . . http://www.summersault.com/ . . . . . . . .


-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
  http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Re: [cgiapp] Re: CGI::App 3.2 proposed release available

[Thilo Planz]

Solution 1.
We could simply remove the catch/rethrow altogether, and leave it up to
Perl to say that the method can't be dispatched if that's the case:
Solution 2.
Slightly nicer than that:  Pick off those cases where the run mode
method dispatch might fail and use the catch/rethrow to give a nice
error in those cases.  Don't use it otherwise.
Solution 3.

Retain the exception catching in all circumstances, but provide (yet
another) overridable method, cgiapp_exception(), to deal with the
exception that we've just caught.  The default implementation of the 
new
method would, of course, be the current behaviour.


Which solution do you prefer?  What other solutions are there?
I like solution 1 over solution 2.

Solution 2 has the same problems as the current implementation in that 
it treats exceptions returned from auto-loaded methods as strings.
I never thought of auto-loading in the first place, but if we are to 
support it, we should do it properly (including allowing it to die with 
exceptions).

I see your problem with solution 3, in that cgiapp_exception adds an 
additional stack frame if it dies.
However, cgiapp_exception does not necessarily have to die at all.
It could just produce a nice error page, and then return to let run() 
finish its work (headers, teardown and all)
What I am currently doing is an extended version of solution 3.

Solution 4

cgiapp_exception is allowed to return a result, which is then sent to 
the client (instead of the normal runmode output).

# Process run mode!
 my $body = eval { $autoload_mode ? $self-$rmeth($rm) :
$self-$rmeth() };
-die Error executing run mode '$rm': $@ if $@;
+if ($@) {
+$body = $self-cgiapp_exception($rm, $@); # in case 
cgiapp_exception does not die
+}


We could solve the stack frame thing by not having cgiapp_exception 
defined in the base class,
and only calling it if we can()  (because it has been defined in 
subclasses).
If it is not defined, we just die as in solution 1

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


Thilo





-
Web Archive:  http://www.mail-archive.com/[EMAIL PROTECTED]/
 http://marc.theaimsgroup.com/?l=cgiappr=1w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]