Bug #114724 for Catalyst-Runtime: Behavior of $c->state inside $c->execute differs significantly from documentation

Thu May 26 05:21:43 2016 chase.venters [...] gmail.com - Ticket created

Subject:	Behavior of $c->state inside $c->execute differs significantly from documentation
Date:	Thu, 26 May 2016 14:51:34 +0530
To:	bug-Catalyst-Runtime [...] rt.cpan.org
From:	Chase Venters <chase.venters [...] gmail.com>

Greetings, Commit e459bd036c9076bd7301ea2fbdd5d4ba9900ddf0 to Catalyst::Runtime introduced a regression to our Catalyst code bases. We use a controller action, "authenticate" to check authentication credentials on web service endpoints called on our app. Because the behavior of $c->forward() is to wrap the action in eval { }, we followed the advice in the documentation to call $c->forward like this: $c->forward('authenticate') || return; This used to work just fine. Unfortunately, after the aforementioned commit, $c->state() is not set to 0. The line(s) setting $c->state to 0 are commented out in that commit along with the message "preserve $c->state". The unfortunate effect of this is that if we throw an exception inside our authenticate action, it is now short circuiting and allowing control flow to continue, despite our exception. For now, we resolved the issue by returning if $c->has_errors but it seems that this commit should either be reverted, or the documentation updated to reflect the fact that checking the $c->forward() return value is no longer a reliable way to handle exceptions. Thanks, Chase

Fri Jun 03 10:05:11 2016 jjnapiork [...] cpan.org - Correspondence added

On Thu May 26 05:21:43 2016, chase.venters@gmail.com wrote: Show quoted text

> Greetings, > > Commit e459bd036c9076bd7301ea2fbdd5d4ba9900ddf0 to Catalyst::Runtime > introduced a regression to our Catalyst code bases. > > We use a controller action, "authenticate" to check authentication > credentials on web service endpoints called on our app. Because the > behavior of $c->forward() is to wrap the action in eval { }, we followed > the advice in the documentation to call $c->forward like this: > > $c->forward('authenticate') || return; > > This used to work just fine. Unfortunately, after the aforementioned > commit, $c->state() is not set to 0. The line(s) setting $c->state to 0 are > commented out in that commit along with the message "preserve $c->state". > > The unfortunate effect of this is that if we throw an exception inside our > authenticate action, it is now short circuiting and allowing control flow > to continue, despite our exception. > > For now, we resolved the issue by returning if $c->has_errors but it seems > that this commit should either be reverted, or the documentation updated to > reflect the fact that checking the $c->forward() return value is no longer > a reliable way to handle exceptions. > > Thanks, > Chase

Hi, So let me see if I understand. If you forward to an action that throws an exception, you expect the return value of the forward to be false, is that it? Can you help me find the docs around this, you mentioned it was documented this way but I can't spot it in the ponderous POD :) jnap

Fri Jun 03 10:05:14 2016 The RT System itself - Status changed from 'new' to 'open'

Fri Jun 03 10:08:21 2016 jjnapiork [...] cpan.org - Correspondence added

On Fri Jun 03 10:05:11 2016, JJNAPIORK wrote: Show quoted text

> On Thu May 26 05:21:43 2016, chase.venters@gmail.com wrote:

> > Greetings, > > > > Commit e459bd036c9076bd7301ea2fbdd5d4ba9900ddf0 to Catalyst::Runtime > > introduced a regression to our Catalyst code bases. > > > > We use a controller action, "authenticate" to check authentication > > credentials on web service endpoints called on our app. Because the > > behavior of $c->forward() is to wrap the action in eval { }, we > > followed > > the advice in the documentation to call $c->forward like this: > > > > $c->forward('authenticate') || return; > > > > This used to work just fine. Unfortunately, after the aforementioned > > commit, $c->state() is not set to 0. The line(s) setting $c->state to > > 0 are > > commented out in that commit along with the message "preserve $c-

> > >state".

> > > > The unfortunate effect of this is that if we throw an exception > > inside our > > authenticate action, it is now short circuiting and allowing control > > flow > > to continue, despite our exception. > > > > For now, we resolved the issue by returning if $c->has_errors but it > > seems > > that this commit should either be reverted, or the documentation > > updated to > > reflect the fact that checking the $c->forward() return value is no > > longer > > a reliable way to handle exceptions. > > > > Thanks, > > Chase

> > Hi, > > So let me see if I understand. If you forward to an action that > throws an exception, you expect the return value of the forward to be > false, is that it? Can you help me find the docs around this, you > mentioned it was documented this way but I can't spot it in the > ponderous POD :) > > jnap

Also, I really don't follow RT because I get RT messages for every module I have comaint on, which is a lot and its basically all spam to me. Please register this over on github issues so I can better track where we are going with it, if you don't mind.

Fri Jun 03 13:36:41 2016 chase.venters [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #114724] Behavior of $c->state inside $c->execute differs significantly from documentation
Date:	Fri, 3 Jun 2016 23:06:30 +0530
To:	bug-Catalyst-Runtime [...] rt.cpan.org
From:	Chase Venters <chase.venters [...] gmail.com>

OK, I'm opening the issue on GitHub. Thanks. https://github.com/perl-catalyst/catalyst-runtime/issues/136 On Fri, Jun 3, 2016 at 7:38 PM, John Napiorkowski via RT < bug-Catalyst-Runtime@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=114724 > > > On Fri Jun 03 10:05:11 2016, JJNAPIORK wrote:

> > On Thu May 26 05:21:43 2016, chase.venters@gmail.com wrote:

> > > Greetings, > > > > > > Commit e459bd036c9076bd7301ea2fbdd5d4ba9900ddf0 to Catalyst::Runtime > > > introduced a regression to our Catalyst code bases. > > > > > > We use a controller action, "authenticate" to check authentication > > > credentials on web service endpoints called on our app. Because the > > > behavior of $c->forward() is to wrap the action in eval { }, we > > > followed > > > the advice in the documentation to call $c->forward like this: > > > > > > $c->forward('authenticate') || return; > > > > > > This used to work just fine. Unfortunately, after the aforementioned > > > commit, $c->state() is not set to 0. The line(s) setting $c->state to > > > 0 are > > > commented out in that commit along with the message "preserve $c-

> > > >state".

> > > > > > The unfortunate effect of this is that if we throw an exception > > > inside our > > > authenticate action, it is now short circuiting and allowing control > > > flow > > > to continue, despite our exception. > > > > > > For now, we resolved the issue by returning if $c->has_errors but it > > > seems > > > that this commit should either be reverted, or the documentation > > > updated to > > > reflect the fact that checking the $c->forward() return value is no > > > longer > > > a reliable way to handle exceptions. > > > > > > Thanks, > > > Chase

> > > > Hi, > > > > So let me see if I understand. If you forward to an action that > > throws an exception, you expect the return value of the forward to be > > false, is that it? Can you help me find the docs around this, you > > mentioned it was documented this way but I can't spot it in the > > ponderous POD :) > > > > jnap

> > Also, I really don't follow RT because I get RT messages for every module > I have comaint on, which is a lot and its basically all spam to me. Please > register this over on github issues so I can better track where we are > going with it, if you don't mind. > > >

Sun Oct 21 17:14:14 2018 haarg [...] haarg.org - Status changed from 'open' to 'resolved'

Sun Oct 21 17:14:14 2018 haarg [...] haarg.org - Fixed in 5.90105 added