This is an excellent module, and I use it a lot.
Unfotunately it has a few shortcomings.

The most important issue is that it gets quite slow when sending a lof of messages (hundereds or eve thousands of mail).
This leads to random errors, as other database requests might time out and do all sorts of strange stuff.
I have a small patch (1) that moves the actual sending to a shutdown_function, this takes care of the main problem, but one issue still bothers my users.
The browser never outputs anything before all mails are sent, so there is no feedback to the sender, and it always makes thme wonder if things are ok. It also might lead to them clicking "stop" in the browser, and interrupting the sending process.

So the correct patch would probably be to move the actual sending to a hook_exit()-function, so the sending keeps running it the background after all output is sent to the browser.

Patch 1:


--- mail.module.orig    2005-03-05 14:01:05.000000000 +0100
+++ mail.module 2005-03-09 20:46:39.000000000 +0100
@@ -161,6 +161,7 @@
  * Send mail to specified users
  */
 function _mail_send($node, $main = 0) {
+  ignore_user_abort(TRUE);
   global $user, $base_url;
   $roles = $node->roles_selected;
   if(empty($roles)) {
@@ -186,11 +187,12 @@
     if($account->mail) {
       $variables = array('%username' => $account->name, '%site' => variable_get('site_name', 'drupal'), '%uri' => $base_url, '%uri_brief' => substr($base_url, strlen('http://')), '%mailto' => $account->mail, '%date' => format_date(time()), '%login_uri' => url('user/login', NULL, NULL, TRUE), '%edit_uri' => url('user/edit', NULL, NULL, TRUE));
       $message = strtr($node->body, $variables);
-      user_mail($account->mail, $node->title, $message, $headers);
+      register_shutdown_function('user_mail', $account->mail, $node->title, $message, $headers);
+      ++$num_mails;
     }
   }

-  drupal_set_message('Message "' . $node->title . '" sent to users with the following roles: ' . implode(', ', _mail_get_roles_names($roles)));
+  drupal_set_message('Message "' . $node->title . '" sent to ' . $num_mails . ' users with the following roles: ' . implode(', ', _mail_get_roles_names($roles)));
 }

 /**

Comments

Olen’s picture

Olen commented

A patch that implements hook_exit to do the actual sending of the emails.
There still seems to be a delay before the page returns, but hopefully it is better than other solutions.

--- mail.module.orig    2005-03-05 14:01:05.000000000 +0100
+++ mail.module 2005-03-13 16:50:00.361855041 +0100
@@ -151,6 +151,33 @@
 }

 /**
+ * Implementation of _exit hook
+ */
+function mail_exit() {
+  global $mail_sendqueue, $mail_message;
+  if (is_array($mail_sendqueue)) {
+    foreach ($mail_sendqueue as $qnum => $vals) {
+      $variables = array(
+        '%username' => $vals['name'],
+        '%site' => variable_get('site_name', 'drupal'),
+        '%uri' => $base_url,
+        '%uri_brief' => substr($base_url, strlen('http://')),
+        '%mailto' => $vals['mail'],
+        '%date' => format_date(time()),
+        '%login_uri' => url('user/login', NULL, NULL, TRUE),
+        '%edit_uri' => url('user/edit', NULL, NULL, TRUE)
+      );
+      $message = strtr($mail_message['body'], $variables);
+      user_mail($vals['mail'], $mail_message['subject'], $message, $mail_message['headers']);
+
+    }
+  }
+  unset($mail_sendqueue);
+  unset($mail_message);
+}
+
+
+/**
  * Prepare a node's body content for viewing
  */
 function mail_content($node, $main = 0) {
@@ -161,7 +188,7 @@
  * Send mail to specified users
  */
 function _mail_send($node, $main = 0) {
-  global $user, $base_url;
+  global $user, $base_url, $mail_message, $mail_sendqueue;
   $roles = $node->roles_selected;
   if(empty($roles)) {
     $roles = array();
@@ -173,24 +200,27 @@
   (count($roles_where) > 0) ? $where = ' AND (' . implode(' OR ', $roles_where) . ') ': $where = '';
   if($user->uid == 1) {
     $from = variable_get('site_mail', ini_get('sendmail_from'));
-    $headers = "From: " . variable_get('site_name', 'drupal') . " <" . $from . ">\n";
-    $headers .= "Reply-To: " . $from . "\n";
+    $mail_message['headers'] = "From: " . variable_get('site_name', 'drupal') . " <" . $from . ">\n";
+    $mail_message['headers'] .= "Reply-To: " . $from . "\n";
   }

   else {
-    $headers = "From: " . $user->name . " at " . variable_get('site_name', 'drupal') . " <" . $user->mail . ">\n";
-    $headers .= "Reply-To: " . $user->mail . "\n";
+    $mail_message['headers'] = "From: " . $user->name . " at " . variable_get('site_name', 'drupal') . " <" . $user->mail . ">\n";
+    $mail_message['headers'] .= "Reply-To: " . $user->mail . "\n";
   }
   $result = db_query('SELECT DISTINCT(u.uid), u.name, u.mail FROM {users} u, {role} r, {users_roles} s WHERE u.uid = s.uid AND r.rid = s.rid AND u.status != 0' . $where);
   while ($account = db_fetch_object($result)) {
     if($account->mail) {
-      $variables = array('%username' => $account->name, '%site' => variable_get('site_name', 'drupal'), '%uri' => $base_url, '%uri_brief' => substr($base_url, strlen('http://')), '%mailto' => $account->mail, '%date' => format_date(time()), '%login_uri' => url('user/login', NULL, NULL, TRUE), '%edit_uri' => url('user/edit', NULL, NULL, TRUE));
-      $message = strtr($node->body, $variables);
-      user_mail($account->mail, $node->title, $message, $headers);
+      $mail_sendqueue[] = array(
+        'name' => $account->name,
+        'mail' => $account->mail,
+      );
+      ++$num_mails;
     }
   }
-
-  drupal_set_message('Message "' . $node->title . '" sent to users with the following roles: ' . implode(', ', _mail_get_roles_names($roles)));
+  $mail_message['body'] = $node->body;
+  $mail_message['subject'] = $node->title;
+  drupal_set_message('Message "' . $node->title . '" sent to ' . $num_mails . ' users with the following roles: ' . implode(', ', _mail_get_roles_names($roles)));
 }

 /**
nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Thanks very much for this. If you have CVS write access, could you apply the patch yourself please? If not I will do so when I get a chance, but I'm going to be on the road for the next two weeks so it won't be until after that.

Olen’s picture

Olen commented

Updated now.
Thanks, have a nice holiday

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Thanks.

Anonymous’s picture

(not verified) commented