Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Janeway notifications result in system error, don't send #3545

Open
alainna opened this issue May 12, 2023 · 8 comments
Open

Janeway notifications result in system error, don't send #3545

alainna opened this issue May 12, 2023 · 8 comments
Labels
bug Something's not working

Comments

@alainna
Copy link
Contributor

alainna commented May 12, 2023

Describe the bug
No reply-to is being set by the caller of send_email

Janeway version
5.1.0 RC3

To Reproduce
Steps to reproduce the behavior:

  1. Submit an article for a journal
  2. After final submission step, observe server error
  3. Submit an article to a repository
  4. After final submission step, observe server error

Expected behavior
Email would successfully send.

Logs
After adding logging to notify_email.py:

    # if a replyto is passed to this function, use that.
    if replyto:
        reply_to = replyto

    # if there is no reply_to set yet, check if the journal has a custom replyto_address and
    # use that.
    if not reply_to:
        logger.info("no reply-to provided, attempting to get a journal reply-to...")
        custom_reply_to = setting_handler.get_setting(
            'general', 'replyto_address', journal,
        ).value
        if custom_reply_to:
            logger.info("found a custom_reply_to, using: %s" % custom_reply_to)
            reply_to = (custom_reply_to,)

    if not reply_to:
        logger.info("no reply-to set, that's concerning")
    else:
        logger.info("preparing to send message, reply_to='%s'" % reply_to)

    # reply_to must always be a tuple or list.
    if reply_to and not isinstance(reply_to, (tuple, list)):
        reply_to = [reply_to]

    # log the message we're about to send
    logger.info("sending message with reply_to=%s" % reply_to)

which yields this output when Janeway tries to send a submission confirmation message from a journal (demo-dev) and repository (ea-dev):

INFO 2023-05-12 09:59:16,950 notify_email P:15509 T:140570450089728 [demo-dev] no reply-to provided, attempting to get a journal reply-to...
INFO 2023-05-12 09:59:16,956 notify_email P:15509 T:140570450089728 [demo-dev] no reply-to set, that's concerning
INFO 2023-05-12 09:59:16,957 notify_email P:15509 T:140570450089728 [demo-dev] sending message with reply_to=[]
INFO 2023-05-12 09:59:17,720 notify_email P:15505 T:140570550802176 [EA-DEV] no reply-to provided, attempting to get a journal reply-to...
INFO 2023-05-12 09:59:17,725 notify_email P:15505 T:140570550802176 [EA-DEV] no reply-to set, that's concerning
INFO 2023-05-12 09:59:17,725 notify_email P:15505 T:140570550802176 [EA-DEV] sending message with reply_to=[]
@alainna alainna added the bug Something's not working label May 12, 2023
@hardyoyo
Copy link
Contributor

In further investigation, I added logging for all the e-mail addresses used in building a notification e-mail, and noticed that the from address isn't a string, it's a tuple. Which is presumably the result of this line:

    # This call is ported from django 1.11
    full_from_string = parseaddr(force_str(full_from_string))
    ```
    
    I commented out that line of code, and our notifications were again being sent correctly.

@mauromsl
Copy link
Member

A tuple in the format (name, address) is a valid input for the form_address value passed to msg.send() as this is an expected value for django's sanitizer: https://github.com/django/django/blob/e1bbbbe6acb69e755554088bc573cc1835673209/django/core/mail/message.py#L76

This suggests one of two potential problems: either Django's sanitization is not being invoked by your configured EMAIL_BACKEND or an address is not being parsed correctly by parseaddr

@hardyoyo can you confirm what from address is actually being passed to the SMTP server as well as confirm what email backend is configured on the faulty server?

@hardyoyo
Copy link
Contributor

sure, with the line commented out, here's what from is set to:
INFO 2023-05-15 11:47:27,003 notify_email P:7835 T:140125014886144 [EA-DEV] from=Alainna T W [email protected]

with the line in place, it's:
from=('Alainna T W', '[email protected]')

The string sends an e-mail without error, the tuple does not.

The EMAIL_BACKEND is set to:
EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'

@mauromsl
Copy link
Member

mauromsl commented May 16, 2023

sure, with the line commented out, here's what from is set to: INFO 2023-05-15 11:47:27,003 notify_email P:7835 T:140125014886144 [EA-DEV] from=Alainna T W [email protected]

with the line in place, it's: from=('Alainna T W', '[email protected]')

The string sends an e-mail without error, the tuple does not.

The EMAIL_BACKEND is set to: EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'

Thank you @hardyoyo.

The line you logged is from Janeway's notify_email function, at which point the from_address needs to be a a tuple of (common name, address) so that the Django's SMTP library can sanitize and format the value into the correct format of display-name <local-name@domain>. In fact, when you set that value to a string, Django is internally casting it into a tuple automatically (but with an empty name) https://github.com/django/django/blob/e1bbbbe6acb69e755554088bc573cc1835673209/django/core/mail/message.py#L80

What we need to see is what the SMTP server itself is receiving from the application, so we can determine where the problem lies.

My guess would be that the display-name is being sanitized and formatted with Django 3.2 in such a way that is not compatible with your SMTP server.

@hardyoyo
Copy link
Contributor

I was able to get the full text of the email message by enabling the file email backend. Here's the message that fails:

Content-Type: multipart/alternative;
 boundary="===============0622671085063671821=="
MIME-Version: 1.0
Subject: [demo-dev] Article Submission
From: ('', '[email protected]')
To: [email protected]
Reply-To: [email protected]
Date: Tue, 16 May 2023 19:57:05 -0000
Message-ID:
 <168426702515.20067.9200987368572112795@pub-janeway2-dev.escholarship.org>

--===============0622671085063671821==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

Dear Hardy Pottinger, Thank you for submitting "29387 - test" to demo-dev. Your work will now be reviewed by an editor and we will be in touch as the peer-review process progresses.Regards,Dev Demo Journal
--===============0622671085063671821==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

Dear Hardy Pottinger, <br><br>Thank you for submitting "29387 - test" to demo-dev.<br><br> Your work will now be reviewed by an editor and we will be in touch as the peer-review process progresses.<br><br>Regards,<br><br />Dev Demo Journal
--===============0622671085063671821==--

-------------------------------------------------------------------------------

@hardyoyo
Copy link
Contributor

I suspect that the culprit here is on this line https://github.com/BirkbeckCTP/janeway/blob/master/src/utils/notify_plugins/notify_email.py#L24

I think setting_handler.get_setting('general', 'from_address', journal).value returns a string, not an e-mail tuple.

@hardyoyo
Copy link
Contributor

hardyoyo commented May 16, 2023

I'm wrong... I created a tuple instead of a string on line 24, and got another error SMTP error.

ValueError: Invalid address " <('Dev Demo Journal', '[[email protected]](mailto:[email protected])')>"

There's definitely a bug here, where a tuple is getting sent to SMTP as if it's a string. I'll be darned if I can find it, though.

This is a blocker for our continued testing and deployment of Janeway 1.5.x ... we require a working e-mail notification system.

@ajrbyers
Copy link
Member

@mauromsl can we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's not working
Projects
None yet
Development

No branches or pull requests

4 participants