diff --git a/lib/Plack/Middleware/HTTPExceptions.pm b/lib/Plack/Middleware/HTTPExceptions.pm index 23a7e935f..b2c62ac91 100644 --- a/lib/Plack/Middleware/HTTPExceptions.pm +++ b/lib/Plack/Middleware/HTTPExceptions.pm @@ -24,22 +24,30 @@ 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 { @@ -47,7 +55,7 @@ sub transform_error { 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 for C<503>. Finally, the exception object may implement C, 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 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 is set to C, so that the L 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 diff --git a/t/Plack-Middleware/httpexceptions-as_psgi.t b/t/Plack-Middleware/httpexceptions-as_psgi.t new file mode 100644 index 000000000..b94475690 --- /dev/null +++ b/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);