[NUUG fiksgatami] [patch] Avoid internal server errors and improve error message from fixmystreet

[Petter Reinholdtsen]

Hi.  You get this email because you were listed in the header of the
affected files, and Matthew Somerville earlier told me that it was the
proper contact address for patches.

For some of the pages in our fixmystreet copy, we ran into internal
server error.  I finally tracked it down, and propose this patch to
fix it properly.

The source of the first problem was select statements failing with a
norwegian locale.  The source of the second problem was an unhandled
RABX::Error exception from the RABX library, caused by the fact that
<URL: http://services.mysociety.org/mapit > refuses to talk to our
server.  This exception is not handled by Page::do_fastcgi() and I
belive that is the error to be fixed.  This patch implement four
things:

 - Improve the text in the database search error to include the
   failing SQL statement.

 - Improve the text message in the RABX::Error sent from the RABX
   library, to make it easier to understand which URL is failing.

 - Catch Error as well as Error::Simple in Page::do_fastcgi(), to make
   sure the DB and RABX exceptions are handled and reported instead of
   getting a internal server error.

 - Make sure to HTML escape exception messages presented on a web
   page.  This was required to be able to properly display SQL
   statements.

Please include these patches in a future version of the fixmystreet
code.

Index: perllib/RABX.pm
===================================================================
RCS file: /repos/mysociety/perllib/RABX.pm,v
retrieving revision 1.23
diff -u -3 -p -u -r1.23 RABX.pm
--- perllib/RABX.pm     14 Aug 2007 12:09:32 -0000      1.23
+++ perllib/RABX.pm     24 Jan 2009 08:46:47 -0000
@@ -542,7 +542,7 @@ sub call ($$@) {
     my $resp = $self->ua()->request($req);

     if (!$resp->is_success()) {
-        throw RABX::Error("HTTP error: " . $resp->status_line(), RABX::Error::TRANSPORT);
+        throw RABX::Error("HTTP error for <" . $self->url() . ">: " . $resp->status_line(), RABX::Error::TRANSPORT);
     } else {
         my $parsed = RABX::return_string_parse($resp->content());
         return $parsed;


Index: perllib/mySociety/DBHandle.pm
===================================================================
RCS file: /repos/mysociety/perllib/mySociety/DBHandle.pm,v
retrieving revision 1.19
diff -u -3 -p -r1.19 DBHandle.pm
--- perllib/mySociety/DBHandle.pm       20 Apr 2007 00:47:19 -0000      1.19
+++ perllib/mySociety/DBHandle.pm       24 Jan 2009 08:53:32 -0000
@@ -188,6 +188,14 @@ END {

 sub select_all {
     my ($query, @bind_values) = @_;
+    local dbh()->{HandleError} =
+        sub ($$$) {
+            my ($err) = @_;
+            # Let's not make any unwise assumptions about reentrancy here.
+            local dbh()->{HandleError} = sub ($$$) { };
+            dbh()->rollback();
+            throw mySociety::DBHandle::Error($err . ": '" . $query . "' - args '" . join("','", @bind_values) . "'");
+        };
     dbh()->selectall_arrayref($query, { Slice => {} }, @bind_values);
 }

Index: bci/perllib/Page.pm
===================================================================
RCS file: /repos/mysociety/bci/perllib/Page.pm,v
retrieving revision 1.135
diff -u -3 -p -u -r1.135 Page.pm
--- bci/perllib/Page.pm 20 Jan 2009 14:58:05 -0000      1.135
+++ bci/perllib/Page.pm 24 Jan 2009 08:52:16 -0000
@@ -59,8 +59,17 @@ sub do_fastcgi {
             $W->exit_if_changed();
         }
     } catch Error::Simple with {
+        report_error(@_);
+    } catch Error with {
+        report_error(@_);
+    };
+    dbh()->rollback() if $mySociety::DBHandle::conf_ok;
+    exit(0);
+}
+
+sub report_error {
         my $E = shift;
-        my $msg = sprintf('%s:%d: %s', $E->file(), $E->line(), $E->text());
+        my $msg = sprintf('%s:%d: %s', $E->file(), $E->line(), CGI::escapeHTML($E->text()));
         warn "caught fatal exception: $msg";
         warn "aborting";
         ent($msg);
@@ -74,9 +83,6 @@ sub do_fastcgi {
                 q(<p>The text of the error was:</p>),
                 qq(<blockquote class="errortext">$msg</blockquote>),
                 q(</body></html>);
-    };
-    dbh()->rollback() if $mySociety::DBHandle::conf_ok;
-    exit(0);
 }

 =item microsite Q