[Catalyst-dev] Minor Catalyst::Script::Server bug - with patch!

[Louis Erickson]

I found a minor bug in Catalyst today.  It showed up in our development environment, and made me crazy trying to track it down.

If you have a MyApp::Script::Server class, when you run myapp_server.pl, it will sometimes miss checking the values for MYAPP_PORT and MYAPP_RELOAD from the environment.

It's the "sometimes" that made this hard to sort out.

As far as I can tell, the values from the constructor to the Server object are being set up in a different order.  Sometimes application_name is set up before port or reload, in which case the right app-specific environment variables are checked.  Sometimes port or reload (or both) are set up before application_name is initialized, and in those cases 'MyApp' isn't sent to env_value, so the MYAPP variant of the environment block isn't checked.

I never did figure out what caused the order to be different.  I can guess wildly, but don't really know for sure.

The solution - already used in one of the accessors in Catalyst::Script::Server - is to make port and reload lazy, so application_name will always be set before they're called.  Any of the accessors that use env_value in their default should be lazy so you can depend on application_name.  It's a trivial change.

It did break one of the tests, though, as the test uses direct hash access to test retrieved values.  For, as far as I can tell, no good reason.  Changing the test to a method call fixes it.

I couldn't figure out how to write a test to trigger this every time, so I have no idea how I'd write a test to prove it's really fixed.

Here, as requested on the web site, are diffs.  If there's a better way to get them to you, I do have this checked out in git and could push them someplace or something.

Let me know and I'll be happy to do so, so no one else has to spend half a day scratching their head over this one.

diff -u -b lib/Catalyst/Script/Server-orig.pm lib/Catalyst/Script/Server.pm 
--- lib/Catalyst/Script/Server-orig.pm	2014-05-26 20:03:00.000000000 -0700
+++ lib/Catalyst/Script/Server.pm	2014-05-26 20:02:23.000000000 -0700
@@ -37,6 +37,7 @@
     cmd_aliases   => 'p',
     isa           => 'Int',
     is            => 'ro',
+    lazy          => 1,
     default       => sub {
         Catalyst::Utils::env_value(shift->application_name, 'port') || 3000
     },
@@ -107,6 +108,7 @@
     cmd_aliases   => 'r',
     isa           => 'Bool',
     is            => 'ro',
+    lazy          => 1,
     default       => sub {
         Catalyst::Utils::env_value(shift->application_name, 'reload') || 0;
     },

diff -u -b t/aggregate/unit_core_script_server-orig.t t/aggregate/unit_core_script_server.t 
--- t/aggregate/unit_core_script_server-orig.t	2014-05-26 20:13:25.000000000 -0700
+++ t/aggregate/unit_core_script_server.t	2014-05-26 20:13:34.000000000 -0700
@@ -151,7 +151,7 @@
 
     ## Check a few args
     is_deeply $app->{ARGV}, $argstring;
-    is $app->{port}, '3000';
+    is $app->port, '3000';
     is($app->{background}, 1);
 }
 




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/catalyst-dev/attachments/20140526/99de64ac/attachment.htm>