Re: Re: [Mason-checkins] CVS: mason/dist/t 13-errors.t,1.28,1.29

Hi John -

Your test definitely uncovered a bug in HEAD. I think it needs to be handled
with a different fix though.

In Mason 1.2x, an error occurring in the Request constructor (usually a
failure to load the top level component) would be placed in a prepare_error
slot that would get checked in exec(). That was taken out in HEAD, but we
never put anything in to prevent faulty requests from getting exec'd(). See

    http://masonhq.com/?WhyRequestPrepareErrors

In particular,

    "to make it work for html error mode, any code that calls
$interp->make_request() separately will need some way to know that an error
occurred, so that it doesn't go on to call $request->exec()."

We never did this, even for our own $interp->exec().

I'm still not sure what the right API is when a request fails during the
constructor - should we return undef or a failed request? For now, I've
stuck to the latter by adding an initialized flag - if this is unset
(meaning an error occurred during the constructor), $request->exec() just
returns. It is closest to the existing behavior.

diff -a -u -r1.339 Request.pm
--- Request.pm  13 Dec 2004 20:22:28 -0000      1.339
+++ Request.pm  21 Dec 2004 06:34:32 -0000
@@ -156,6 +156,7 @@
     ( read_only => [ qw(
                        count
                        dhandler_arg
+                       initialized
                        interp
                        parent_request
                        plugin_instances
@@ -179,6 +180,7 @@
     %$self = (%$self,
              dhandler_arg   => undef,
              execd          => 0,
+             initialized    => 0,
              stack          => [],
              top_stack      => undef,
              wrapper_chain  => undef,
@@ -313,6 +315,8 @@
     my $err = $@;
     if ($err and !$self->_aborted_or_declined($err)) {
        $self->_handle_error($err);
+    } else {
+       $self->{initialized} = 1;
     }
 }

@@ -359,6 +363,10 @@
     my ($self) = @_;
     my $interp = $self->interp;

+    # If the request failed to initialize, the error has already been
handled
+    # at the bottom of _initialize(); just return.
+    return unless $self->initialized();
+



----- Original Message -----
From: "John Williams" <[email protected]>
To: <[email protected]>
Sent: Friday, December 17, 2004 1:10 PM
Subject: [Mason-devel] Re: [Mason-checkins] CVS: mason/dist/t
13-errors.t,1.28,1.29


> The test just commited (below) is a problem which has been showing up in
> the live apache tests for me, but it is now isolated in a non-live test.
>
> A compilation error with error_mode=>'output' does not stop the processing
> soon enough, and we end up with an error like "cannot call method on
> undefined value" being output instead of the real error.
>
> Here is one way to fix it:
>
> Index: lib/HTML/Mason/Request.pm
> ===================================================================
> RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Request.pm,v
> retrieving revision 1.333
> diff -u -r1.333 Request.pm
> --- lib/HTML/Mason/Request.pm 18 Oct 2004 06:09:23 -0000 1.333
> +++ lib/HTML/Mason/Request.pm 30 Oct 2004 05:37:02 -0000
> @@ -506,6 +506,7 @@
>   rethrow_exception $err;
>      } else {
>   UNIVERSAL::isa( $self->out_method, 'CODE' ) ?
$self->out_method->("$err") : ( ${ $self->out_method } = "$err" );
> + $self->abort if UNIVERSAL::isa( $err,
'HTML::Mason::Exception::Compilation' );
>      }
>  }
>
>
> The downside of that is that Mason::Tests doesn't currently handle an
> "abort with success" result.  To get around that I have added an
> "abort_ok => 1" attribute to the test, and the patch below.
>
>
> Index: lib/HTML/Mason/Tests.pm
> ===================================================================
> RCS file: /cvsroot/mason/mason/dist/lib/HTML/Mason/Tests.pm,v
> retrieving revision 1.60
> diff -u -r1.60 Tests.pm
> --- lib/HTML/Mason/Tests.pm 12 Oct 2004 18:20:38 -0000 1.60
> +++ lib/HTML/Mason/Tests.pm 30 Oct 2004 05:37:14 -0000
> @@ -535,6 +535,10 @@
>          if exists $self->{current_test}{todo};
>      $Test->todo if exists $self->{current_test}{todo};
>
> +    if ($test->{abort_ok} &&
UNIVERSAL::isa($error,'HTML::Mason::Exception::Abort')) {
> + undef $error;
> +    }
> +
>      if ($error)
>      {
>   if ( $test->{expect_error} )
>
>
> I just wonder if the other developers have any thoughts on whether this is
> the best way to fix the problem, or the best way to handle the abort in
> Mason::Test?
>
> ~ John Williams
>
>
>
> On Fri, 17 Dec 2004, John Williams wrote:
>
> > Update of /cvsroot/mason/mason/dist/t
> > In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv1611/t
> >
> > Modified Files:
> > 13-errors.t
> > Log Message:
> > TODO test:
> > compilation errors are not thrown properly with error_mode=>'output'
> >
> >
> > Index: 13-errors.t
> > ===================================================================
> > RCS file: /cvsroot/mason/mason/dist/t/13-errors.t,v
> > retrieving revision 1.28
> > retrieving revision 1.29
> > diff -u -r1.28 -r1.29
> > --- 13-errors.t 12 Oct 2004 18:20:39 -0000 1.28
> > +++ 13-errors.t 17 Dec 2004 20:40:28 -0000 1.29
> > @@ -335,6 +335,23 @@
> >
> >  #------------------------------------------------------------
> >
> > +    $group->add_test( name => 'error_during_compilation',
> > +       description => "Make sure compiler errors work in output mode",
> > +                      interp_params => {
> > + error_format => 'text',
> > + error_mode => 'output',
> > +                                       },
> > +       component => <<'EOF',
> > +% my $x =
> > +EOF
> > + # match "Error during compilation" followed by
> > + # exactly one occurance of "Stack:"
> > + # (Mason should stop after the first error)
> > +       expect => qr/Error during
compilation((?!Stack:).)*Stack:((?!Stack:).)*$/s,
> > +     );
> > +
> > +#------------------------------------------------------------
> > +
> >      return $group;
> >  }
> >
>
>
>
>
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://productguide.itmanagersjournal.com/
> _______________________________________________
> Mason-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/mason-devel
>



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/