AttributeError: 'SendCampaignThread' object has no attribute 'campaign'. Did you mean: 'campaign_pk'?

[danhayden]

Got a smtplib.SMTPRecipientsRefused exception (due to hitting email rate limit) when sending email using birdsong and another exception occurred

During handling of the above exception, another exception occurred:                                                                                                                                                                         

Traceback (most recent call last):                                                                                                                                                                                                          
  File "/usr/local/lib/python3.10/threading.py", line 1016, in _bootstrap_inner                                                                                                                                                             
    self.run()                                                                                                                                                                                                                              
  File ".../birdsong/backends/smtp.py", line 40, in run                                                                                                                                                                 
    self.campaign.status = CampaignStatus.FAILED                                                                                                                                                                                            
AttributeError: 'SendCampaignThread' object has no attribute 'campaign'. Did you mean: 'campaign_pk'?  

(smtp.py#L40)

[danhayden]

Just hit this error again. Would it resolve the issue by changing smtp.py#L40 from

self.campaign.status = CampaignStatus.FAILED

to:

Campaign.objects.filter(pk=self.campaign_pk).update(
    status=CampaignStatus.FAILED,
)

[seb-b]

That looks like the likely culprit. I think before it was a thread it was a just a property on a class and this is just a hangover from that

[danhayden]

Thanks for response Seb, I'd like to create a PR to address this issue.

Taking a look at the run method (https://github.com/neon-jungle/wagtail-birdsong/blob/master/birdsong/backends/smtp.py#L24) on the SendCampaignThread class would it make sense to get the campaign object at the beginning to use in the try/except blocks. Also is there any reason to to use Campaign.objects.filter(pk=self.campaign_pk) rather than Campaign.objects.get(pk=self.campaign_pk)? both are used but not sure there is any benefit or simply an other hangover from the previous code.

Simply put, here is the change I am suggesting:

def run(self):
    campaign = Campaign.objects.get(pk=self.campaign_pk)
    try:
        logger.info(f"Sending {len(self.messages)} emails")
        send_mass_html_mail(self.messages)
        logger.info("Emails finished sending")
        with transaction.atomic():
            campaign.update(status=CampaignStatus.SENT, sent_date=timezone.now())
            fresh_contacts = Contact.objects.filter(pk__in=self.contact_pks)
            campaign.receipts.add(*fresh_contacts)
    except SMTPException:
        logger.exception(f"Problem sending campaign: {self.campaign_pk}")
        campaign.update(status=CampaignStatus.FAILED)
    finally:
        close_old_connections()

[seb-b]

yeah that looks sensible - couple of things

• filter(...) returns a queryset instead of an individual model, .update(...) is only available on querysets
• .get can raise an exception if the Campaign doesn't exist (unlikely, but possible)

Feel free to open a pull request and we can go over the details there 😄

[danhayden]

Thanks for the info Seb, I hadn't realised .update() only works on querysets. I have opened a draft PR for us to continue the discussion.