From: John Napiorkowski Date: Wed, 16 Jan 2019 16:03:00 +0000 (-0600) Subject: solution for warnings when using from_psgi_response with streaing X-Git-Tag: v5.90124~9 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=5757858f133f7f1e92dd996dad0f6e28b0b919ed;hp=53779e13d3ae3e52c7c739aa17b97092cde4a4d7 solution for warnings when using from_psgi_response with streaing --- diff --git a/lib/Catalyst/Response.pm b/lib/Catalyst/Response.pm index b0f04a3..4f9c391 100644 --- a/lib/Catalyst/Response.pm +++ b/lib/Catalyst/Response.pm @@ -180,6 +180,14 @@ sub from_psgi_response { ref $body eq 'ARRAY' ? $self->body(join('', @$body)) : $self->body($body); } } elsif(ref $psgi_res eq 'CODE') { + + # Its not clear to me this is correct. Right now if the PSGI application wants + # to stream, we stream immediately and then completely bypass the rest of the + # Catalyst finalization process (unlike if the PSGI app sets an arrayref). Part of + # me thinks we should override the current _response_cb and then let finalize_body + # call that. I'm not sure the downside of bypassing those bits. I'm going to leave + # this be for now and document the behavior. + $psgi_res->(sub { my $response = shift; my ($status, $headers, $maybe_body) = @$response; @@ -196,12 +204,14 @@ sub from_psgi_response { die "You can't set a Catalyst response from that, expect a valid PSGI response"; } - return unless $self->_context->has_encoding; - # Encoding compatibilty. If the response set a charset, well... we need # to assume its properly encoded and NOT encode for this response. Otherwise # We risk double encoding. - if($self->content_type_charset) { + + # We check first to make sure headers have not been finalized. Headers might be finalized + # in the case where a PSGI response is streaming and the PSGI application already wrote + # to the output stream and close the filehandle. + if(!$self->finalized_headers && $self->content_type_charset) { # We have to do this since for backcompat reasons having a charset doesn't always # mean that the body is already encoded :( $self->_context->clear_encoding; @@ -589,6 +599,17 @@ Example: $c->res->from_psgi_response($app->($c->req->env)); } + sub streaming_body :Local { + my ($self, $c) = @_; + my $psgi_app = sub { + my $respond = shift; + my $writer = $respond->([200,["Content-Type" => "text/plain"]]); + $writer->write("body"); + $writer->close; + }; + $c->res->from_psgi_response($psgi_app); + } + Please note this does not attempt to map or nest your PSGI application under the Controller and Action namespace or path. You may wish to review 'PSGI Helpers' under L for help in properly nesting applications. @@ -598,6 +619,13 @@ set associated with the content type (such as "text/html; charset=UTF-8") we set $c->clear_encoding to remove any additional content type encoding processing later in the application (this is done to avoid double encoding issues). +B If your external PSGI application is streaming, we assume you completely +handle the entire jobs (including closing the stream). This will also bypass +the output finalization methods on Catalyst (such as 'finalize_body' which gets +called but then skipped when it finds that output is already finished.) Its possible +this might cause issue with some plugins that want to do 'things' during those +finalization methods. Just understand what is happening. + =head2 encodable_content_type This is a regular expression used to determine of the current content type diff --git a/t/psgi_utils.t b/t/psgi_utils.t index 923defb..ba744a0 100644 --- a/t/psgi_utils.t +++ b/t/psgi_utils.t @@ -73,7 +73,7 @@ my $psgi_app = sub { $writer->write("body"); $writer->close; }; - $c->clear_encoding; + #$c->clear_encoding; $c->res->from_psgi_response($psgi_app); }