Bug #79890 for CGI-Application-Dispatch: PSGI exception handling problem

Thu Sep 27 17:38:14 2012 ford.rick [...] gmail.com - Ticket created

Subject:

PSGI exception handling problem

In the http_error method of CAD::Dispatch, lines 351-353 reference $ENV{REQUEST_URI}, but %ENV may not be populated with that information if running as a PSGI app - that would normally be stored in $env... but $env is not passed to http_error(). These lines are also going to die if passed a $e which doesn't do ->error() (like a string, for example). Finally, it should probably be tied to $DEBUG like most of the other warnings.

Tue Oct 02 09:33:28 2012 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #79890] PSGI exception handling problem
Date:	Tue, 02 Oct 2012 09:33:04 -0400
To:	bug-cgi-application-dispatch [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

Thanks for the report, Rick. Are you up for submitting a test case and/or patch? Mark

Tue Oct 02 09:33:29 2012 The RT System itself - Status changed from 'new' to 'open'

Tue Oct 02 15:15:28 2012 rick [...] peregrin.org - Correspondence added

Subject:	Re: [rt.cpan.org #79890] PSGI exception handling problem
Date:	Tue, 2 Oct 2012 14:15:08 -0500
To:	bug-CGI-Application-Dispatch [...] rt.cpan.org
From:	Rick Ford <rick [...] peregrin.org>

I probably could write a patch, but I'm not sure what the actual intent behind the code is. The most obvious solution is just to delete those lines. It looks like debugging code to me, and getting it to work where it is with $env instead of $ENV would be more work than it's worth. As for a test case, run a PSGI app and request a non-existent URL. You'll get a 404 and a warning "[Dispatch] ERROR: Resource not found" without the request URI. -Rick On Tue, Oct 2, 2012 at 8:33 AM, mark@summersault.com via RT <bug-CGI-Application-Dispatch@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=79890 > > > Thanks for the report, Rick. > > Are you up for submitting a test case and/or patch? > > Mark >

Wed Oct 03 14:37:24 2012 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #79890] PSGI exception handling problem
Date:	Wed, 03 Oct 2012 14:36:57 -0400
To:	bug-CGI-Application-Dispatch [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

On 10/02/2012 03:15 PM, Rick Ford via RT wrote: Show quoted text

> Queue: CGI-Application-Dispatch > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=79890 > > > I probably could write a patch, but I'm not sure what the actual > intent behind the code is. > > The most obvious solution is just to delete those lines. It looks > like debugging code to me, and getting it to work where it is with > $env instead of $ENV would be more work than it's worth. > > As for a test case, run a PSGI app and request a non-existent URL. > You'll get a 404 and a warning "[Dispatch] ERROR: Resource not found" > without the request URI.

Thanks for the reply, Rick, I think it's useful for developers to see some context about why dispatching failed. What do you think about this approach: ? Update the signature for http_error(), so instead of this: http_error($e) We pass this: http_error($env->{REQUEST_URI},$e)? According to the PSGI spec, it should be there: https://metacpan.org/module/PSGI Mark

Wed Oct 03 15:57:46 2012 rick [...] peregrin.org - Correspondence added

Subject:	Re: [rt.cpan.org #79890] PSGI exception handling problem
Date:	Wed, 3 Oct 2012 14:57:34 -0500
To:	bug-CGI-Application-Dispatch [...] rt.cpan.org
From:	Rick Ford <rick [...] peregrin.org>

I don't think changing the signature is necessary - I'd just include the path_info in the error if it's relevant at that point (line 365 in the current code). If you do decide to change the signature, I'd suggest going all the way and passing it the $env reference instead of just the REQUEST_URI - that way you have access to everything you might want to log in the future. Either way, I'd suggest getting rid of the warning or making it depend on the debug setting. And if you keep it, check to make sure $e does ->error() before calling it. -Rick On Wed, Oct 3, 2012 at 1:37 PM, mark@summersault.com via RT <bug-CGI-Application-Dispatch@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=79890 > > > On 10/02/2012 03:15 PM, Rick Ford via RT wrote:

>> Queue: CGI-Application-Dispatch >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=79890 > >> >> I probably could write a patch, but I'm not sure what the actual >> intent behind the code is. >> >> The most obvious solution is just to delete those lines. It looks >> like debugging code to me, and getting it to work where it is with >> $env instead of $ENV would be more work than it's worth. >> >> As for a test case, run a PSGI app and request a non-existent URL. >> You'll get a 404 and a warning "[Dispatch] ERROR: Resource not found" >> without the request URI.

> > Thanks for the reply, Rick, > > I think it's useful for developers to see some context about why > dispatching failed. What do you think about this approach: ? > > Update the signature for http_error(), so instead of this: > > http_error($e) > > We pass this: > > http_error($env->{REQUEST_URI},$e)? > > According to the PSGI spec, it should be there: > > https://metacpan.org/module/PSGI > > Mark >

Thu Oct 04 10:13:12 2012 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #79890] PSGI exception handling problem
Date:	Thu, 04 Oct 2012 10:12:38 -0400
To:	bug-cgi-application-dispatch [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

Thanks for your feedback, Rick.