[PATCH v5] send-email: --batch-size to work around some SMTP server limit

[PATCH v5] send-email: --batch-size to work around some SMTP server limit

[xiaoqiang zhao]

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.

Teach send-email to disconnect after sending a number of messages
(configurable via the --batch-size=<num> option), wait for a few
seconds (configurable via the --relogin-delay=<seconds> option) and
reconnect, to work around such a limit.

Also add this two configuration option.

Note:
   Re-authentication will happen every $<batch-size> messages, so it
will be much more acceptable if you use some form of credential helper
(e.g. the 'sendemail.smtppass' config option), otherwise you will have
to retype password every time when asked.

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 Documentation/config.txt               |  8 ++++++++
 Documentation/git-send-email.txt       | 11 +++++++++++
 contrib/completion/git-completion.bash |  2 ++
 git-send-email.perl                    | 18 ++++++++++++++++++
 4 files changed, 39 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 96e9cf8b7..173ed63f6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2917,6 +2917,14 @@ sendemail.xmailer::
 sendemail.signedoffcc (deprecated)::
 	Deprecated alias for `sendemail.signedoffbycc`.
 
+sendemail.smtpbatchsize::
+	Number of messages to be sent per connection, after that a relogin
+	will happen. if the value is 0 or undefined, send all messages in
+	one connection.
+
+sendemail.smtprelogindelay::
+	Seconds wait before reconnecting to smtp server.
+
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
 	See linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 9d66166f6..5380d8c95 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -248,6 +248,17 @@ must be used for each option.
 	commands and replies will be printed. Useful to debug TLS
 	connection and authentication problems.
 
+--batch-size=<num>::
+	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.  With this option, send-email will disconnect after
+	sending $<num> messages and wait for a few seconds (see --relogin-delay)
+	and reconnect, to work around such a limit.
+
+--relogin-delay=<int>::
+	Waiting $<int> seconds before reconnecting to smtp server. Used together
+	with --batch-size option.
+
 Automating
 ~~~~~~~~~~
 
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1ed0a09fe..933e7badf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2611,6 +2611,8 @@ _git_config ()
 		sendemail.thread
 		sendemail.to
 		sendemail.validate
+		sendemail.smtpbatchsize
+		sendemail.smtprelogindelay
 		showbranch.default
 		status.relativePaths
 		status.showUntrackedFiles
diff --git a/git-send-email.perl b/git-send-email.perl
index eea0a517f..8a1ee0f0d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -81,6 +81,10 @@ git send-email --dump-aliases
                                      This setting forces to use one of the listed mechanisms.
     --smtp-debug            <0|1>  * Disable, enable Net::SMTP debug.
 
+    --batch-size            <int>  * send max <int> message per connection.
+    --relogin-delay         <int>  * delay <int> seconds between two successive login.
+                                     This option can only be used with --batch-size
+
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
     --to-cmd                <str>  * Email To: via `<str> \$patch_path`
@@ -153,6 +157,7 @@ my $have_email_valid = eval { require Email::Valid; 1 };
 my $have_mail_address = eval { require Mail::Address; 1 };
 my $smtp;
 my $auth;
+my $num_sent = 0;
 
 # Regexes for RFC 2047 productions.
 my $re_token = qr/[^][()<>@,;:\\"\/?.= \000-\037\177-\377]+/;
@@ -216,6 +221,7 @@ my ($cover_cc, $cover_to);
 my ($to_cmd, $cc_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
+my ($batch_size, $relogin_delay);
 my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
 my ($validate, $confirm);
 my (@suppress_cc);
@@ -247,6 +253,8 @@ my %config_settings = (
     "smtppass" => \$smtp_authpass,
     "smtpdomain" => \$smtp_domain,
     "smtpauth" => \$smtp_auth,
+    "smtpbatchsize" => \$batch_size,
+    "smtprelogindelay" => \$relogin_delay,
     "to" => \@initial_to,
     "tocmd" => \$to_cmd,
     "cc" => \@initial_cc,
@@ -358,6 +366,8 @@ $rc = GetOptions(
 		    "force" => \$force,
 		    "xmailer!" => \$use_xmailer,
 		    "no-xmailer" => sub {$use_xmailer = 0},
+		    "batch-size=i" => \$batch_size,
+		    "relogin-delay=i" => \$relogin_delay,
 	 );
 
 usage() if $help;
@@ -1664,6 +1674,14 @@ foreach my $t (@files) {
 		}
 	}
 	$message_id = undef;
+	$num_sent++;
+	if (defined $batch_size && $num_sent == $batch_size) {
+		$num_sent = 0;
+		$smtp->quit if defined $smtp;
+		undef $smtp;
+		undef $auth;
+		sleep($relogin_delay) if defined $relogin_delay;
+	}
 }
 
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
-- 
2.13.0.rc2.116.g565bdd0fc.dirty

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[Junio C Hamano]

I think this is almost perfect.

I'd propose squashing the patch below to

 - Add cross reference between config and option

 - Spell configuration variables in camelCase to mimic other
   sendemail.* variables

 - Spell SMTP in all caps to mimic other parts of the manual

 - Suggest use of credential helper in the document to help the end
   users, not in the proposed log message.

If you are fine with all of these changes, there is no need to
resend (you can say so and I can locally squash these in).  Of
course, anyone is very welcome to point out documentation bugs I may
be introducing with this patch.

Thanks.

-- >8 --
Subject: fixup! send-email: --batch-size to work around some SMTP server limit

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.

Teach send-email to disconnect after sending a number of messages
(configurable via the --batch-size=<num> option), wait for a few
seconds (configurable via the --relogin-delay=<seconds> option) and
reconnect, to work around such a limit.

Also add two configuration variables to give these options the default.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt         |  8 +++++---
 Documentation/git-send-email.txt | 12 ++++++++----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a13315ed69..ee4a111878 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2914,13 +2914,15 @@ sendemail.xmailer::
 sendemail.signedoffcc (deprecated)::
 	Deprecated alias for `sendemail.signedoffbycc`.
 
-sendemail.smtpbatchsize::
+sendemail.smtpBatchSize::
 	Number of messages to be sent per connection, after that a relogin
-	will happen. if the value is 0 or undefined, send all messages in
+	will happen.  If the value is 0 or undefined, send all messages in
 	one connection.
+	See also the `--batch-size` option of linkgit:git-send-email[1].
 
-sendemail.smtprelogindelay::
+sendemail.smtpReloginDelay::
 	Seconds wait before reconnecting to smtp server.
+	See also the `--relogin-delay` option of linkgit:git-send-email[1].
 
 showbranch.default::
 	The default set of branches for linkgit:git-show-branch[1].
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 5380d8c956..79b418bfa5 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -250,14 +250,18 @@ must be used for each option.
 
 --batch-size=<num>::
 	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
+	sent per session (connection) and this will lead to a faliure when
 	sending many messages.  With this option, send-email will disconnect after
 	sending $<num> messages and wait for a few seconds (see --relogin-delay)
-	and reconnect, to work around such a limit.
+	and reconnect, to work around such a limit.  You may want to
+	use some form of credential helper to avoid having to retype
+	your password every time this happens.  Defaults to the
+	`sendemail.smtpBatchSize` configuration variable.
 
 --relogin-delay=<int>::
-	Waiting $<int> seconds before reconnecting to smtp server. Used together
-	with --batch-size option.
+	Waiting $<int> seconds before reconnecting to SMTP server. Used together
+	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
+	configuration variable.
 
 Automating
 ~~~~~~~~~~
-- 
2.13.0-440-g3ce6d2d5b8

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[Zhaoxiangqiang]

Junio C Hamano 于 2017 年 5 月 22 日 星期一 写道:
> I think this is almost perfect.
> 
> I'd propose squashing the patch below to
> 
>  - Add cross reference between config and option
> 
>  - Spell configuration variables in camelCase to mimic other
>    sendemail.* variables
> 
>  - Spell SMTP in all caps to mimic other parts of the manual
> 
>  - Suggest use of credential helper in the document to help the end
>    users, not in the proposed log message.
> 
> If you are fine with all of these changes, there is no need to
> resend (you can say so and I can locally squash these in).  Of
> course, anyone is very welcome to point out documentation bugs I may
> be introducing with this patch.
> 
> Thanks.
> 
> -- >8 --
> Subject: fixup! send-email: --batch-size to work around some SMTP server limit
> 
> 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.
> 
> Teach send-email to disconnect after sending a number of messages
> (configurable via the --batch-size=<num> option), wait for a few
> seconds (configurable via the --relogin-delay=<seconds> option) and
> reconnect, to work around such a limit.
> 
> Also add two configuration variables to give these options the default.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/config.txt         |  8 +++++---
>  Documentation/git-send-email.txt | 12 ++++++++----
>  2 files changed, 13 insertions(+), 7 deletions(-)
>

It's fine with me, just  go ahead.
Thank you  Hamano!
 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index a13315ed69..ee4a111878 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2914,13 +2914,15 @@ sendemail.xmailer::
>  sendemail.signedoffcc (deprecated)::
>  	Deprecated alias for `sendemail.signedoffbycc`.
>  
> -sendemail.smtpbatchsize::
> +sendemail.smtpBatchSize::
>  	Number of messages to be sent per connection, after that a relogin
> -	will happen. if the value is 0 or undefined, send all messages in
> +	will happen.  If the value is 0 or undefined, send all messages in
>  	one connection.
> +	See also the `--batch-size` option of linkgit:git-send-email[1].
>  
> -sendemail.smtprelogindelay::
> +sendemail.smtpReloginDelay::
>  	Seconds wait before reconnecting to smtp server.
> +	See also the `--relogin-delay` option of linkgit:git-send-email[1].
>  
>  showbranch.default::
>  	The default set of branches for linkgit:git-show-branch[1].
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 5380d8c956..79b418bfa5 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -250,14 +250,18 @@ must be used for each option.
>  
>  --batch-size=<num>::
>  	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
> +	sent per session (connection) and this will lead to a faliure when
>  	sending many messages.  With this option, send-email will disconnect after
>  	sending $<num> messages and wait for a few seconds (see --relogin-delay)
> -	and reconnect, to work around such a limit.
> +	and reconnect, to work around such a limit.  You may want to
> +	use some form of credential helper to avoid having to retype
> +	your password every time this happens.  Defaults to the
> +	`sendemail.smtpBatchSize` configuration variable.
>  
>  --relogin-delay=<int>::
> -	Waiting $<int> seconds before reconnecting to smtp server. Used together
> -	with --batch-size option.
> +	Waiting $<int> seconds before reconnecting to SMTP server. Used together
> +	with --batch-size option.  Defaults to the `sendemail.smtpReloginDelay`
> +	configuration variable.
>  
>  Automating
>  ~~~~~~~~~~
> -- 
> 2.13.0-440-g3ce6d2d5b8
> 
>

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[Ævar Arnfjörð Bjarmason]

On Sun, May 21, 2017 at 2:59 PM, xiaoqiang zhao <zxq_yx_007@163.com> 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?

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.

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> 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?
>
> 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):

Yeah, if the issues users of 163.com are having can be resolved with
a more general approach like this, that would be very much preferred.

> 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).

Let's wait for a few days to see if xiaoqiang wants to take your
outline of more general approach and polish it.  I do prefer the "no
config" solution as xiaoqiang won't be the only 163.com user, but
Individual Contributors cannot be forced, so ...

Thanks.

Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[Jan Viktorin]

On Tue, 23 May 2017 16:46:27 +0900
Junio C Hamano <gitster@pobox.com> wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > 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?

Hello,

I have issues with a company SMTP server that returns:

Net::SMTP::SSL=GLOB(0x20d6510)<<< 451 4.3.0 Please try again later,
rate limited.
4.3.0 Please try again later, rate limited.

Unfortunately, I didn't find out the exact properties of the limit yet.
It seems that sending more then 10 patches at once fails. Thus, I have
to send longer patch sets in 2 rounds:

1. normal git send-email
2. git send-email --no-thread --in-reply-to="<COVER LETTER ID>" \
     <REST-OF-PATCHES>...

It is not exactly the same as sending all the patches at once.

The xiaoqiang's solution sounds promising to me. However, probably a
more general solution would be to "just" enable sending a whole patch
set in 2 rounds manually. But I didn't find any way how to do it right.

Regards
Jan

> >
> > 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):  
> 
> Yeah, if the issues users of 163.com are having can be resolved with
> a more general approach like this, that would be very much preferred.
> 
> > 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).  
> 
> Let's wait for a few days to see if xiaoqiang wants to take your
> outline of more general approach and polish it.  I do prefer the "no
> config" solution as xiaoqiang won't be the only 163.com user, but
> Individual Contributors cannot be forced, so ...
> 
> Thanks.

Re:Re: [PATCH v5] send-email: --batch-size to work around some SMTP server limit

[赵小强]

At 2017-05-22 17:26:41, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> wrote:
>On Sun, May 21, 2017 at 2:59 PM, xiaoqiang zhao <zxq_yx_007@163.com> 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.

[PATCH] send-email: have default batch size when relogin delay is given

[Stefan Beller]

When the batch size is neither configured nor given on the command
line, but the relogin delay is given, then the user is not using the
the feature as intended. But as the user gave a relogin delay, there is
clearly the intention to delay sending out emails. Assume a batch size
of 1 instead of silently ignoring the given relogin delay.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..5672e05b98 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,12 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+if (defined $relogin_delay) {
+	if (not defined $batch_size) {
+		$batch_size = 1;
+	}
+}
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.15.1.433.g936d1b9894.dirty

Re: [PATCH] send-email: have default batch size when relogin delay is given

[Eric Sunshine]

On Wed, Feb 7, 2018 at 2:45 PM, Stefan Beller <sbeller@google.com> wrote:
> When the batch size is neither configured nor given on the command
> line, but the relogin delay is given, then the user is not using the
> the feature as intended. But as the user gave a relogin delay, there is
> clearly the intention to delay sending out emails. Assume a batch size
> of 1 instead of silently ignoring the given relogin delay.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,12 @@ unless ($rc) {
> +if (defined $relogin_delay) {
> +       if (not defined $batch_size) {
> +               $batch_size = 1;
> +       }
> +}

Maybe also print a message that this batch size has been used as
default lest the user wonder why it's sending "slowly" without
apparently batching anything.

Alternately, complain and die if both options are not specified.

[PATCH] send-email: error out when relogin delay is missing

[Stefan Beller]

When the batch size is neither configured nor given on the command
line, but the relogin delay is given, then the current code ignores
the relogin delay setting.

This is unsafe as there was some intention when setting the batch size.
One workaround would be to just assume a batch size of 1 as a default.
This however may be bad UX, as then the user may wonder why it is sending
slowly without apparent batching.

Error out for now instead of potentially confusing the user.
As 5453b83bdf (send-email: --batch-size to work around some SMTP
server limit, 2017-05-21) lays out, we rather want to not have this
interface anyway and would rather want to react on the server throttling
dynamically.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..bc0d3ade16 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,9 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+die __("When a batch size is given, the relogin delay must be set\n")
+	if defined $relogin_delay and not defined $batch_size;
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.16.0.rc1.238.g530d649a79-goog

Re: [PATCH] send-email: error out when relogin delay is missing

[xiaoqiang zhao]

> 在 2018年2月8日，上午7:43，Stefan Beller <sbeller@google.com> 写道：
> 
> +die __("When a batch size is given, the relogin delay must be set\n")
> +    if defined $relogin_delay and not defined $batch_size;
> +
 
According the code, maybe you want to say “When relogin delay is given, a batch size must be set “ ?

Re: [PATCH] send-email: error out when relogin delay is missing

[Eric Sunshine]

On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
> [...]
> Error out for now instead of potentially confusing the user.
> As 5453b83bdf (send-email: --batch-size to work around some SMTP
> server limit, 2017-05-21) lays out, we rather want to not have this
> interface anyway and would rather want to react on the server throttling
> dynamically.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -379,6 +379,9 @@ unless ($rc) {
> +die __("When a batch size is given, the relogin delay must be set\n")
> +       if defined $relogin_delay and not defined $batch_size;

This only makes sense is 'batch-size' is specified but not 'relogin'.
If the other way around, then the error is confusing. How about this
instead?

    "--batch-size and --relogin must be specified together"

...or something.

Re: [PATCH] send-email: error out when relogin delay is missing

[Stefan Beller]

On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
>> [...]
>> Error out for now instead of potentially confusing the user.
>> As 5453b83bdf (send-email: --batch-size to work around some SMTP
>> server limit, 2017-05-21) lays out, we rather want to not have this
>> interface anyway and would rather want to react on the server throttling
>> dynamically.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> @@ -379,6 +379,9 @@ unless ($rc) {
>> +die __("When a batch size is given, the relogin delay must be set\n")
>> +       if defined $relogin_delay and not defined $batch_size;
>
> This only makes sense is 'batch-size' is specified but not 'relogin'.
> If the other way around, then the error is confusing. How about this
> instead?
>
>     "--batch-size and --relogin must be specified together"
>
> ...or something.

I like this for its expressiveness as it would have helped me a lot.
I dislike this because it is incorrect when you use the config options
instead of command line arguments.

Stefan

Re: [PATCH] send-email: error out when relogin delay is missing

[Eric Sunshine]

On Thu, Feb 8, 2018 at 1:21 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Feb 8, 2018 at 12:08 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> On Wed, Feb 7, 2018 at 6:43 PM, Stefan Beller <sbeller@google.com> wrote:
>>> +die __("When a batch size is given, the relogin delay must be set\n")
>>> +       if defined $relogin_delay and not defined $batch_size;
>>
>> This only makes sense is 'batch-size' is specified but not 'relogin'.
>> If the other way around, then the error is confusing. How about this
>> instead?
>>     "--batch-size and --relogin must be specified together"
>> ...or something.
>
> I like this for its expressiveness as it would have helped me a lot.
> I dislike this because it is incorrect when you use the config options
> instead of command line arguments.

Perhaps:

    "`batch-size` and `relogin` must be specified together
      (via command-line or configuration option)"

[PATCH] send-email: error out when relogin delay is missing

[Stefan Beller]

When the batch size is neither configured nor given on the command
line, but the relogin delay is given, then the current code ignores
the relogin delay setting.

This is unsafe as there was some intention when setting the batch size.
One workaround would be to just assume a batch size of 1 as a default.
This however may be bad UX, as then the user may wonder why it is sending
slowly without apparent batching.

Error out for now instead of potentially confusing the user.
As 5453b83bdf (send-email: --batch-size to work around some SMTP
server limit, 2017-05-21) lays out, we rather want to not have this
interface anyway and would rather want to react on the server throttling
dynamically.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 git-send-email.perl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 340b5c8482..f7913f7c2c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -379,6 +379,10 @@ unless ($rc) {
 die __("Cannot run git format-patch from outside a repository\n")
 	if $format_patch and not $repo;
 
+die __("`batch-size` and `relogin` must be specified together " .
+	"(via command-line or configuration option)\n")
+	if defined $relogin_delay and not defined $batch_size;
+
 # Now, let's fill any that aren't set in with defaults:
 
 sub read_config {
-- 
2.15.1.433.g936d1b9894.dirty