At 2017-05-22 17:26:41, "Ævar Arnfjörð Bjarmason" <avarab@xxxxxxxxx> wrote: >On Sun, May 21, 2017 at 2:59 PM, xiaoqiang zhao <zxq_yx_007@xxxxxxx> wrote: >> Some email servers (e.g. smtp.163.com) limit the number emails to be >> sent per session(connection) and this will lead to a faliure when >> sending many messages. > >This OK to me, the nits I had are addressed by Junio's reply. > >Looking at this the Nth time now though I wonder about this approach >in general. In all your E-Mails I don't think you ever said /what/ >sort of error you had from the SMTP server, you just said you had a >failure or an error, I assume you hit one of the die's in the >send_message() function. Can you paste the actual error you get >without this patch? > When I send a patch series which has 13 (plus cover) messages as a test, I got errors as follows and send-email quit when sending the 11th message: MI:DMC 163 smtp14,EsCowAD3o71TKyRZTBlJHw--.20496S13 1495542613 http://mail.163.com/help/help_spam_16.htm?ip=1.203.183.150&hostid=smtp14&time=1495542613 Follow the link above, I find two error code: •450 MI:DMC 当前连接发送的邮件数量超出限制。请减少每次连接中投递的邮件数量 •451 MI:DMC 当前连接发送的邮件数量超出限制。请控制每次连接中投递的邮件数量 Translate into English: •450 MI:DMC The number of messages sent execeeds the limits. Please reduce the number of messages to be sent per connection. •451 MI:DMC The number of messages sent execeeds the limits. Please control the number of messages to be sent per connection. Although has different error code, but says similar reason. Testing with --smtp-debug option produce the same error. >I wonder if something like this would Just Work for this case without >any configuration or command-line options, with the added benefit of >just working for anyone with transitory SMTP issues as well (patch >posted with -w, full version at >https://github.com/avar/git/commit/acb60c4bde50bdcb62b71ed46f49617e2caef84e.patch): > >diff --git a/git-send-email.perl b/git-send-email.perl >index 8a1ee0f0d4..c2d85236d1 100755 >--- a/git-send-email.perl >+++ b/git-send-email.perl >@@ -1363,6 +1363,10 @@ EOF > die __("The required SMTP server is not >properly defined.") > } > >+ my $num_tries = 0; >+ my $max_tries = 5; >+ smtp_again: >+ eval { > if ($smtp_encryption eq 'ssl') { > $smtp_server_port ||= 465; # ssmtp > require Net::SMTP::SSL; >@@ -1429,6 +1433,22 @@ EOF > } > $smtp->dataend() or die $smtp->message; > $smtp->code =~ /250|200/ or die >sprintf(__("Failed to send %s\n"), $subject).$smtp->message; >+ 1; >+ } or do { >+ my $error = $@ || "Zombie Error"; >+ >+ warn sprintf(__("Failed to send %s due to >error: %s"), $subject, $error); >+ if ($num_tries++ < $max_tries) { >+ $smtp->quit if defined $smtp; >+ $smtp = undef; >+ $auth = undef; >+ my $sleep = $num_tries * 3; # 3, 6, 9, ... >+ warn sprintf(__("This is retry %d/%d. >Sleeping %d before trying again"), >+ $num_tries, $max_tries, $sleep); >+ sleep($sleep); >+ goto smtp_again; >+ } >+ }; > } > if ($quiet) { > printf($dry_run ? __("Dry-Sent %s\n") : __("Sent >%s\n"), $subject); > >Now that's very much a WIP and I don't have a server like that to test against. > >Having worked with SMTP a lot in a past life/job, I'd say it's *very* >likely that you're just getting a /^4/ error code from 163.com, >probably 421, which would make this logic even simpler. I.e. we could >just adjust this to back-off for /^4/ instead of trying to handle >arbitrary errors. > >Anyway, I'm not interested in pursuing that WIP patch, and I don't >think perfect should be the enemy of the good here. Your patch works >for you, doesn't really damage anything else, so if you're not >interested in hacking up something like the above I think we should >just take it. > >But I do think it would be very good to get a reply to you / details >in the commit message about what error you get exactly in this >scenario, see if you get better details with --smtp-debug, and if so >paste that (sans any secret info like user/password you don't want to >share). > >Then if we're poking at this code in the future we can maybe just fix >this in some more general fashion while keeping this use-case in mind.