Bug #94392 for Catalyst-Plugin-SubRequest: Tests break on 5.90006x

Wed Apr 02 22:12:50 2014 andreas.marienborg [...] gmail.com - Ticket created

Subject:

Tests break on 5.90006x

the t/02subreq.t test breaks on 5.90006x. I am not sure exactly why, but the change in catalyst that breaks it deals with changes in body handling (https://github.com/perl-catalyst/catalyst-runtime/commit/46fff667a1229a59422010500311e48a6f9da824) https://metacpan.org/source/EDENC/Catalyst-Plugin-SubRequest-0.20/lib/Catalyst/Plugin/SubRequest.pm#L112 If $writer->body is changed to $i_ctx->res->body, all the tests pass again. I don't understand all the mechanics, so this might not be the right fix. I have tested all Catalyt 5.9x versions, and it works back to 5.90008. Before that it breaks on request->env not existing. The response_cb is called, so $writer is returned to somewhere, but the write method is newer called as far as I can tell, so the $writer->body remains empty. This might be a regression in Catalyst itself, I am not sure.

Wed Apr 02 23:48:51 2014 andreas.marienborg [...] gmail.com - Correspondence added

On Wed Apr 02 22:12:50 2014, ANDREMAR wrote: Show quoted text

> the t/02subreq.t test breaks on 5.90006x. > > I am not sure exactly why, but the change in catalyst that breaks it > deals with changes in body handling (https://github.com/perl- > catalyst/catalyst- > runtime/commit/46fff667a1229a59422010500311e48a6f9da824) > > https://metacpan.org/source/EDENC/Catalyst-Plugin-SubRequest- > 0.20/lib/Catalyst/Plugin/SubRequest.pm#L112 > > If $writer->body is changed to $i_ctx->res->body, all the tests pass > again. I don't understand all the mechanics, so this might not be the > right fix. I have tested all Catalyt 5.9x versions, and it works back > to 5.90008. Before that it breaks on request->env not existing. > > The response_cb is called, so $writer is returned to somewhere, but > the write method is newer called as far as I can tell, so the $writer-

> >body remains empty.

> > This might be a regression in Catalyst itself, I am not sure.

https://gist.github.com/jjn1056/9947765 also has some proposed fixes from jnap, but I haven't tested those myself.

Thu Apr 03 00:13:36 2014 andreas.marienborg [...] gmail.com - Correspondence added

some more notes from jnap: jnap: like you are using a write_fh or calling $response->write jnap: the test should include an assigned body and delayed one jnap: I'd want to add a note that people should look at something like Plack::Middleware::Recursive instead jnap: I nearly wanted to add that as a default middleware but in the end felt I was going too far

Sun May 11 15:54:29 2014 dam [...] cpan.org - Correspondence added

Show quoted text

> https://gist.github.com/jjn1056/9947765 also has some proposed fixes > from jnap, but I haven't tested those myself.

Attached is a patch that corresponds to the above source. Fixes the test for me and will be included in the Debian package. Cheers, dam

Subject:

Cat-5.9.patch

Description: compatibility with Catalyst 5.9 Taken from https://gist.github.com/jjn1056/9947765 Author: John Napiorkowski Bug: https://rt.cpan.org/Public/Bug/Display.html?id=94392 Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=747502 --- a/lib/Catalyst/Plugin/SubRequest.pm +++ b/lib/Catalyst/Plugin/SubRequest.pm @@ -102,18 +102,29 @@ sub sub_request_response { # need this so that my $writer = Catalyst::Plugin::SubRequest::Writer->new; - my $response_cb = sub { $writer }; + my $response_cb = sub { + my $response = shift; + my ($status, $headers, $body) = @$response; + if($body) { + return; + } else { + return $writer; + } + }; + my $i_ctx = $class->prepare( env => $env, response_cb => $response_cb ); $i_ctx->stash($stash); $i_ctx->dispatch; $i_ctx->finalize; $c->stats->profile( end => 'subrequest: ' . $path ) if $c->debug; - $i_ctx->response->body($writer->body); + if($writer->_is_closed) { + $i_ctx->response->body($writer->body); + } return $i_ctx->response; } package Catalyst::Plugin::SubRequest::Writer; use Moose; has body => (

Sun May 11 15:54:31 2014 The RT System itself - Status changed from 'new' to 'open'

Mon May 26 11:13:47 2014 alexm [...] cpan.org - Cc ALEXM added

Mon May 26 11:44:42 2014 alexm [...] cpan.org - Correspondence added

Installation of Gitalist fails in perl-5.18.2 due to this bug. Dam's patch fixes it. Thanks!

Mon Sep 15 16:12:59 2014 rt-cpan [...] trout.me.uk - Correspondence added

On Sun May 11 15:54:29 2014, DAM wrote: Show quoted text

> > https://gist.github.com/jjn1056/9947765 also has some proposed fixes > > from jnap, but I haven't tested those myself.

> > Attached is a patch that corresponds to the above source. Fixes the > test for me and will be included in the Debian package.

Should probably be 'if ($writer->body) {' actually.

Mon Nov 17 18:23:22 2014 ether [...] cpan.org - Correspondence added

Since this tickets (and others in the queue) have gone unattended for a few years, I nominate this dist for adoption by the catalyst cabal. My $work repo has several patches to this module as well. I would welcome being able to delete them and incorporate the patches back into the cpan version.