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

[smtplib] collect response data for all recipients #73725

Open
FirefighterBlu3 mannequin opened this issue Feb 13, 2017 · 9 comments
Open

[smtplib] collect response data for all recipients #73725

FirefighterBlu3 mannequin opened this issue Feb 13, 2017 · 9 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir topic-email type-feature A feature request or enhancement

Comments

@FirefighterBlu3
Copy link
Mannequin

FirefighterBlu3 mannequin commented Feb 13, 2017

BPO 29539
Nosy @warsaw, @bitdancer, @FirefighterBlu3, @Windsooon, @sls89
PRs
  • bpo-29539: Implementation to return all mta status codes #12148
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2017-02-13.00:45:25.541>
    labels = ['type-feature', '3.8', 'expert-email']
    title = '[smtplib] collect response data for all recipients'
    updated_at = <Date 2019-03-03.18:12:51.111>
    user = 'https://github.com/FirefighterBlu3'

    bugs.python.org fields:

    activity = <Date 2019-03-03.18:12:51.111>
    actor = 'sls'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['email']
    creation = <Date 2017-02-13.00:45:25.541>
    creator = 'David Ford (FirefighterBlu3)'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29539
    keywords = ['patch']
    message_count = 9.0
    messages = ['287662', '318518', '318706', '318723', '336876', '336886', '336894', '336960', '337040']
    nosy_count = 5.0
    nosy_names = ['barry', 'r.david.murray', 'David Ford (FirefighterBlu3)', 'Windson Yang', 'sls']
    pr_nums = ['12148']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29539'
    versions = ['Python 3.8']

    @FirefighterBlu3
    Copy link
    Mannequin Author

    FirefighterBlu3 mannequin commented Feb 13, 2017

    Feature request; collect SMTP response results for all recipients so data likely including the queue ID or SMTP delay expectation can be collected

    I propose the keyword "keep_results=False" be added to smtplib.sendmail() to accomplish this. The default value of False maintains the legacy functionality of prior versions. No other changes have been done to smtplib.send_message() pending discussion of the following.

    @@ -785,7 +785,7 @@
             return (resp, reply)
     
         def sendmail(self, from_addr, to_addrs, msg, mail_options=[],
    -                 rcpt_options=[]):
    +                 rcpt_options=[], keep_results=False):
             """This command performs an entire mail transaction.
     
             The arguments are:
    @@ -797,6 +797,8 @@
                                  mail command.
                 - rcpt_options : List of ESMTP options (such as DSN commands) for
                                  all the rcpt commands.
    +            - keep_results : If True, return a dictionary of recipients and the
    +                             SMTP result for each.
     
             msg may be a string containing characters in the ASCII range, or a byte
             string.  A string is encoded to bytes using the ascii codec, and lone
    @@ -807,17 +809,20 @@
             and each of the specified options will be passed to it.  If EHLO
             fails, HELO will be tried and ESMTP options suppressed.
     
    -        This method will return normally if the mail is accepted for at least
    -        one recipient.  It returns a dictionary, with one entry for each
    -        recipient that was refused.  Each entry contains a tuple of the SMTP
    -        error code and the accompanying error message sent by the server.
    +        If keep_results is False, this method will return normally if the mail
    +        is accepted for at least one recipient.  It returns a dictionary, with
    +        one entry for each recipient that was refused.  Each entry contains a
    +        tuple of the SMTP error code and the accompanying error message sent by
    +        the server.  If keep_results is True, this method returns a dictionary
    +        of all recipients and the SMTP result whether refused or accepted
    +        unless all are refused then the normal exception is raised.
     
             This method may raise the following exceptions:
     
              SMTPHeloError          The server didn't reply properly to
                                     the helo greeting.
    -         SMTPRecipientsRefused  The server rejected ALL recipients
    -                                (no mail was sent).
    +         SMTPRecipientsRefused  The server rejected ALL recipients (no mail
    +                                was sent).
              SMTPSenderRefused      The server didn't accept the from_addr.
              SMTPDataError          The server replied with an unexpected
                                     error code (other than a refusal of
    @@ -879,6 +884,10 @@
                 self._rset()
                 raise SMTPRecipientsRefused(senderrs)
             (code, resp) = self.data(msg)
    +        if keep_results:
    +            for each in to_addrs:
    +                if not each in senderrs:
    +                    senderrs[each]=(code, resp)
             if code != 250:
                 if code == 421:
                     self.close()

    @FirefighterBlu3 FirefighterBlu3 mannequin added 3.7 (EOL) end of life topic-email type-feature A feature request or enhancement labels Feb 13, 2017
    @FirefighterBlu3
    Copy link
    Mannequin Author

    FirefighterBlu3 mannequin commented Jun 3, 2018

    3.7 release candidates are in the queue, any thoughts on this simple enhancement?

    @warsaw
    Copy link
    Member

    warsaw commented Jun 4, 2018

    It's too late for 3.7, but something like this could be an interesting enhancement for 3.8. I'm not so sure about the name of the suggested parameter since it seems more about recording successful deliveries in addition to the normally failed deliveries. But we can quibble about that later.

    Would you be willing and able to turn your patch into a pull request against our GitHub repo? You would need to sign the CLA. We're also going to want a test, some additional documentation, and a news entry.

    See devguide.python.org for details.

    @warsaw warsaw added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Jun 4, 2018
    @FirefighterBlu3
    Copy link
    Mannequin Author

    FirefighterBlu3 mannequin commented Jun 5, 2018

    Yes, it is distinctly intended to collect the results for every recipient as in modern MTAs, there's more than just success/fail results. This is to collect data such as queue ID and the MTA's programmatic response for each individual recipient. I have several needs of knowing if the message was immediately relayed, queued for later because of a remote issue, queued because of MTA issue, graylisted or blocked because of milter reasons, or ... any of a list of failure reasons.

    This data can be collected if there is only one recipient per message, but that is considerably resource expensive.

    Without this patch, when sending to say 100 recipients, the only response returned to the caller is the very last (100th) result. A 250 then assumes that all 100 were transmitted successfully when in truth, the first 99 could have failed.

    Yes, I'll make a PR, do the CLA, and add some tests.

    @sls89
    Copy link
    Mannequin

    sls89 mannequin commented Feb 28, 2019

    Did you make a PR ? I didn't notice this issue and did a quick fix: #12104

    I was hoping this or similar fixes would be implemented in 3.7.x

    Anyways. I'd suggest to return both success and errors for each recipient. sendmail returns a dict already. Why not adding each status?

    Maybe using something like "mta_result" as variable instead of senderrs, which is in my opinion more clear to understand. Also, this would have the advantage to parse the return value easily and you're able to access more verbose information like mta msg id and so on.

    @Windsooon
    Copy link
    Mannequin

    Windsooon mannequin commented Mar 1, 2019

    sls, are you working on this feature now?

    @FirefighterBlu3
    Copy link
    Mannequin Author

    FirefighterBlu3 mannequin commented Mar 1, 2019

    i have a fully built patch and personally tested (i use it 24/7) but
    haven't done test_* yet as was requested

    On Thu, Feb 28, 2019 at 9:16 PM Windson Yang <report@bugs.python.org> wrote:

    Windson Yang <wiwindson@outlook.com> added the comment:

    sls, are you working on this feature now?

    ----------
    nosy: +Windson Yang


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue29539\>


    @sls89
    Copy link
    Mannequin

    sls89 mannequin commented Mar 1, 2019

    I closed my PR. I'd just rename "senderrs" to "mta_status" or so as aforementioned change means not just storing errors.

    FirefighterBlu3, do you open a PR for this? I'd like to see this moving into 3.8 as we don't compile Python from source but using it from deb repos.

    @sls89
    Copy link
    Mannequin

    sls89 mannequin commented Mar 3, 2019

    I opened a new PR.

    I picked up some of FirefighterBlu3's suggestions and added some unittests and refactoring to assist. Hope this helps.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 23, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes stdlib Python modules in the Lib dir topic-email type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants