added retry cutoff and send_fail data to problem table for webservice send-reports

4 files changed, 68 insertions, 11 deletions

diff --git a/bin/send-reports b/bin/send-reports
index 8ea43da11..db9a7f3df 100755
--- a/bin/send-reports
+++ b/bin/send-reports
@@ -30,7 +30,10 @@ use mySociety::MaPit;
 use mySociety::Web qw(ent);
 
 use Open311;
- 
+
+# maximum number of webservice attempts to send before not trying any more (XXX may be better in config?)
+use constant SEND_FAIL_RETRIES_CUTOFF => 3;
+
 # specific council numbers
 use constant COUNCIL_ID_BARNET     => 2489;
 use constant COUNCIL_ID_EAST_HANTS => 2330;
@@ -293,7 +296,13 @@ while (my $row = $unsent->next) {
     } elsif ($send_method eq SEND_METHOD_BARNET) {
         $h{message} = construct_barnet_message(%h);
         if (!$nomail) {
-            $result *= post_barnet_message( $row, %h );
+            if (my $err_msg = does_exceed_cutoff_limit($row, "barnet")) {
+                print "$err_msg\n"; # if $verbose
+            } else {
+                my ($barnet_result, $err_msg) = post_barnet_message( $row, %h );
+                update_send_fail_data($row, $err_msg) if $barnet_result;
+                $result *= $barnet_result;
+            } 
         }
     } elsif ($send_method eq SEND_METHOD_LONDON) {
         $h{message} = construct_london_message(%h);
@@ -453,12 +462,12 @@ $h{url}
 
 $h{closest_address}
 EOF
-    return $message;
 }
 
 sub post_barnet_message {
     my ( $problem, %h ) = @_;
     my $return = 1;
+    my $err_msg = "";
 
     my $default_kbid = 14; # This is the default, "Street Scene"
     my $kbid = sprintf( "%050d",  Utils::barnet_categories()->{$h{category}} || $default_kbid);
@@ -524,27 +533,28 @@ sub post_barnet_message {
                 $problem->external_id( $barnet_id );
                 $problem->external_body( 'Barnet Borough Council' ); # better to use $problem->body()?
                 $return = 0;
-            } else {
-                print "Failed (problem id $h{id}): service returned no external id\n";                
+            } else { 
+                $err_msg = "Failed (problem id $h{id}): service returned no external id";
             }
         } else {
             my %fault = (
                     'code' =>  $result->get_faultcode(),
                     'actor' =>  $result->get_faultactor(),
                     'string' =>  $result->get_faultstring(),
-                    # 'detail' =>  $result->get_detail(), # possibly only contains debug info
+                    # 'detail' =>  $result->get_detail(), # possibly only contains debug info
                 );
             $fault{$_}=~s/^\s*|\s*$//g foreach keys %fault;
             $fault{actor}&&=" (actor: $fault{actor})";
-            print "Failed (problem id $h{id}): Fault $fault{code}$fault{actor}\n$fault{string}\n";
+            $err_msg = "Failed (problem id $h{id}): Fault $fault{code}$fault{actor}\n$fault{string}";
         }
 
     };
+    print "$err_msg\n" if $err_msg;
     if ($@) {
         my $e = shift;
         print "Caught an error: $@\n"; 
     }
-    return $return;
+    return ($return, $err_msg);
 }
 
 # London
@@ -696,3 +706,29 @@ sub split_text_with_entities {
     }
     return @lines;
 }
+
+# tests send_fail_count agains cutoff limit
+#   args: problem (row from problem db)
+#   returns false if there is no cutoff, otherwise error message
+sub does_exceed_cutoff_limit {
+    my ($problem, $council_name) = @_;
+    my $err_msg = "";
+    if ($problem->send_fail_count >= SEND_FAIL_RETRIES_CUTOFF) {
+        $council_name &&= " to $council_name";
+        $err_msg = "skipped: problem id=" . $problem->id . " send$council_name has failed " 
+                . $problem->send_fail_count . " times, cutoff is " . SEND_FAIL_RETRIES_CUTOFF;
+    }
+    return $err_msg;
+}
+
+# update_send_fail_data records the failure (of a webservice send)
+#  args: problem (row from problem db)
+#  returns: no return value (updates record)
+sub update_send_fail_data {
+    my ($problem, $err_msg) = @_;
+    $problem->update( {
+        send_fail_count => $problem->send_fail_count + 1,
+        send_fail_timestamp => \'ms_current_timestamp()',
+        send_fail_reason => $err_msg
+    } ); 
+}
\ No newline at end of file
diff --git a/db/schema.sql b/db/schema.sql
index d24d4dd96..5824b2d6d 100644
--- a/db/schema.sql
+++ b/db/schema.sql
@@ -189,7 +189,12 @@ create table problem (
     send_questionnaire boolean not null default 't',
     extra text, -- extra fields required for open311
     flagged boolean not null default 'f',
-    geocode bytea
+    geocode bytea,
+    
+    -- logging sending failures (used by webservices)
+    send_fail_count integer not null default 0, 
+    send_fail_reason text, 
+    send_fail_timestamp timestamp
 );
 create index problem_state_latitude_longitude_idx on problem(state, latitude, longitude);
 create index problem_user_id_idx on problem ( user_id );
diff --git a/db/schema_0014-add_send_fail_columns_to_problem.sql b/db/schema_0014-add_send_fail_columns_to_problem.sql
new file mode 100644
index 000000000..369c4118d
--- /dev/null
+++ b/db/schema_0014-add_send_fail_columns_to_problem.sql
@@ -0,0 +1,10 @@
+begin;
+
+ALTER table problem
+    ADD column send_fail_count integer not null default 0;
+ALTER table problem
+    ADD column send_fail_reason text; 
+ALTER table problem
+    ADD column send_fail_timestamp timestamp;
+
+commit;
diff --git a/perllib/FixMyStreet/DB/Result/Problem.pm b/perllib/FixMyStreet/DB/Result/Problem.pm
index 7429db832..4b738b66c 100644
--- a/perllib/FixMyStreet/DB/Result/Problem.pm
+++ b/perllib/FixMyStreet/DB/Result/Problem.pm
@@ -84,6 +84,12 @@ __PACKAGE__->add_columns(
   { data_type => "boolean", default_value => \"false", is_nullable => 0 },
   "geocode",
   { data_type => "bytea", is_nullable => 1 },
+  "send_fail_count",
+  { data_type => "integer", is_nullable => 1 },
+  "send_fail_reason",
+  { data_type => "text", is_nullable => 1 },
+  "send_fail_timestamp",
+  { data_type => "timestamp", is_nullable => 1 },
 );
 __PACKAGE__->set_primary_key("id");
 __PACKAGE__->has_many(
@@ -106,8 +112,8 @@ __PACKAGE__->belongs_to(
 );
 
 
-# Created by DBIx::Class::Schema::Loader v0.07017 @ 2012-03-08 17:19:55
-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:B5hs93gt+TwgPbdTFTJMjA
+# Created by DBIx::Class::Schema::Loader v0.07017 @ 2012-03-16 10:08:56
+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:VODeZlWk8l/+IzBBlRNV0A
 
 # Add fake relationship to stored procedure table
 __PACKAGE__->has_one(