Bug #78821 for Catalyst-Plugin-SubRequest: Catalyst-Plugin-SubRequest 0.18 breaks MojoMojo's t/formatter

Mon Aug 06 16:50:35 2012 fschlich [...] zedat.fu-berlin.de - Ticket created

Subject:

Catalyst-Plugin-SubRequest 0.18 breaks MojoMojo's t/formatter_include.t

Hi, the recent "major refactoring" in Catalyst-Plugin-SubRequest 0.18 causes breakage in MojoMojo's t/formatter_include.t, which instead of being given a minimal page as would be created by MojoMojo::Controller::Page->inline(), receives a complete page as created by calling MojoMojo::Controller::Page->view() directly. The relevant call in MojoMojo::Formatter::Include is: return $c->subreq( '/inline', { path => $rel.'' eq './' ? '/' : '/'.$rel } ); For full test output see for example http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=680796 or (among a number of other issues) http://www.cpantesters.org/cpan/report/84a482d2-c2a9-11e1-b3ca-c21d08f5d1fc I have confirmed that downgrading Catalyst-Plugin-SubRequest from 0.18 to 0.17 fixes MojoMojo's build test failure, so I'm filing this bug here. I cannot claim to grok the MojoMojo/Catalyst stack, though, so please advise if this belongs elsewhere. Florian

Mon Aug 06 20:11:45 2012 EDENC [...] cpan.org - Correspondence added

On Mon Aug 06 16:50:35 2012, fschlich wrote: Show quoted text

> Hi, > > the recent "major refactoring" in Catalyst-Plugin-SubRequest 0.18 > causes breakage in MojoMojo's > t/formatter_include.t, which instead of being given a minimal page as > would be created by > MojoMojo::Controller::Page->inline(), receives a complete page as > created by calling > MojoMojo::Controller::Page->view() directly. The relevant call in > MojoMojo::Formatter::Include is: > > return $c->subreq( '/inline', { path => $rel.'' eq './' ? '/' : > '/'.$rel } ); > > For full test output see for example http://bugs.debian.org/cgi- > bin/bugreport.cgi?bug=680796 or (among > a number of other issues) > http://www.cpantesters.org/cpan/report/84a482d2-c2a9-11e1-b3ca- > c21d08f5d1fc > > I have confirmed that downgrading Catalyst-Plugin-SubRequest from 0.18 > to 0.17 fixes MojoMojo's build > test failure, so I'm filing this bug here. I cannot claim to grok the > MojoMojo/Catalyst stack, though, > so please advise if this belongs elsewhere. > > Florian

There was an implementation flaw in the old version of Catalyst::Plugin::SubRequest that made it not go through the full request/dispatch cycle and fixing it broke MojoMojo. You can assert this by firing up the test wiki that comes with MojoMojo: CATALYST_DEBUG=1 CATALYST_CONFIG=t/var/mojomojo.yml plackup -Ilib -MMojoMojo -e'MojoMojo->psgi_app' If you request the /inline URL, you'll notice that it dispatches to MojoMojo::Controller::Page->view instead, because of the hacking done by the override in MojoMojo->prepare_path. The docs say the call to subrequest is supposed to "work like an external url call", and that's what the current implementation is doing. Solving this is complicated, we either: 1) change the specification of how SubRequest is supposed to work; 2) fix MojoMojo.

Mon Aug 06 20:11:47 2012 The RT System itself - Status changed from 'new' to 'open'

Tue Aug 07 09:02:10 2012 EDENC [...] cpan.org - Correspondence added

On Mon Aug 06 16:50:35 2012, fschlich wrote: Show quoted text

> Hi, > > the recent "major refactoring" in Catalyst-Plugin-SubRequest 0.18 > causes breakage in MojoMojo's > t/formatter_include.t, which instead of being given a minimal page as > would be created by > MojoMojo::Controller::Page->inline(), receives a complete page as > created by calling > MojoMojo::Controller::Page->view() directly. The relevant call in > MojoMojo::Formatter::Include is: > > return $c->subreq( '/inline', { path => $rel.'' eq './' ? '/' : > '/'.$rel } ); > > For full test output see for example http://bugs.debian.org/cgi- > bin/bugreport.cgi?bug=680796 or (among > a number of other issues) > http://www.cpantesters.org/cpan/report/84a482d2-c2a9-11e1-b3ca- > c21d08f5d1fc

This is a separate issue from the mojomojo one, the problem is that SubRequest's behaviour is undefined for delayed streaming responses, I think what I have in the latest svn commit should fix the problem. If you can assert that it works for the cases you presented, and I'll release the fix as 0.20 Show quoted text

> I have confirmed that downgrading Catalyst-Plugin-SubRequest from 0.18 > to 0.17 fixes MojoMojo's build > test failure, so I'm filing this bug here. I cannot claim to grok the > MojoMojo/Catalyst stack, though, > so please advise if this belongs elsewhere.

Confirmed that this is an issue with MojoMojo and it has been fixed: https://github.com/marcusramberg/mojomojo/commit/f52d2941c0c2ddc59a1015563c622f0fbb26a19d

Tue Aug 07 14:51:03 2012 fschlich [...] zedat.fu-berlin.de - Correspondence added

Subject:	Re: [rt.cpan.org #78821] Catalyst-Plugin-SubRequest 0.18 breaks MojoMojo's t/formatter_include.t
Date:	Tue, 7 Aug 2012 20:50:51 +0200
To:	Eden Cardim via RT <bug-Catalyst-Plugin-SubRequest [...] rt.cpan.org>
From:	Florian Schlichting <fschlich [...] zedat.fu-berlin.de>

On Tue, Aug 07, 2012 at 09:02:12AM -0400, Eden Cardim via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=78821 > > On Mon Aug 06 16:50:35 2012, fschlich wrote:

> > http://www.cpantesters.org/cpan/report/84a482d2-c2a9-11e1-b3ca- > > c21d08f5d1fc

> > This is a separate issue from the mojomojo one,

you're right, obviously I wasn't looking to closely. It still helped me narrow down who to ask :-) Show quoted text

> Confirmed that this is an issue with MojoMojo and it has been fixed: > > https://github.com/marcusramberg/mojomojo/commit/f52d2941c0c2ddc59a1015563c622f0fbb26a19d

yup, nice! Thank you very much for your quick reply and help in sorting this out! Florian

Tue Aug 07 17:15:33 2012 EDENC [...] cpan.org - Correspondence added

fixed in 0.20

Tue Aug 07 17:15:36 2012 EDENC [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #78821 for Catalyst-Plugin-SubRequest: Catalyst-Plugin-SubRequest 0.18 breaks MojoMojo's t/formatter_include.t