proexchange / com.pesc.sparkpost

Integrates SparkPost to CiviCRM, so email can be sent out over the SparkPost service and bounces can be processed in CiviCRM
Other
9 stars 5 forks source link

Fatal error on invalid email address when sending scheduled reminders #27

Open lolaslade opened 7 years ago

lolaslade commented 7 years ago

Contacts with invalid emails like abc@xyzmail (missing the .com TLD for example) are triggering a fatal error when using this extension. Then the rest of the scheduled reminders are getting skipped. The problem is that line 220 of sparkpost.php an empty array is being returned for contactID and this is not being checked. Then on line 260 this call fails with a "DB Error: constraint violation" due to the null contactID.

$eventQueue = CRM_Mailing_Event_BAO_Queue::create($queueParams);

CiviCRM Core by comparison would attempt to send the mail but has temporary error handler set in order to ignore errors. I believe it doesn't log the failed reminder anywhere. (Looking at lines 279-280 of CRM/Utils/Mail.php: $errorScope = CRM_Core_TemporaryErrorScope::ignoreException(); $result = $mailer->send($to, $headers, $message); I propose a quick fix for now that simply checks for a null contact ID and skips the remaining code to create the mailing queue. Then Civi Core will silently fail on the email address as usual.

joseltorres commented 7 years ago

Thanks, we will fix soon.

davejenx commented 6 years ago

I found that this issue can happen with valid email addresses too, e.g. for domains under the .space top level domain. The regex in sparkpost_getTargetContactId assumes that top level domains are 2 to 4 characters but see https://en.wikipedia.org/wiki/List_of_Internet_top-level_domains#Brand_top-level_domains .

Example of foreign constraint error that results:

INSERT INTO civicrm_mailing_event_queue (job_id , email_id , hash ) VALUES ( 1649 ,  67592 , '22fd973bd7091b85' )
  [nativecode=1452 ** Cannot add or update a child row: a foreign key constraint fails (`civicrm`.`civicrm_mailing_event_queue`, CONSTRAINT `FK_civicrm_mailing_event_queue_contact_id` FOREIGN KEY (`contact_id`) REFERENCES `civicrm_contact` (`id`) ON DELETE CASCADE)]

One option could be to use filter_var() rather than a regex. Agree with @lolaslade that in any case, error trapping is needed.

davyrhys commented 3 years ago

I found that this issue can happen with valid email addresses too, e.g. for domains under the .space top level domain. The regex in sparkpost_getTargetContactId assumes that top level domains are 2 to 4 characters but see https://en.wikipedia.org/wiki/List_of_Internet_top-level_domains#Brand_top-level_domains .

Example of foreign constraint error that results:

INSERT INTO civicrm_mailing_event_queue (job_id , email_id , hash ) VALUES ( 1649 ,  67592 , '22fd973bd7091b85' )
  [nativecode=1452 ** Cannot add or update a child row: a foreign key constraint fails (`civicrm`.`civicrm_mailing_event_queue`, CONSTRAINT `FK_civicrm_mailing_event_queue_contact_id` FOREIGN KEY (`contact_id`) REFERENCES `civicrm_contact` (`id`) ON DELETE CASCADE)]

One option could be to use filter_var() rather than a regex. Agree with @lolaslade that in any case, error trapping is needed.

Thanks for this post - I was experiencing this problem with CiviEvent registrations and reminders failing. I'm not a developer but this helped me fix the problem by:

  1. installing the latest code (not the latest release which is older)
  2. editing sparkpost.php by replacing the two instances of the regex pregmatch('/[A-Z0-9.%+-]+@[A-Z0-9.-]+.[A-Z]{2,4}\b/i', $email, $matches); - I simply changed from 2,4 to 2,20

Next step is to search for the offending email address.