let an http-exception object that ->can("as_psgi") recieve the psgi $env...

lib/Plack/Middleware/HTTPExceptions.pm

@@ -24,30 +24,38 @@ sub call {
 
     return $res if ref $res eq 'ARRAY';
 
-    return sub {
-        my $respond = shift;
-
+    my $unroll_coderef_responses;
+    $unroll_coderef_responses = sub {
+        my ($responder, $response)  = @_;
         my $writer;
         try {
-            $res->(sub { return $writer = $respond->(@_) });
+            $response->(sub { return $writer = $responder->(@_) });
         } catch {
-            if ($writer) {
-                Carp::cluck $_;
-                $writer->close;
-            } else {
-                my $res = $self->transform_error($_, $env);
-                $respond->($res);
-            }
+          if($writer) {
+              # In the case where the exception happens part way through a write
+              # We just die since we can't at this point change the response
+              Carp::cluck $_;
+              $writer->close;
+          } else {
+            my $error_psgi_response = $self->transform_error($_, $env);
+            return $responder->($error_psgi_response) if ref $error_psgi_response eq 'ARRAY';
+            return $unroll_coderef_responses->($responder, $error_psgi_response);
+          }
         };
     };
+
+    return sub {
+        my $responder = shift;
+        return $unroll_coderef_responses->($responder, $res);
+    };
 }
 
 sub transform_error {
     my($self, $e, $env) = @_;
 
     my($code, $message);
     if (blessed $e && $e->can('as_psgi')) {
-        return $e->as_psgi;
+        return $e->as_psgi($env);
     }
     if (blessed $e && $e->can('code')) {
         $code = $e->code;
@@ -123,7 +131,8 @@ the status message of the error code, such as I<Service Unavailable> for
 C<503>.
 
 Finally, the exception object may implement C<as_psgi>, and the result
-of this will be returned directly as the PSGI response.
+of this will be returned directly as the PSGI response.  When calling
+the C<as_psgi> method, the PSGI C<$env> will be passed.
 
 If the code is in the 3xx range and the exception implements the 'location'
 method (HTTP::Exception::3xx does), the Location header will be set in the
@@ -143,6 +152,29 @@ enabled by default when C<PLACK_ENV> is set to C<development>, so that
 the L<StackTrace|Plack::Middleware::StackTrace> middleware can catch it
 instead.
 
+=head1 NOTES
+
+In the case where an exception rises during the middle of a streaming
+response (such as the following):
+
+    my $psgi_app = sub {
+    my $env = shift;
+        return sub {
+        my $responder = shift;
+        my $writer = $responder->([200, ['content-type'=>'text/html']]);
+        $writer->write('ok');
+
+        # Stuff...
+
+        die MyApp::Exception::ServerError->new($env);
+    };
+
+We can't meaningfully set the response from this exception, since at this
+point HTTP Headers and a partial body have been returned.  If you need to
+verify such a case you'll need to rely on alternative means, such as setting
+expected content-length or providing a checksum, that the client can use to
+valid the returned content.
+
 =head1 AUTHOR
 
 Tatsuhiko Miyagawa

t/Plack-Middleware/httpexceptions-as_psgi.t

@@ -0,0 +1,224 @@
+use strict;
+use warnings;
+use Plack::Test;
+use HTTP::Request::Common;
+use Plack::Middleware::HTTPExceptions;
+use Test::More;
+
+{
+  package MyApp::Exception::Tuple;
+
+  sub new {
+    my ($class, @args) = @_;
+    return bless +{res => \@args}, $class;
+  }
+
+  sub as_psgi {
+    my ($self, $env) = @_;
+    Test::More::ok $env && $env->{'psgi.version'}, 
+      'has $env and its a psgi $env';
+    return $self->{res};
+  }
+
+  package MyApp::Exception::CodeRef;
+
+  sub new {
+    my ($class, @args) = @_;
+    return bless +{res => \@args}, $class;
+  }
+
+  sub as_psgi {
+    my ($self, $env) = @_;
+    Test::More::ok $env && $env->{'psgi.version'}, 
+      'has $env and its a psgi $env';
+
+    my ($code, $headers, $body) = @{$self->{res}};
+
+    return sub {
+      my $responder = shift;
+      $responder->([$code, $headers, $body]);
+    };
+  }
+
+  package MyApp::Exception::CodeRefWithWrite;
+
+  sub new {
+    my ($class, @args) = @_;
+    return bless +{res => \@args}, $class;
+  }
+
+  sub as_psgi {
+    my ($self, $env) = @_;
+    Test::More::ok $env && $env->{'psgi.version'}, 
+      'has $env and its a psgi $env';
+
+    my ($code, $headers, $body) = @{$self->{res}};
+
+    return sub {
+      my $responder = shift;
+      my $writer = $responder->([$code, $headers]);
+      $writer->write($_) for @$body;
+      $writer->close;
+    };
+  }
+
+}
+
+ok my $psgi_app = sub {
+  my $env = shift;
+
+  die MyApp::Exception::Tuple->new(
+    404, ['content-type'=>'text/plain'], ['Not Found'])
+      if $env->{PATH_INFO} eq '/tuple';
+
+  die MyApp::Exception::CodeRef->new(
+    303, ['content-type'=>'text/plain'], ['See Other'])
+      if $env->{PATH_INFO} eq '/coderef';
+
+  die MyApp::Exception::CodeRefWithWrite->new(
+    400, ['content-type'=>'text/plain'], ['Bad Request'])
+      if $env->{PATH_INFO} eq '/coderefwithwrite';
+
+  return [200, ['Content-Type'=>'html/plain'], ['ok']]
+      if $env->{PATH_INFO} eq '/ok';
+};
+
+ok $psgi_app = Plack::Middleware::HTTPExceptions->wrap($psgi_app);
+
+test_psgi $psgi_app, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/tuple");
+    is $res->code, 404;
+    is $res->content, 'Not Found', 'NOT FOUND';
+};
+
+test_psgi $psgi_app, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/ok");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+test_psgi $psgi_app, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderef");
+    is $res->code, 303;
+    is $res->content, 'See Other', 'SEE OTHER';
+};
+
+test_psgi $psgi_app, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderefwithwrite");
+    is $res->code, 400;
+    is $res->content, 'Bad Request', 'BAD REQUEST';
+};
+
+ok my $psgi_app_delayed = sub {
+  my $env = shift;
+  return sub {
+    my $responder = shift;
+
+    die MyApp::Exception::Tuple->new(
+      404, ['content-type'=>'text/plain'], ['Not Found'])
+        if $env->{PATH_INFO} eq '/tuple';
+
+    die MyApp::Exception::CodeRef->new(
+      303, ['content-type'=>'text/plain'], ['See Other'])
+        if $env->{PATH_INFO} eq '/coderef';
+
+    die MyApp::Exception::CodeRefWithWrite->new(
+      400, ['content-type'=>'text/plain'], ['Bad Request'])
+        if $env->{PATH_INFO} eq '/coderefwithwrite';
+
+    return $responder->
+      ([200, ['Content-Type'=>'html/plain'], ['ok']])
+        if $env->{PATH_INFO} eq '/ok';
+  };
+};
+
+ok $psgi_app_delayed = Plack::Middleware::HTTPExceptions->wrap($psgi_app_delayed);
+
+test_psgi $psgi_app_delayed, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/tuple");
+    is $res->code, 404;
+    is $res->content, 'Not Found', 'NOT FOUND';
+};
+
+test_psgi $psgi_app_delayed, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/ok");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+test_psgi $psgi_app_delayed, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderef");
+    is $res->code, 303;
+    is $res->content, 'See Other', 'SEE OTHER';
+};
+
+test_psgi $psgi_app_delayed, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderefwithwrite");
+    is $res->code, 400, 'correct 400 code';
+    is $res->content, 'Bad Request', 'BAD REQUEST';
+};
+
+ok my $psgi_app_delayed_with_write = sub {
+  my $env = shift;
+  return sub {
+    my $responder = shift;
+    my $writer = $responder->([200, ['content-type'=>'text/html']]);
+    $writer->write('ok');
+
+    die MyApp::Exception::Tuple->new(
+      404, ['content-type'=>'text/plain'], ['Not Found'])
+        if $env->{PATH_INFO} eq '/tuple';
+
+    die MyApp::Exception::CodeRef->new(
+      303, ['content-type'=>'text/plain'], ['See Other'])
+        if $env->{PATH_INFO} eq '/coderef';
+
+    die MyApp::Exception::CodeRefWithWrite->new(
+      400, ['content-type'=>'text/plain'], ['Bad Request'])
+        if $env->{PATH_INFO} eq '/coderefwithwrite';
+
+    return $writer->close if $env->{PATH_INFO} eq '/ok';  };
+};
+
+ok $psgi_app_delayed_with_write = Plack::Middleware::HTTPExceptions->wrap($psgi_app_delayed_with_write);
+
+test_psgi $psgi_app_delayed_with_write, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/tuple");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+test_psgi $psgi_app_delayed_with_write, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/ok");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+test_psgi $psgi_app_delayed_with_write, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderef");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+test_psgi $psgi_app_delayed_with_write, sub {
+    my $cb = shift;
+    my $res = $cb->(GET "/coderefwithwrite");
+    is $res->code, 200;
+    is $res->content, 'ok', 'OK';
+};
+
+
+# need to list the expected test number because of the test case in the
+# exception class.
+done_testing(36);