improve error handling on mass email jobs, RT#8720
authormark <mark>
Tue, 13 Jul 2010 23:11:44 +0000 (23:11 +0000)
committermark <mark>
Tue, 13 Jul 2010 23:11:44 +0000 (23:11 +0000)
FS/FS/Misc.pm
FS/FS/cust_main.pm

index 895681f..19ac35c 100644 (file)
@@ -13,7 +13,7 @@ use File::Temp;
 #instead
 
 @ISA = qw( Exporter );
 #instead
 
 @ISA = qw( Exporter );
-@EXPORT_OK = qw( generate_email send_email send_fax
+@EXPORT_OK = qw( send_email generate_email send_fax
                  states_hash counties cities state_label
                  card_types
                  generate_ps generate_pdf do_print
                  states_hash counties cities state_label
                  card_types
                  generate_ps generate_pdf do_print
@@ -36,136 +36,12 @@ FS::Misc - Miscellaneous subroutines
 
 Miscellaneous subroutines.  This module contains miscellaneous subroutines
 called from multiple other modules.  These are not OO or necessarily related,
 
 Miscellaneous subroutines.  This module contains miscellaneous subroutines
 called from multiple other modules.  These are not OO or necessarily related,
-but are collected here to elimiate code duplication.
+but are collected here to eliminate code duplication.
 
 =head1 SUBROUTINES
 
 =over 4
 
 
 =head1 SUBROUTINES
 
 =over 4
 
-=item generate_email OPTION => VALUE ...
-
-Options:
-
-=over 4
-
-=item from
-
-Sender address, required
-
-=item to
-
-Recipient address, required
-
-=item subject
-
-email subject, required
-
-=item html_body
-
-Email body (HTML alternative).  Arrayref of lines, or scalar.
-
-Will be placed inside an HTML <BODY> tag.
-
-=item text_body
-
-Email body (Text alternative).  Arrayref of lines, or scalar.
-
-=back
-
-Returns an argument list to be passsed to L<send_email>.
-
-=cut
-
-#false laziness w/FS::cust_bill::generate_email
-
-use MIME::Entity;
-use HTML::Entities;
-
-sub generate_email {
-  my %args = @_;
-
-  my $me = '[FS::Misc::generate_email]';
-
-  my %return = (
-    'from'    => $args{'from'},
-    'to'      => $args{'to'},
-    'subject' => $args{'subject'},
-  );
-
-  #if (ref($args{'to'}) eq 'ARRAY') {
-  #  $return{'to'} = $args{'to'};
-  #} else {
-  #  $return{'to'} = [ grep { $_ !~ /^(POST|FAX)$/ }
-  #                         $self->cust_main->invoicing_list
-  #                  ];
-  #}
-
-  warn "$me creating HTML/text multipart message"
-    if $DEBUG;
-
-  $return{'nobody'} = 1;
-
-  my $alternative = build MIME::Entity
-    'Type'        => 'multipart/alternative',
-    'Encoding'    => '7bit',
-    'Disposition' => 'inline'
-  ;
-
-  my $data;
-  if ( ref($args{'text_body'}) eq 'ARRAY' ) {
-    $data = $args{'text_body'};
-  } else {
-    $data = [ split(/\n/, $args{'text_body'}) ];
-  }
-
-  $alternative->attach(
-    'Type'        => 'text/plain',
-    #'Encoding'    => 'quoted-printable',
-    'Encoding'    => '7bit',
-    'Data'        => $data,
-    'Disposition' => 'inline',
-  );
-
-  my @html_data;
-  if ( ref($args{'html_body'}) eq 'ARRAY' ) {
-    @html_data = @{ $args{'html_body'} };
-  } else {
-    @html_data = split(/\n/, $args{'html_body'});
-  }
-
-  $alternative->attach(
-    'Type'        => 'text/html',
-    'Encoding'    => 'quoted-printable',
-    'Data'        => [ '<html>',
-                       '  <head>',
-                       '    <title>',
-                       '      '. encode_entities($return{'subject'}), 
-                       '    </title>',
-                       '  </head>',
-                       '  <body bgcolor="#e8e8e8">',
-                       @html_data,
-                       '  </body>',
-                       '</html>',
-                     ],
-    'Disposition' => 'inline',
-    #'Filename'    => 'invoice.pdf',
-  );
-
-  #no other attachment:
-  # multipart/related
-  #   multipart/alternative
-  #     text/plain
-  #     text/html
-
-  $return{'content-type'} = 'multipart/related';
-  $return{'mimeparts'} = [ $alternative ];
-  $return{'type'} = 'multipart/alternative'; #Content-Type of first part...
-  #$return{'disposition'} = 'inline';
-
-  %return;
-
-}
-
 =item send_email OPTION => VALUE ...
 
 Options:
 =item send_email OPTION => VALUE ...
 
 Options:
@@ -365,6 +241,144 @@ sub send_email {
   }
 }
 
   }
 }
 
+=item generate_email OPTION => VALUE ...
+
+Options:
+
+=over 4
+
+=item from
+
+Sender address, required
+
+=item to
+
+Recipient address, required
+
+=item subject
+
+email subject, required
+
+=item html_body
+
+Email body (HTML alternative).  Arrayref of lines, or scalar.
+
+Will be placed inside an HTML <BODY> tag.
+
+=item text_body
+
+Email body (Text alternative).  Arrayref of lines, or scalar.
+
+=back
+
+Constructs a multipart message from text_body and html_body.
+
+=cut
+
+#false laziness w/FS::cust_bill::generate_email
+
+use MIME::Entity;
+use HTML::Entities;
+
+sub generate_email {
+  my %args = @_;
+
+  my $me = '[FS::Misc::generate_email]';
+
+  my %return = (
+    'from'    => $args{'from'},
+    'to'      => $args{'to'},
+    'subject' => $args{'subject'},
+  );
+
+  #if (ref($args{'to'}) eq 'ARRAY') {
+  #  $return{'to'} = $args{'to'};
+  #} else {
+  #  $return{'to'} = [ grep { $_ !~ /^(POST|FAX)$/ }
+  #                         $self->cust_main->invoicing_list
+  #                  ];
+  #}
+
+  warn "$me creating HTML/text multipart message"
+    if $DEBUG;
+
+  $return{'nobody'} = 1;
+
+  my $alternative = build MIME::Entity
+    'Type'        => 'multipart/alternative',
+    'Encoding'    => '7bit',
+    'Disposition' => 'inline'
+  ;
+
+  my $data;
+  if ( ref($args{'text_body'}) eq 'ARRAY' ) {
+    $data = $args{'text_body'};
+  } else {
+    $data = [ split(/\n/, $args{'text_body'}) ];
+  }
+
+  $alternative->attach(
+    'Type'        => 'text/plain',
+    #'Encoding'    => 'quoted-printable',
+    'Encoding'    => '7bit',
+    'Data'        => $data,
+    'Disposition' => 'inline',
+  );
+
+  my @html_data;
+  if ( ref($args{'html_body'}) eq 'ARRAY' ) {
+    @html_data = @{ $args{'html_body'} };
+  } else {
+    @html_data = split(/\n/, $args{'html_body'});
+  }
+
+  $alternative->attach(
+    'Type'        => 'text/html',
+    'Encoding'    => 'quoted-printable',
+    'Data'        => [ '<html>',
+                       '  <head>',
+                       '    <title>',
+                       '      '. encode_entities($return{'subject'}), 
+                       '    </title>',
+                       '  </head>',
+                       '  <body bgcolor="#e8e8e8">',
+                       @html_data,
+                       '  </body>',
+                       '</html>',
+                     ],
+    'Disposition' => 'inline',
+    #'Filename'    => 'invoice.pdf',
+  );
+
+  #no other attachment:
+  # multipart/related
+  #   multipart/alternative
+  #     text/plain
+  #     text/html
+
+  $return{'content-type'} = 'multipart/related';
+  $return{'mimeparts'} = [ $alternative ];
+  $return{'type'} = 'multipart/alternative'; #Content-Type of first part...
+  #$return{'disposition'} = 'inline';
+
+  %return;
+
+}
+
+=item process_send_email OPTION => VALUE ...
+
+Takes arguments as per generate_email() and sends the message.  This 
+will die on any error and can be used in the job queue.
+
+=cut
+
+sub process_send_email {
+  my %message = @_;
+  my $error = send_email(generate_email(%message));
+  die "$error\n" if $error;
+  '';
+}
+
 =item send_fax OPTION => VALUE ...
 
 Options:
 =item send_fax OPTION => VALUE ...
 
 Options:
index 0578a97..d8f525e 100644 (file)
@@ -7985,8 +7985,10 @@ sub email_search_result {
   my $subject = delete $params->{subject};
   my $html_body = delete $params->{html_body};
   my $text_body = delete $params->{text_body};
   my $subject = delete $params->{subject};
   my $html_body = delete $params->{html_body};
   my $text_body = delete $params->{text_body};
+  my $error = '';
 
 
-  my $job = delete $params->{'job'};
+  my $job = delete $params->{'job'}
+    or die "email_search_result must run from the job queue.\n";
 
   $params->{'payby'} = [ split(/\0/, $params->{'payby'}) ]
     unless ref($params->{'payby'});
 
   $params->{'payby'} = [ split(/\0/, $params->{'payby'}) ]
     unless ref($params->{'payby'});
@@ -8006,35 +8008,68 @@ sub email_search_result {
 
 
   my( $num, $last, $min_sec ) = (0, time, 5); #progresbar foo
 
 
   my( $num, $last, $min_sec ) = (0, time, 5); #progresbar foo
+  my @retry_jobs = ();
+  my $success = 0;
 
   #eventually order+limit magic to reduce memory use?
   foreach my $cust_main ( qsearch($sql_query) ) {
 
 
   #eventually order+limit magic to reduce memory use?
   foreach my $cust_main ( qsearch($sql_query) ) {
 
+    #progressbar first, so that the count is right
+    $num++;
+    if ( time - $min_sec > $last ) {
+      my $error = $job->update_statustext(
+        int( 100 * $num / $num_cust )
+      );
+      die $error if $error;
+      $last = time;
+    }
+
     my $to = $cust_main->invoicing_list_emailonly_scalar;
     my $to = $cust_main->invoicing_list_emailonly_scalar;
-    next unless $to;
 
 
-    my $error = send_email(
-      generate_email(
+    if( $to ) {
+      my @message = (
         'from'      => $from,
         'to'        => $to,
         'subject'   => $subject,
         'html_body' => $html_body,
         'text_body' => $text_body,
         'from'      => $from,
         'to'        => $to,
         'subject'   => $subject,
         'html_body' => $html_body,
         'text_body' => $text_body,
-      )
-    );
-    return $error if $error;
+      );
 
 
-    if ( $job ) { #progressbar foo
-      $num++;
-      if ( time - $min_sec > $last ) {
-        my $error = $job->update_statustext(
-          int( 100 * $num / $num_cust )
-        );
-        die $error if $error;
-        $last = time;
+      $error = send_email( generate_email( @message ) );
+
+      if($error) {
+        # queue the sending of this message so that the user can see what we 
+        # tried to do, and retry if desired
+        my $queue = new FS::queue {
+          'job'        => 'FS::Misc::process_send_email',
+          'custnum'    => $cust_main->custnum,
+          'status'     => 'failed',
+          'statustext' => $error,
+        };
+        $queue->insert(@message);
+        push @retry_jobs, $queue;
+      }
+      else {
+        $success++;
       }
     }
 
       }
     }
 
+    if($success == 0 and 
+        (scalar(@retry_jobs) > 10 or $num == $num_cust)
+      ) {
+      # 10 is arbitrary, but if we have enough failures, that's 
+      # probably a configuration or network problem, and we 
+      # abort the batch and run away screaming.
+      # We NEVER do this if anything was successfully sent.
+      $_->delete foreach (@retry_jobs);
+      return "multiple failures: '$error'\n";
+    }
+  }
+
+  if(@retry_jobs) {
+    # fail the job, but with a status message that makes it clear
+    # something was sent.
+    return "Sent $success, failed ".scalar(@retry_jobs).". Failed attempts placed in job queue.\n";
   }
 
   return '';
   }
 
   return '';