fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2011-01-04 12:17 am

[dw-free] Routing should output text/plain errors for unknown ( and non-HTML ) formats

[commit: http://hg.dwscoalition.org/dw-free/rev/5288d9d5a226]

http://bugs.dwscoalition.org/show_bug.cgi?id=3171

Return an error page in the proper format, depending on the content type of
the page.

Patch by [personal profile] fu.

Files modified:
  • cgi-bin/DW/Request/Standard.pm
  • cgi-bin/DW/Routing.pm
  • t/routing.t
--------------------------------------------------------------------------------
diff -r 3ec2525b0c3e -r 5288d9d5a226 cgi-bin/DW/Request/Standard.pm
--- a/cgi-bin/DW/Request/Standard.pm	Mon Jan 03 15:51:45 2011 -0600
+++ b/cgi-bin/DW/Request/Standard.pm	Tue Jan 04 08:17:10 2011 +0800
@@ -282,6 +282,7 @@ sub OK        { return 200; }
 sub OK        { return 200; }
 sub REDIRECT  { return 302; }
 sub NOT_FOUND { return 404; }
+sub HTTP_SERVER_ERROR { return 500; }
 
 # spawn a process for an external program
 sub spawn {
diff -r 3ec2525b0c3e -r 5288d9d5a226 cgi-bin/DW/Routing.pm
--- a/cgi-bin/DW/Routing.pm	Mon Jan 03 15:51:45 2011 -0600
+++ b/cgi-bin/DW/Routing.pm	Tue Jan 04 08:17:10 2011 +0800
@@ -189,7 +189,7 @@ sub _call_hash {
         $r->print(objToJson( { error => $text } ));
         return $r->OK;
     # default error rendering
-    } else {
+    } elsif ( $format eq "html" ) {
         $msg = $err->as_html;
 
         chomp $msg;
@@ -201,6 +201,22 @@ sub _call_hash {
         my $remote = LJ::get_remote();
         $text = "<b>[Error: $msg]</b>" if ( $remote && $remote->show_raw_errors ) || $LJ::IS_DEV_SERVER;
         return DW::Template->render_string( $text, { status=>500, content_type=>'text/html' } );
+    } else {
+        $msg = $err->as_string;
+
+        chomp $msg;
+        $msg .= " \@ $LJ::SERVER_NAME" if $LJ::SERVER_NAME;
+        warn "$msg\n";
+
+        my $text = $LJ::MSG_ERROR || "Sorry, there was a problem.";
+        my $remote = LJ::get_remote();
+        $text = "Error: $msg" if ( $remote && $remote->show_raw_errors ) || $LJ::IS_DEV_SERVER;
+
+        return DW::Template->render_string( $text, {
+            status => 500,
+            content_type => 'text/plain',
+            no_sitescheme => 1
+        } );
     }
 }
 
diff -r 3ec2525b0c3e -r 5288d9d5a226 t/routing.t
--- a/t/routing.t	Mon Jan 03 15:51:45 2011 -0600
+++ b/t/routing.t	Tue Jan 04 08:17:10 2011 +0800
@@ -14,7 +14,7 @@
 # 'perldoc perlartistic' or 'perldoc perlgpl'.
 #
 use strict;
-use Test::More tests => 195;
+use Test::More tests => 203;
 use lib "$ENV{LJHOME}/cgi-bin";
 
 # don't let DW::Routing load DW::Controller subclasses
@@ -251,6 +251,16 @@ handle_redirect( '/xx3', '/xx3/' );
 handle_redirect( '/xx3', '/xx3/' );
 # 194
 
+
+# test dying
+DW::Routing->register_string( "/test/die/all_format", \&died_handler, app => 1, formats => 1 );
+
+handle_server_error( "/test die implied_format (app)", "/test/die/all_format", "html" ); # 2 tests
+handle_server_error( "/test die .json format (app)", "/test/die/all_format.json", "json" ); # 2 tests
+handle_server_error( "/test die .html format (app)", "/test/die/all_format.html", "html" ); # 2 tests
+handle_server_error( "/test die .blah format (app)", "/test/die/all_format.blah", "blah" ); # 2 tests
+
+
 sub handle_redirect {
     my ( $uri, $expected ) = @_;
 
@@ -296,6 +306,26 @@ sub handle_request {
     is ( $result, $expected, "$name: handler set wrong value.");
 }
 
+sub handle_server_error {
+    my ( $name, $uri, $format, %opts ) = @_;
+
+    $DW::Request::determined = 0;
+    $DW::Request::cur_req = undef;
+
+    my $req = HTTP::Request->new( GET => "$uri" );
+    my $r = DW::Request::Standard->new( $req );
+
+    $result = undef;
+    $__name = $name;
+    eval { DW::Routing->call( %opts ) };
+
+    is( $r->status, $r->HTTP_SERVER_ERROR, "$name: wrong return" );
+    is( $r->content_type, {
+        html => "text/html",
+        json => "application/json",
+    }->{$format} || "text/plain", "$name: wrong returned content type for $format" );
+}
+
 sub handler {
     my $r = DW::Request->get;
     $result = $_[0]->args;
@@ -310,3 +340,7 @@ sub regex_handler {
     is( $_[0]->subpatterns->[0], $_[0]->args->[0], "$__name: capture wrong!" );
     return $r->OK;
 }
+
+sub died_handler {
+    die "deliberate die()";
+}
--------------------------------------------------------------------------------