[Catalyst-dev] Annoying undef warnings from CP::Static::Simple

[Dave Rolsky]

On Sat, 31 May 2008, Matt S Trout wrote:

> On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
>> I'm getting a bunch of warnings from requests for static content that look
>> like this:
>>
>>  Use of uninitialized value in length at
>>  /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
>>
>> The problem is that Catalyst::Dispatcher expects $c->req->match to be
>> defined, but it isn't when CPSS does the dispatching.
>>
>> I'm not sure where this should be fixed. Should $c->req->match always be
>> set? Or should CD just not assume it is defined?
>
> There's no $c->action at all when CPSS fires so I don't think I can really
> see $c->req->match as making a lot of sense in that case.
>
> This has already been reported via rt.cpan but sans test; if you can whip one
> up there's no reason we can't add a defined() check.

I've attached a new test and a patch to make it pass.

As an aside, I realized that part of the reason this was never fixed is 
that the core Catalyst code does not enable "use warnings" in the modules. 
However, if you run under the -w flag like the standalone server does, 
then you get all the warnings from everywhere.

I think it'd be a good idea to turn on warnings in the core modules and 
then shut them up. Are such patches welcome?


-dave

/*==========================
VegGuide.Org
Your guide to all that's veg
==========================*/
-------------- next part --------------
=== lib/Catalyst/Dispatcher.pm
==================================================================
--- lib/Catalyst/Dispatcher.pm	(revision 7868)
+++ lib/Catalyst/Dispatcher.pm	(local)
@@ -285,7 +285,7 @@
     s/%([0-9A-Fa-f]{2})/chr(hex($1))/eg for grep { defined } @{$c->req->captures||[]};
 
     $c->log->debug( 'Path is "' . $c->req->match . '"' )
-      if ( $c->debug && length $c->req->match );
+      if ( $c->debug && defined $c->req->match && length $c->req->match );
 
     $c->log->debug( 'Arguments are "' . join( '/', @args ) . '"' )
       if ( $c->debug && @args );
=== t/unit_core_prepare_action.t
==================================================================
--- t/unit_core_prepare_action.t	(revision 7868)
+++ t/unit_core_prepare_action.t	(local)
@@ -0,0 +1,42 @@
+#!perl -w
+
+# Note that the -w flag is important since we need to turn on warnings
+# _globally_ rather than just lexically to test warnings from
+# Catalyst::Dispatcher.
+
+use strict;
+use warnings;
+
+use FindBin;
+use lib "$FindBin::Bin/lib";
+
+use Test::More;
+
+eval "use Test::Warn";
+plan skip_all => 'Test::Warn required' if $@;
+
+plan tests => 2;
+
+use_ok('TestApp');
+
+my $dispatcher = TestApp->dispatcher;
+
+my $request = Catalyst::Request->new( {
+                base => URI->new('http://127.0.0.1/foo'),
+                uri => URI->new('http://127.0.0.1/foo'),
+              } );
+
+my $context = TestApp->new( {
+                request => $request,
+                namespace => 'yada',
+              } );
+
+# We want to turn this on after making the object or else we get a ton
+# of debugging spew
+eval 'sub TestApp::debug { 1 }';
+
+$request->match(undef);
+$dispatcher->dispatch_types( [] );
+
+warning_is( sub { $dispatcher->prepare_action($context) }, '',
+            'no warnings from prepare_action when debugging is on and $req->match is undef' );

Property changes on: t/unit_core_prepare_action.t
___________________________________________________________________
Name: svn:eol-style
 +native
Name: svn:keywords
 +Author Date Id Rev