{"thread":{"id":"63097","subject":"[GSoC PATCH v3 0/1] Refactor SMTP Auth Error Handling","startedAt":"2025-03-12T06:47:01Z","lastAt":"2025-03-26T07:53:09Z","messageCount":27,"participants":["Zheng Yuting","Junio C Hamano","Yuting Zheng","Meet Soni"],"isPatch":true,"patchVersion":3,"patchTotal":1},"messages":[{"id":"514020","messageId":"20250312064639.668875-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":null,"subject":"[GSoC PATCH v3 0/1] Refactor SMTP Auth Error Handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-12T06:46:35Z","receivedAt":"2025-03-12T06:47:01Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch unifies error capture for both SASL and plain SMTP authentication.\nIt replaces regex-based error detection with SMTP status code parsing,\ndifferentiating transient (retryable) errors from permanent failures.\n\n\nZheng Yuting (1):\n  SMTP Auth: Use status codes to differentiate transient vs. permanent\n    errors\n\n git-send-email.perl | 72 +++++++++++++++++++++++++++------------------\n 1 file changed, 43 insertions(+), 29 deletions(-)\n\n--\n2.49.0.rc0.57.gdb91954e18\n"},{"id":"514021","messageId":"20250312064639.668875-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250312064639.668875-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v3 1/1] Unify SMTP auth error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-12T06:46:36Z","receivedAt":"2025-03-12T06:47:11Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Refactored SMTP authentication to use a unified error capture block for\nboth SASL and plain methods. Errors are now handled by parsing SMTP status\ncodes (4yz for transient, 5yz for permanent) instead of relying on regex\nmatching. This change improves clarity .\n\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 72 +++++++++++++++++++++++++++------------------\n 1 file changed, 43 insertions(+), 29 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex a012d61abb..532dda264c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1411,7 +1411,7 @@ sub smtp_auth_maybe {\n \teval {\n \t\trequire Authen::SASL;\n \t\tAuthen::SASL->import(qw(Perl));\n-\t};\n+\t}\n\n \t# Check mechanism naming as defined in:\n \t# https://tools.ietf.org/html/rfc4422#page-8\n@@ -1426,42 +1426,56 @@ sub smtp_auth_maybe {\n \t\t'protocol' => 'smtp',\n \t\t'host' => smtp_host_string(),\n \t\t'username' => $smtp_authuser,\n+\t\t# if there's no password, \"git credential fill\" will\n+\t\t# give us one, otherwise it'll just pass this one.\n \t\t'password' => $smtp_authpass\n-\n \t}, sub {\n \t\tmy $cred = shift;\n \t\tmy $result;\n \t\tmy $error;\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t} else {\n-\t\t\t# Handle plain authentication errors\n-\t\t\teval {\n+\n+\t\t# catch all SMTP auth error\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser => $cred->{'username'},\n+\t\t\t\t\t\tpass => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n \t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t\t\t\t1; # Ensure true value is returned\n-\t\t\t} or do {\n-\t\t\t\t$error = $@ || 'Unknown error';\n-\t\t\t};\n-\t\t}\n-\t\t# Unified error handling logic\n+\t\t\t}\n+\t\t\t1; # Ensure true value is returned if no exception is thrown.\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n+\n+\t\t#check if an error was captured\n \t\tif ($error) {\n-\t\t\t# Match temporary errors\n-\t\t\tif ($error =~ /timeout|temporary|greylist|throttled|quota\\s+exceeded|queue|overload|try\\s+again|connection\\s+lost|network\\s+error/i) {\n-\t\t\t\twarn \"SMTP temporary error: $error\";\n-\t\t\t\treturn 1;\n+\t\t\t#Parse SMTP status code from error message in:\n+\t\t\t#https://www.rfc-editor.org/rfc/rfc5321.html\n+\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\t\t\tmy $status_code = $1;\n+\t\t\t\tif ($status_code =~ /^4/) {\n+\t\t\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\t\t\treturn 0;\n+\t\t\t\t}\n+\t\t\t\t# If no status code is found, treat as permanent error\n+\t\t\t\twarn \"SMTP unknown error: $error\";\n+\t\t\t\treturn 0;\n \t\t\t}\n-\t\t\treturn 0;\n-\t\t}\n-\t\treturn !!$result;\n-\t});\n+\t\t\treturn $result ? 1 : 0;\n+\t\t});\n+\n \treturn $auth;\n }\n\n--\n2.49.0.rc0.57.gdb91954e18\n"},{"id":"514227","messageId":"xmqqsengn1ms.fsf@gitster.g","threadId":"63097","inReplyTo":"20250312064639.668875-2-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v3 1/1] Unify SMTP auth error handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-13T19:58:51Z","receivedAt":"2025-03-13T19:58:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zheng Yuting <05zyt30@gmail.com> writes:\n\n> Refactored SMTP authentication to use a unified error capture block for\n> both SASL and plain methods. Errors are now handled by parsing SMTP status\n> codes (4yz for transient, 5yz for permanent) instead of relying on regex\n> matching. This change improves clarity .\n\n\"improves clarity .\" is (not well formatted and) a bit subjective\nand does not apply to all three changes the patch is making here,\ndoes it?\n\n> Signed-off-by: Zheng Yuting <05ZYT30@gmail.com>\n> ---\n>  git-send-email.perl | 72 +++++++++++++++++++++++++++------------------\n>  1 file changed, 43 insertions(+), 29 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index a012d61abb..532dda264c 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1411,7 +1411,7 @@ sub smtp_auth_maybe {\n>  \teval {\n>  \t\trequire Authen::SASL;\n>  \t\tAuthen::SASL->import(qw(Perl));\n> -\t};\n> +\t}\n\nHmph, the interpreter may tolerate the new block-eval \"eval {}\"\nsimple statement that lacks terminating ';' but is this an\nimprovement?  The original look more kosher from syntactic point of\nview.  It seems to be totally unrelated change from the rest of the\npatch.\n\n> @@ -1426,42 +1426,56 @@ sub smtp_auth_maybe {\n>  \t\t'protocol' => 'smtp',\n>  \t\t'host' => smtp_host_string(),\n>  \t\t'username' => $smtp_authuser,\n> +\t\t# if there's no password, \"git credential fill\" will\n> +\t\t# give us one, otherwise it'll just pass this one.\n>  \t\t'password' => $smtp_authpass\n> -\n\nWe seem to already have the comment added by this hunk, since\n4d31a44a (git-send-email: use git credential to obtain password,\n2013-02-12).  Am I looking at a wrong version of the source (or a\nwrong version of the patch)?\n\n>  \t}, sub {\n>  \t\tmy $cred = shift;\n>  \t\tmy $result;\n>  \t\tmy $error;\n> -\t\tif ($smtp_auth) {\n> -\t\t\tmy $sasl = Authen::SASL->new(\n> -\t\t\t\tmechanism => $smtp_auth,\n> -\t\t\t\tcallback => {\n> -\t\t\t\t\tuser => $cred->{'username'},\n> -\t\t\t\t\tpass => $cred->{'password'},\n> -\t\t\t\t\tauthname => $cred->{'username'},\n> -\t\t\t\t}\n> -\t\t\t);\n> -\t\t\treturn !!$smtp->auth($sasl);\n> -\t\t} else {\n> -\t\t\t# Handle plain authentication errors\n> -\t\t\teval {\n\nAnd curiously we do not seem to have this else clause with the\ncomment that is getting removed.\n\n> +\t\t# catch all SMTP auth error\n> +\t\teval {\n> +\t\t\tif ($smtp_auth) {\n> +\t\t\t\tmy $sasl = Authen::SASL->new(\n> +\t\t\t\t\tmechanism => $smtp_auth,\n> +\t\t\t\t\tcallback => {\n> +\t\t\t\t\t\tuser => $cred->{'username'},\n> +\t\t\t\t\t\tpass => $cred->{'password'},\n> +\t\t\t\t\t\tauthname => $cred->{'username'},\n> +\t\t\t\t\t}\n> +\t\t\t\t);\n> +\t\t\t\t$result = $smtp->auth($sasl);\n> +\t\t\t} else {\n>  \t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n> -\t\t\t\t1; # Ensure true value is returned\n> -\t\t\t} or do {\n> -\t\t\t\t$error = $@ || 'Unknown error';\n> -\t\t\t};\n> -\t\t}\n> -\t\t# Unified error handling logic\n> +\t\t\t}\n> +\t\t\t1; # Ensure true value is returned if no exception is thrown.\n> +\t\t} or do {\n> +\t\t\t$error = $@ || 'Unknown error';\n> +\t\t};\n> +\n> +\t\t#check if an error was captured\n\nAs I do not see two evals in our copy of git-send-email.perl source,\nit may be moot at this point to comment on this patch, but if we did\nhave a eval block each of the if/else arms, moving the control\nstructure around and turning \"if eval {} else eval {}\" into \"eval {\nif ... else ...}\" may make it cleaner to see what is going on,\nespecially if we plan to extend the choices and add elsif to the\nchain later.\n\n>  \t\tif ($error) {\n> -\t\t\t# Match temporary errors\n> -\t\t\tif ($error =~ /timeout|temporary|greylist|throttled|quota\\s+exceeded|queue|overload|try\\s+again|connection\\s+lost|network\\s+error/i) {\n> -\t\t\t\twarn \"SMTP temporary error: $error\";\n> -\t\t\t\treturn 1;\n> +\t\t\t#Parse SMTP status code from error message in:\n> +\t\t\t#https://www.rfc-editor.org/rfc/rfc5321.html\n\nHave a SP between \"#\" and the comment body.\n\nUsing the numeric error codes allows us to give more precise errors,\nwhich should be a good change that can be done regardless of the\neval change.  IOW, this part should be in a separate patch on its\nown, either before or after the if-eval-else-eval change. \n\nI'll stop here, as the patch does not seem to be designed to apply\nto our source tree.\n\n"},{"id":"514259","messageId":"CAMvj1+pn_+8PRXCUds0NHrRPBWh1uUzOOeNGhXTmHRTg_DGqHg@mail.gmail.com","threadId":"63097","inReplyTo":"xmqqsengn1ms.fsf@gitster.g","subject":"Re: [GSoC PATCH v3 1/1] Unify SMTP auth error handling","fromName":"Yuting Zheng","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-14T12:55:22Z","receivedAt":"2025-03-14T12:55:36Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"On Fri Mar 14, 2025 at 3:58 AM CST, Junio C Hamano wrote:\n\nThank you for the thorough review. As a newcomer, I really appreciate you\ntaking time to help me improve.\n\n> \"improves clarity .\" is (not well formatted and) a bit subjective\n> and does not apply to all three changes the patch is making here,\n> does it?\n\nI'll reformat the commit message and split the patch into more detailed\nparts.\n\n> Hmph, the interpreter may tolerate the new block-eval \"eval {}\"\n> simple statement that lacks terminating ';' but is this an\n> improvement?  The original look more kosher from syntactic point of\n> view.  It seems to be totally unrelated change from the rest of the\n> patch.\n\nI'll revert it to the original state.\n\n> We seem to already have the comment added by this hunk, since\n> 4d31a44a (git-send-email: use git credential to obtain password,\n> 2013-02-12).  Am I looking at a wrong version of the source (or a\n> wrong version of the patch)?\n>\n> And curiously we do not seem to have this else clause with the\n> comment that is getting removed.\n\nYou're correct - this was caused by my failure to rebase before\nsubmission.I'll clean up all duplicate comments.\n\n> As I do not see two evals in our copy of git-send-email.perl source,\n> it may be moot at this point to comment on this patch, but if we did\n> have a eval block each of the if/else arms, moving the control\n> structure around and turning \"if eval {} else eval {}\" into \"eval {\n> if ... else ...}\" may make it cleaner to see what is going on,\n> especially if we plan to extend the choices and add elsif to the\n> chain later.\n\nI'll use if/else structure which is more extensible.\n\n> Have a SP between \"#\" and the comment body.\n\nUnderstood. I'll rigorously adhere to code style guidelines by adding\nspace after comment markers.\n\n> I'll stop here, as the patch does not seem to be designed to apply\n> to our source tree.\n\nThis was caused by my local branch being several commits behind upstream.\nI've now synchronized and will resubmit properly.\n"},{"id":"514374","messageId":"20250316050920.3264895-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"xmqqsengn1ms.fsf@gitster.g","subject":"[GSoC PATCH v4 0/2] smtp_auth_maybe: unified error capture and status code processing optimization","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-16T05:09:18Z","receivedAt":"2025-03-16T05:09:36Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This v4 patch series includes two improvements:\n\n1. Unified error capture:\nConsolidate exception handling within a single eval block by introducing\nlocal variables to store results and error states, thereby streamlining\ncode structure and enabling future extensibility.\n\n2. Status code processing optimization:\nAfter catching the authentication exception, parse the three-digit status\ncode in the error message, For temporary errors (4yz), only print warnings\nand return success, while for permanent errors (5xx), return failure,\nUnrecognized status codes are treated as permanent errors by default.\n\nZheng Yuting (2):\n  Unify capture of SMTP errors\n  Error handling for SMTP status codes\n\n git-send-email.perl | 62 ++++++++++++++++++++++++++++++++-------------\n 1 file changed, 45 insertions(+), 17 deletions(-)\n\n--\n2.48.1\n"},{"id":"514375","messageId":"20250316050920.3264895-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250316050920.3264895-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v4 1/2] Unify capture of SMTP errors","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-16T05:09:19Z","receivedAt":"2025-03-16T05:09:42Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This change adds local variables $result and $error to store authentication\nreturn results and exception information respectively.\n\nIn the eval block, different auth methods are called depending on whether\nthe SMTP authentication mechanism is specified, and true is returned when\nthere is no exception.\n\nAfter catching the exception, the error information is saved. This makes\nthe error capture logic more centralized and easier to understand, and\nlays the foundation for subsequent expansion.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 45 ++++++++++++++++++++++++++-------------------\n 1 file changed, 26 insertions(+), 19 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..8feb43e9f7 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n\n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,25 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t});\n-\n-\treturn $auth;\n-}\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n+\t\treturn $result ? 1 : 0;\n+\t}\n\n sub ssl_verify_params {\n \teval {\n--\n2.48.1\n"},{"id":"514376","messageId":"20250316050920.3264895-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250316050920.3264895-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v4 2/2] Error handling for SMTP status codes","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-16T05:09:20Z","receivedAt":"2025-03-16T05:09:45Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This change further parses and processes the captured exception information\nbased on the previous patch's unified error capture. Specifically, a\nthree-digit status code is extracted from the error information through a\nregular expression, and judged according to the definition of RFC 5321:\n\n- If the status code starts with \"4\" (temporary error), only a warning is\nprinted and success (1) is returned to allow subsequent retries;\n\n- If the status code starts with \"5\" (permanent error), a warning is\nprinted and failure (0) is returned;\n\n- If the status code is not recognized in the error, it is considered a\npermanent error, and an unknown error message is printed and failure is\nreturned.\n\nIn the absence of an error, the authentication result is still returned\naccording to the original logic. This change makes SMTP authentication\nerror handling more refined,\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 27 ++++++++++++++++++++++++---\n 1 file changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8feb43e9f7..69ba328653 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,9 +1454,30 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n\n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n-\t\treturn $result ? 1 : 0;\n-\t}\n+\t\t# check if an error was captured\n+\t\tif ($error) {\n+\t\t\t# parse SMTP status code from error message in:\n+\t\t\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\t\t\tmy $status_code = $1;\n+\t\t\t\tif ($status_code =~ /^4/) {\n+\t\t\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\t\t\treturn 0;\n+\t\t\t\t}\n+\t\t\t\t# if no recognized status code is found, treat as permanent error\n+\t\t\t\twarn \"SMTP unknown error: $error\";\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\treturn $result ? 1 : 0;\n+\t\t} else {\n+\t\t\treturn $result ? 1 : 0;\n+\t\t}\n+}\n\n sub ssl_verify_params {\n \teval {\n--\n2.48.1\n"},{"id":"514465","messageId":"xmqq5xk76z4d.fsf@gitster.g","threadId":"63097","inReplyTo":"20250316050920.3264895-1-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v4 0/2] smtp_auth_maybe: unified error capture and status code processing optimization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-17T23:01:06Z","receivedAt":"2025-03-17T23:01:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zheng Yuting <05zyt30@gmail.com> writes:\n\n> This v4 patch series includes two improvements:\n>\n> 1. Unified error capture:\n> Consolidate exception handling within a single eval block by introducing\n> local variables to store results and error states, thereby streamlining\n> code structure and enabling future extensibility.\n>\n> 2. Status code processing optimization:\n> After catching the authentication exception, parse the three-digit status\n> code in the error message, For temporary errors (4yz), only print warnings\n> and return success, while for permanent errors (5xx), return failure,\n> Unrecognized status codes are treated as permanent errors by default.\n>\n> Zheng Yuting (2):\n>   Unify capture of SMTP errors\n>   Error handling for SMTP status codes\n\nGive title your commits following the project convention\n(Documentation/SubmittingPatches:summary-section).\n\nI think these two can share \"sendemail:\" as their \"<area>:\" part.\n\n\tsendemail: capture errors in an eval {} block\n\tsendemail: finer-grained SMTP error handling\n\nor something like that, perhaps.\n\nFor both patches, the usual way to compose a log message of this\nproject is to\n\n - Give an observation on how the current system work in the present\n   tense (so no need to say \"Currently X is Y\", just \"X is Y\"), and\n   discuss what you perceive as a problem in it.\n\n - Propose a solution (optional---often, problem description\n   trivially leads to an obvious solution in reader's minds).\n\n - Give commands to the codebase to \"become like so\".\n\nin this order.  I got an impression that at least your 1/2 it was\nunclear which part was explaining the state before the patch and\nwhich part was about the state after the patch.\n\nThanks.\n"},{"id":"514600","messageId":"20250319020221.2160371-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250316050920.3264895-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v5 0/2] sendemail: improve error capture and status code handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-19T02:02:19Z","receivedAt":"2025-03-19T02:02:38Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch series improves SMTP authentication error handling.\n\nAuth relied solely on return values without capturing exceptions,\nmisjudging non-credential errors as authentication failures.\n\nPatch v5 1/2 wraps the auth process in an eval {} block to catch all\nexceptions, adds var error for future handling, and var result to return\nauth state.\n\nPatch v5 2/2 introduces finer-grained SMTP error handling, extracting\nstatus codes per RFC 5321 to differentiate between temporary (4yz) and\npermanent (5yz) errors. Unrecognized codes are treated as permanent\nfailures. Otherwise return the authentication result.\n\n\nZheng Yuting (2):\n  sendemail: capture errors in an eval {} block\n  sendemail: finer-grained SMTP error handling\n\n git-send-email.perl | 62 ++++++++++++++++++++++++++++++++-------------\n 1 file changed, 45 insertions(+), 17 deletions(-)\n\n--\n2.48.1\n"},{"id":"514601","messageId":"20250319020221.2160371-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250319020221.2160371-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v5 1/2] sendemail: capture errors in an eval {} block","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-19T02:02:20Z","receivedAt":"2025-03-19T02:02:41Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Auth relied solely on return values without catching errors, misjudging\nnon-credential errors as auth failures without details.\n\nWrap the entire auth process in an eval {} block to catch all exceptions,\nincluding non-credential errors.\n\nAdd $error to store exceptions for future handling and $result for\nauth results.\n\nUses 'or do' to replace direct returns.\n\nMerges if/else branches, integrates SASL and basic auth, with comments\nfor future status code handling.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 45 ++++++++++++++++++++++++++-------------------\n 1 file changed, 26 insertions(+), 19 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..8feb43e9f7 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n\n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,25 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t});\n-\n-\treturn $auth;\n-}\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n+\t\treturn $result ? 1 : 0;\n+\t}\n\n sub ssl_verify_params {\n \teval {\n--\n2.48.1\n"},{"id":"514602","messageId":"20250319020221.2160371-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250319020221.2160371-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v5 2/2] sendemail: finer-grained SMTP error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-19T02:02:21Z","receivedAt":"2025-03-19T02:02:44Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Code captured errors but did not process them further.\nThis treated all failures the same without distinguishing SMTP status.\n\nAdd a regex to extract status codes as defined in RFC 5321:\n\n- For 4yz (temporary errors), return 1 and allow retries.\n- For 5yz (permanent errors), return 0 as failure.\n- For unrecognized codes, treat as permanent errors.\n- If no error occurs, return the authentication result.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 27 ++++++++++++++++++++++++---\n 1 file changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8feb43e9f7..69ba328653 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,9 +1454,30 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n\n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n-\t\treturn $result ? 1 : 0;\n-\t}\n+\t\t# check if an error was captured\n+\t\tif ($error) {\n+\t\t\t# parse SMTP status code from error message in:\n+\t\t\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\t\t\tmy $status_code = $1;\n+\t\t\t\tif ($status_code =~ /^4/) {\n+\t\t\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\t\t\treturn 0;\n+\t\t\t\t}\n+\t\t\t\t# if no recognized status code is found, treat as permanent error\n+\t\t\t\twarn \"SMTP unknown error: $error\";\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\treturn $result ? 1 : 0;\n+\t\t} else {\n+\t\t\treturn $result ? 1 : 0;\n+\t\t}\n+}\n\n sub ssl_verify_params {\n \teval {\n--\n2.48.1\n"},{"id":"514605","messageId":"CAPhwyn0Sq0hDktPtf53Qs6LKwNsmn6yXuVyEfcYzyXK4yjd7HA@mail.gmail.com","threadId":"63097","inReplyTo":"20250319020221.2160371-1-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v5 0/2] sendemail: improve error capture and status code handling","fromName":"Meet Soni","fromEmail":"meetsoni3017@gmail.com","sentAt":"2025-03-19T06:35:45Z","receivedAt":"2025-03-19T06:35:58Z","isPatch":true,"sender":{"key":"meetsoni3017@gmail.com","avatar":"https://avatars.githubusercontent.com/u/92802561?v=4"},"body":"On Wed, 19 Mar 2025 at 07:32, Zheng Yuting <05zyt30@gmail.com> wrote:\n>\n> This patch series improves SMTP authentication error handling.\n>\n> Auth relied solely on return values without capturing exceptions,\n> misjudging non-credential errors as authentication failures.\n>\n> Patch v5 1/2 wraps the auth process in an eval {} block to catch all\n> exceptions, adds var error for future handling, and var result to return\n> auth state.\n>\n> Patch v5 2/2 introduces finer-grained SMTP error handling, extracting\n> status codes per RFC 5321 to differentiate between temporary (4yz) and\n> permanent (5yz) errors. Unrecognized codes are treated as permanent\n> failures. Otherwise return the authentication result.\n>\n>\n> Zheng Yuting (2):\n>   sendemail: capture errors in an eval {} block\n>   sendemail: finer-grained SMTP error handling\n>\nI'm not sure if this is worth a re-roll but, `sendemail` should be `send-email`.\n>  git-send-email.perl | 62 ++++++++++++++++++++++++++++++++-------------\n>  1 file changed, 45 insertions(+), 17 deletions(-)\n>\n> --\n> 2.48.1\n>\nThanks\n"},{"id":"514817","messageId":"20250321025128.68463-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250319020221.2160371-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v6 0/2] send-email: improve error capture and status code handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-21T02:51:26Z","receivedAt":"2025-03-21T02:51:45Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch series improves SMTP authentication error handling.\n\nAuth relied solely on return values without capturing exceptions,\nmisjudging non-credential errors as authentication failures.\n\nPatch v6 1/2 wraps the auth process in an eval {} block to catch all\nexceptions, adds var error for future handling, and var result to return\nauth state.\n\nPatch v6 2/2 introduces finer-grained SMTP error handling, extracting\nstatus codes per RFC 5321 to differentiate between temporary (4yz) and\npermanent (5yz) errors. Unrecognized codes are treated as permanent\nfailures. Otherwise return the authentication result.\n\nZheng Yuting (2):\n  send-email: capture errors in an eval {} block\n  send-email: finer-grained SMTP error handling\n\n git-send-email.perl | 62 ++++++++++++++++++++++++++++++++-------------\n 1 file changed, 45 insertions(+), 17 deletions(-)\n\n--\n2.48.1\n"},{"id":"514818","messageId":"20250321025128.68463-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250321025128.68463-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v6 1/2] send-email: capture errors in an eval {} block","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-21T02:51:27Z","receivedAt":"2025-03-21T02:51:48Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Auth relied solely on return values without catching errors. This misjudges\nnon-credential errors as auth failure without error info.\n\nPatch wraps the entire auth process in an eval {} block to catch\nall exceptions, including non-credential errors. It adds a new $error var,\nuses 'or do' to prevent flow break, and returns $result ? 1 : 0. And merges\nif/else branches, integrates SASL and basic auth, with comments for\nfuture status code handling.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 45 ++++++++++++++++++++++++++-------------------\n 1 file changed, 26 insertions(+), 19 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..8feb43e9f7 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n\n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,25 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n-\t});\n-\n-\treturn $auth;\n-}\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n+\t\treturn $result ? 1 : 0;\n+\t}\n\n sub ssl_verify_params {\n \teval {\n--\n2.48.1\n"},{"id":"514819","messageId":"20250321025128.68463-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250321025128.68463-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v6 2/2] send-email: finer-grained SMTP error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-21T02:51:28Z","receivedAt":"2025-03-21T02:51:51Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Code captured errors but did not process them further.\nThis treated all failures the same without distinguishing SMTP status.\n\nAdd a regex to extract status codes as defined in RFC 5321:\n\n- For 4yz (temporary errors), return 1 and allow retries.\n- For 5yz (permanent errors), return 0 as failure.\n- For unrecognized codes, treat as permanent errors.\n- If no error occurs, return the authentication result.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 27 ++++++++++++++++++++++++---\n 1 file changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 8feb43e9f7..69ba328653 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,9 +1454,30 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n \n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit\n-\t\treturn $result ? 1 : 0;\n-\t}\n+\t\t# check if an error was captured\n+\t\tif ($error) {\n+\t\t\t# parse SMTP status code from error message in:\n+\t\t\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\t\t\tmy $status_code = $1;\n+\t\t\t\tif ($status_code =~ /^4/) {\n+\t\t\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\t\t\treturn 0;\n+\t\t\t\t}\n+\t\t\t\t# if no recognized status code is found, treat as permanent error\n+\t\t\t\twarn \"SMTP unknown error: $error\";\n+\t\t\t\treturn 0;\n+\t\t\t}\n+\t\t\treturn $result ? 1 : 0;\n+\t\t} else {\n+\t\t\treturn $result ? 1 : 0;\n+\t\t}\n+}\n \n sub ssl_verify_params {\n \teval {\n-- \n2.48.1\n\n"},{"id":"514835","messageId":"xmqqo6xutmvc.fsf@gitster.g","threadId":"63097","inReplyTo":"20250321025128.68463-1-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v6 0/2] send-email: improve error capture and status code handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-21T15:38:31Z","receivedAt":"2025-03-21T15:38:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Does not pass t9001 when applied to Git 2.49.0.\n\n\n\nTest Summary Report\n-------------------\nt9001-send-email.sh (Wstat: 256 (exited 1) Tests: 215 Failed: 169)\n  Failed tests:  4-7, 9-10, 12-13, 15, 17, 19, 21-28, 31-35\n                37, 39-60, 62, 64-66, 68, 70, 72, 74, 76\n                78, 80, 82, 84-91, 94-99, 101-112, 114-127\n                130, 132-135, 138, 140-142, 144, 146, 148\n                151, 153-167, 169-189, 194-203, 205-215\n  Non-zero exit status: 1\nFiles=1, Tests=215, 24 wallclock secs ( 0.19 usr  0.02 sys +  9.31 cusr 10.44 csys = 19.96 CPU)\nResult: FAIL\n"},{"id":"514869","messageId":"20250323022111.20226-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250321025128.68463-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v7 0/2] send-email: improve error capture and status code handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-23T02:21:09Z","receivedAt":"2025-03-23T02:21:25Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch series improves SMTP authentication error handling.\n\nAuth relied solely on return values without capturing exceptions,\nmisjudging non-credential errors as authentication failures.\n\nPatch v7 1/2 wraps the auth process in an eval {} block to catch all\nexceptions, adds var error for future handling, and var result to return\nauth state.\n\nPatch v7 2/2 introduces finer-grained SMTP error handling, extracting\nstatus codes per RFC 5321 to differentiate between temporary (4yz) and\npermanent (5yz) errors. For 4yz (transient errors), return 1 and allow\nretries. For 5yz (permanent errors), return 0 as failure. Unrecognized\ncodes are treated as transient errors by returning 1. If the status code\nis not caught or no error occurs but no result is defined, return 1 as a\ntransient error. Otherwise, return the authentication result.\n\nZheng Yuting (2):\n  send-email: capture errors in an eval {} block\n  send-email: finer-grained SMTP error handling\n\n git-send-email.perl | 68 +++++++++++++++++++++++++++++++++++----------\n 1 file changed, 54 insertions(+), 14 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"514870","messageId":"20250323022111.20226-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250323022111.20226-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v7 1/2] send-email: capture errors in an eval {} block","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-23T02:21:10Z","receivedAt":"2025-03-23T02:21:29Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Auth relied solely on return values without catching errors. This misjudges\nnon-credential errors as auth failure without error info.\n\nPatch wraps the entire auth process in an eval {} block to catch\nall exceptions, including non-credential errors. It adds a new $error var,\nuses 'or do' to prevent flow break, and returns $result ? 1 : 0. And merges\nif/else branches, integrates SASL and basic auth, with comments for\nfuture status code handling.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 43 +++++++++++++++++++++++++++----------------\n 1 file changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..0f05f55e50 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n \n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,21 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n-\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n+\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n+\t\t# return 1 when failed due to non-credential reasons\n+\t\treturn $error ? 1 : ($result ? 1 : 0);\n \t});\n \n \treturn $auth;\n-- \n2.49.0\n\n"},{"id":"514871","messageId":"20250323022111.20226-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250323022111.20226-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v7 2/2] send-email: finer-grained SMTP error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-23T02:21:11Z","receivedAt":"2025-03-23T02:21:33Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Code captured errors but did not process them further.\nThis treated all failures the same without distinguishing SMTP status.\n\nAdd a regex to extract status codes as defined in RFC 5321:\n\n- For 4yz (transient errors), return 1 and allow retries.\n- For 5yz (permanent errors), return 0 as failure.\n- For unrecognized codes, return 1 as transient errors.\n- For errors where the status code was not caught, return 1 as transient\nerrors.\n- If no error and no result is returned, return 1 as a transient error.\n- If no error occurs with result defined, return the authentication result.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 35 ++++++++++++++++++++++++++++++++---\n 1 file changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 0f05f55e50..e09a4a316f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,9 +1454,38 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n \n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n-\t\t# return 1 when failed due to non-credential reasons\n-\t\treturn $error ? 1 : ($result ? 1 : 0);\n+\t\tif ($error) {\n+\t\t\t# check if an error was captured\n+\t\t\t# parse SMTP status code from error message in:\n+\t\t\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\t\t\tmy $status_code = $1;\n+\t\t\t\tif ($status_code =~ /^4/) {\n+\t\t\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\t\t\treturn 0;\n+\t\t\t\t} else {\n+\t\t\t\t\t# if no recognized status code is found, treat as transient error\n+\t\t\t\t\twarn \"SMTP unknown error: $error. Treating as permanent failure.\";\n+\t\t\t\t\treturn 1;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\t# if no status code is found, treat as transient error\n+\t\t\t\twarn \"SMTP generic error: $error\";\n+\t\t\t\treturn 1;\n+\t\t\t}\n+\t\t} elsif (!defined $result) {\n+\t\t\t# if no error and no result is returned, treat as transient error\n+\t\t\twarn \"SMTP no result error: $error\";\n+\t\t    return 1; \n+\t\t}\n+\t\telse {\n+\t\t\treturn $result ? 1 : 0;\n+\t\t}\n \t});\n \n \treturn $auth;\n-- \n2.49.0\n\n"},{"id":"514902","messageId":"xmqqh63jotn4.fsf@gitster.g","threadId":"63097","inReplyTo":"20250323022111.20226-3-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v7 2/2] send-email: finer-grained SMTP error handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-24T06:00:15Z","receivedAt":"2025-03-24T06:00:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zheng Yuting <05zyt30@gmail.com> writes:\n\n> +\t\tif ($error) {\n\nThis block is full of overly long lines.  Would it make sense to\nturn it into a helper sub and do just this here in the block\n\n\t\t\treturn handle_smtp_error($error);\n\nAnd then the remainder of the block would become a single helper\nfunction that is called only from here, losing 2 levels of\nindentation.\n\n> +\t\t\t# check if an error was captured\n> +\t\t\t# parse SMTP status code from error message in:\n> +\t\t\t# https://www.rfc-editor.org/rfc/rfc5321.html\n> +\t\t\tif ($error =~ /\\b(\\d{3})\\b/) {\n> +\t\t\t\tmy $status_code = $1;\n> +\t\t\t\tif ($status_code =~ /^4/) {\n> +\t\t\t\t\t# 4yz: Transient Negative Completion reply\n> +\t\t\t\t\twarn \"SMTP temporary error (status code $status_code): $error\";\n> +\t\t\t\t\treturn 1;\n> +\t\t\t\t} elsif ($status_code =~ /^5/) {\n> +\t\t\t\t\t# 5yz: Permanent Negative Completion reply\n> +\t\t\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n> +\t\t\t\t\treturn 0;\n> +\t\t\t\t} else {\n> +\t\t\t\t\t# if no recognized status code is found, treat as transient error\n> +\t\t\t\t\twarn \"SMTP unknown error: $error. Treating as permanent failure.\";\n\nThe comment and warning message say different things.  I suspect\nthat the warning message is wrong, as the branch returns 1 to signal\nthe caller to do the same thing a the 4yz transient case?\n\n> +\t\t\t\t\treturn 1;\n> +\t\t\t\t}\n> +\t\t\t} else {\n> +\t\t\t\t# if no status code is found, treat as transient error\n> +\t\t\t\twarn \"SMTP generic error: $error\";\n> +\t\t\t\treturn 1;\n> +\t\t\t}\n> +\t\t} elsif (!defined $result) {\n> +\t\t\t# if no error and no result is returned, treat as transient error\n> +\t\t\twarn \"SMTP no result error: $error\";\n> +\t\t    return 1; \n> +\t\t}\n> +\t\telse {\n> +\t\t\treturn $result ? 1 : 0;\n> +\t\t}\n>  \t});\n>  \n>  \treturn $auth;\n"},{"id":"514933","messageId":"20250324145332.571813-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250321025128.68463-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v8 0/2] send-email: improve error capture and status code handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-24T14:53:30Z","receivedAt":"2025-03-24T14:53:51Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch series improves SMTP authentication error handling.\n\nAuth relied solely on return values without capturing exceptions,\nmisjudging non-credential errors as authentication failures.\n\nPatch v8 1/2 wraps the auth process in an eval {} block to catch all\nexceptions, adds var error for future handling, and var result to return\nauth state.\n\nPatch v8 2/2 introduces finer-grained SMTP error handling by extracting\nstatus codes per RFC 5321. For 4yz (transient) errors, return 1 and allow\nretries; for 5yz (permanent) errors, return 0. Unrecognized or uncaught\nstatus codes are treated as transient errors (return 1). If no error is\npresent and no result is defined, return 1 as a transient error; otherwise,\nreturn the authentication result.\n\n\n Zheng Yuting (2):\n  send-email: capture errors in an eval {} block\n  send-email: finer-grained SMTP error handling\n\n git-send-email.perl | 69 +++++++++++++++++++++++++++++++++++----------\n 1 file changed, 54 insertions(+), 15 deletions(-)\n\n--\n2.49.0\n"},{"id":"514934","messageId":"20250324145332.571813-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250324145332.571813-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v8 1/2] send-email: capture errors in an eval {} block","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-24T14:53:31Z","receivedAt":"2025-03-24T14:53:55Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Auth relied solely on return values without catching errors. This misjudges\nnon-credential errors as auth failure without error info.\n\nPatch wraps the entire auth process in an eval {} block to catch\nall exceptions, including non-credential errors. It adds a new $error var,\nuses 'or do' to prevent flow break, and returns $result ? 1 : 0. And merges\nif/else branches, integrates SASL and basic auth, with comments for\nfuture status code handling.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 43 +++++++++++++++++++++++++++----------------\n 1 file changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..0f05f55e50 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n \n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,21 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n-\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n+\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n+\t\t# return 1 when failed due to non-credential reasons\n+\t\treturn $error ? 1 : ($result ? 1 : 0);\n \t});\n \n \treturn $auth;\n-- \n2.49.0\n\n"},{"id":"514935","messageId":"20250324145332.571813-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250324145332.571813-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v8 2/2] send-email: finer-grained SMTP error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-24T14:53:32Z","receivedAt":"2025-03-24T14:53:59Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Code captured errors but did not process them further.\nThis treated all failures the same without distinguishing SMTP status.\n\nAdd handle-smtp_error to extract SMTP status codes using a regex (as\ndefined in RFC 5321) and handle errors as follows:\n\n- No error present:\n\t- If a result is provided, return 1 to indicate success.\n\t- Otherwise, return 0 to indicate failure.\n\n- Error present with a captured three-digit status code:\n\t- For 4yz (transient errors), return 1 and allow retries.\n\t- For 5yz (permanent errors), return 0 to indicate failure.\n\t- For any other recognized status code, return 1, treating it as\n\ta transient error.\n\n- Error present but no status code found:\n\t- Return 1 as a transient error.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 34 +++++++++++++++++++++++++++++++---\n 1 file changed, 31 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 0f05f55e50..12b1a7c7de 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,14 +1454,42 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n \n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n-\t\t# return 1 when failed due to non-credential reasons\n-\t\treturn $error ? 1 : ($result ? 1 : 0);\n+\t\treturn handle_smtp_error($error, $result);\n \t});\n \n \treturn $auth;\n }\n \n+sub handle_smtp_error {\n+\tmy ($error, $result) = @_;\n+\n+\t# If no error is present, return the result directly\n+\treturn $result ? 1 : 0 unless $error;\n+\n+\t# Check if an error was captured\n+\t# Parse SMTP status code from error message in:\n+\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\tmy $status_code = $1;\n+\t\tif ($status_code =~ /^4/) {\n+\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\twarn \"SMTP transient error (status code $status_code): $error\";\n+\t\t\treturn 1;\n+\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\treturn 0;\n+\t\t}\n+\t\t# If no recognized status code is found, treat as transient error\n+\t\twarn \"SMTP unknown error: $error. Treating as transient failure.\";\n+\t\treturn 1;\n+\t}\n+\n+\t# If no status code is found, treat as transient error\n+\twarn \"SMTP generic error: $error\";\n+\treturn 1;\n+}\n+\n sub ssl_verify_params {\n \teval {\n \t\trequire IO::Socket::SSL;\n-- \n2.49.0\n\n"},{"id":"515029","messageId":"xmqqmsd9m8e6.fsf@gitster.g","threadId":"63097","inReplyTo":"20250324145332.571813-3-05ZYT30@gmail.com","subject":"Re: [GSoC PATCH v8 2/2] send-email: finer-grained SMTP error handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-25T15:34:25Z","receivedAt":"2025-03-25T15:34:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Zheng Yuting <05zyt30@gmail.com> writes:\n\n> -\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n> -\t\t# return 1 when failed due to non-credential reasons\n> -\t\treturn $error ? 1 : ($result ? 1 : 0);\n> +\t\treturn handle_smtp_error($error, $result);\n\nIt was a bit surprising that the new handle-smtp-error sub handles\nthe case without an error.  I would actually have expected for it to\nbe something like:\n\n\t\treturn ($error\n                        ? handle_smtp_error($error)\n\t\t\t: ($result ? 1 : 0));\n\nI.e., we used to unconditionally return 1 upon error, and the only\nchange introduced by this step is to classify $error with the helper\nfunction better and behave differently depending on the error.\n\nHaving said that ...\n\n> +sub handle_smtp_error {\n> +\tmy ($error, $result) = @_;\n> +\n> +\t# If no error is present, return the result directly\n> +\treturn $result ? 1 : 0 unless $error;\n\n... as the \"no error\" case is implemented as an early return, the\nmental burden on the readers is not so bad.  They can concentrate on\nthe error case when reading the remainder of the function.\n\nStill, it would be with even less mental burden if the no-error case\nis handled by the caller to make this function only about error cases.\n\n> +\t# Check if an error was captured\n> +\t# Parse SMTP status code from error message in:\n> +\t# https://www.rfc-editor.org/rfc/rfc5321.html\n> +\tif ($error =~ /\\b(\\d{3})\\b/) {\n> +\t\tmy $status_code = $1;\n> +\t\tif ($status_code =~ /^4/) {\n> +\t\t\t# 4yz: Transient Negative Completion reply\n> +\t\t\twarn \"SMTP transient error (status code $status_code): $error\";\n> +\t\t\treturn 1;\n> +\t\t} elsif ($status_code =~ /^5/) {\n> +\t\t\t# 5yz: Permanent Negative Completion reply\n> +\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t\t# If no recognized status code is found, treat as transient error\n> +\t\twarn \"SMTP unknown error: $error. Treating as transient failure.\";\n> +\t\treturn 1;\n> +\t}\n> +\n> +\t# If no status code is found, treat as transient error\n> +\twarn \"SMTP generic error: $error\";\n> +\treturn 1;\n> +}\n> +\n>  sub ssl_verify_params {\n>  \teval {\n>  \t\trequire IO::Socket::SSL;\n"},{"id":"515082","messageId":"20250326075246.2612627-1-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250324145332.571813-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v9 0/2] send-email: improve error capture and status code handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-26T07:52:44Z","receivedAt":"2025-03-26T07:53:03Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"This patch series improves SMTP authentication error handling.\n\nAuth relied solely on return values without capturing exceptions,\nmisjudging non-credential errors as authentication failures.\n\nPatch v9 1/2 wraps the auth process in an eval {} block to catch all\nexceptions, adds var error for future handling, and var result to return\nauth state.\n\nPatch v9 2/2 introduces finer-grained SMTP error handling by extracting\nstatus codes per RFC 5321. For 4yz (transient) errors, return 1 and allow\nretries; for 5yz (permanent) errors, return 0. Unrecognized or uncaught\nstatus codes are treated as transient errors (return 1). If no error is\npresent and no result is defined, return 1 as a transient error; otherwise,\nreturn the authentication result.\n\n\n Zheng Yuting (2):\n  send-email: capture errors in an eval {} block\n  send-email: finer-grained SMTP error handling\n\n git-send-email.perl | 69 +++++++++++++++++++++++++++++++++++----------\n 1 file changed, 54 insertions(+), 15 deletions(-)\n\n--\n2.49.0\n"},{"id":"515083","messageId":"20250326075246.2612627-2-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250326075246.2612627-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v9 1/2] send-email: capture errors in an eval {} block","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-26T07:52:45Z","receivedAt":"2025-03-26T07:53:06Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Auth relied solely on return values without catching errors. This misjudges\nnon-credential errors as auth failure without error info.\n\nPatch wraps the entire auth process in an eval {} block to catch\nall exceptions, including non-credential errors. It adds a new $error var,\nuses 'or do' to prevent flow break, and returns $result ? 1 : 0. And merges\nif/else branches, integrates SASL and basic auth, with comments for\nfuture status code handling.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 43 +++++++++++++++++++++++++++----------------\n 1 file changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 798d59b84f..0f05f55e50 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1419,7 +1419,7 @@ sub smtp_auth_maybe {\n \t\tdie \"invalid smtp auth: '${smtp_auth}'\";\n \t}\n \n-\t# TODO: Authentication may fail not because credentials were\n+\t# Authentication may fail not because credentials were\n \t# invalid but due to other reasons, in which we should not\n \t# reject credentials.\n \t$auth = Git::credential({\n@@ -1431,21 +1431,32 @@ sub smtp_auth_maybe {\n \t\t'password' => $smtp_authpass\n \t}, sub {\n \t\tmy $cred = shift;\n-\n-\t\tif ($smtp_auth) {\n-\t\t\tmy $sasl = Authen::SASL->new(\n-\t\t\t\tmechanism => $smtp_auth,\n-\t\t\t\tcallback => {\n-\t\t\t\t\tuser => $cred->{'username'},\n-\t\t\t\t\tpass => $cred->{'password'},\n-\t\t\t\t\tauthname => $cred->{'username'},\n-\t\t\t\t}\n-\t\t\t);\n-\n-\t\t\treturn !!$smtp->auth($sasl);\n-\t\t}\n-\n-\t\treturn !!$smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\tmy $result;\n+\t\tmy $error;\n+\n+\t\t# catch all SMTP auth error in a unified eval block\n+\t\teval {\n+\t\t\tif ($smtp_auth) {\n+\t\t\t\tmy $sasl = Authen::SASL->new(\n+\t\t\t\t\tmechanism => $smtp_auth,\n+\t\t\t\t\tcallback => {\n+\t\t\t\t\t\tuser     => $cred->{'username'},\n+\t\t\t\t\t\tpass     => $cred->{'password'},\n+\t\t\t\t\t\tauthname => $cred->{'username'},\n+\t\t\t\t\t}\n+\t\t\t\t);\n+\t\t\t\t$result = $smtp->auth($sasl);\n+\t\t\t} else {\n+\t\t\t\t$result = $smtp->auth($cred->{'username'}, $cred->{'password'});\n+\t\t\t}\n+\t\t\t1; # ensure true value is returned if no exception is thrown\n+\t\t} or do {\n+\t\t\t$error = $@ || 'Unknown error';\n+\t\t};\n+\n+\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n+\t\t# return 1 when failed due to non-credential reasons\n+\t\treturn $error ? 1 : ($result ? 1 : 0);\n \t});\n \n \treturn $auth;\n-- \n2.49.0\n\n"},{"id":"515084","messageId":"20250326075246.2612627-3-05ZYT30@gmail.com","threadId":"63097","inReplyTo":"20250326075246.2612627-1-05ZYT30@gmail.com","subject":"[GSoC PATCH v9 2/2] send-email: finer-grained SMTP error handling","fromName":"Zheng Yuting","fromEmail":"05zyt30@gmail.com","sentAt":"2025-03-26T07:52:46Z","receivedAt":"2025-03-26T07:53:09Z","isPatch":true,"sender":{"key":"05zyt30@gmail.com","avatar":"https://avatars.githubusercontent.com/u/87643662?v=4"},"body":"Code captured errors but did not process them further.\nThis treated all failures the same without distinguishing SMTP status.\n\nAdd handle-smtp_error to extract SMTP status codes using a regex (as\ndefined in RFC 5321) and handle errors as follows:\n\n- No error present:\n\t- If a result is provided, return 1 to indicate success.\n\t- Otherwise, return 0 to indicate failure.\n\n- Error present with a captured three-digit status code:\n\t- For 4yz (transient errors), return 1 and allow retries.\n\t- For 5yz (permanent errors), return 0 to indicate failure.\n\t- For any other recognized status code, return 1, treating it as\n\ta transient error.\n\n- Error present but no status code found:\n\t- Return 1 as a transient error.\n\nSigned-off-by: Zheng Yuting <05ZYT30@gmail.com>\n---\n git-send-email.perl | 32 +++++++++++++++++++++++++++++---\n 1 file changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 0f05f55e50..1f613fa979 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1454,14 +1454,40 @@ sub smtp_auth_maybe {\n \t\t\t$error = $@ || 'Unknown error';\n \t\t};\n \n-\t\t# NOTE: SMTP status code handling will be added in a subsequent commit,\n-\t\t# return 1 when failed due to non-credential reasons\n-\t\treturn $error ? 1 : ($result ? 1 : 0);\n+\t\treturn ($error\n+\t\t\t? handle_smtp_error($error)\n+\t\t\t: ($result ? 1 : 0));\n \t});\n \n \treturn $auth;\n }\n \n+sub handle_smtp_error {\n+\tmy ($error) = @_;\n+\n+\t# Parse SMTP status code from error message in:\n+\t# https://www.rfc-editor.org/rfc/rfc5321.html\n+\tif ($error =~ /\\b(\\d{3})\\b/) {\n+\t\tmy $status_code = $1;\n+\t\tif ($status_code =~ /^4/) {\n+\t\t\t# 4yz: Transient Negative Completion reply\n+\t\t\twarn \"SMTP transient error (status code $status_code): $error\";\n+\t\t\treturn 1;\n+\t\t} elsif ($status_code =~ /^5/) {\n+\t\t\t# 5yz: Permanent Negative Completion reply\n+\t\t\twarn \"SMTP permanent error (status code $status_code): $error\";\n+\t\t\treturn 0;\n+\t\t}\n+\t\t# If no recognized status code is found, treat as transient error\n+\t\twarn \"SMTP unknown error: $error. Treating as transient failure.\";\n+\t\treturn 1;\n+\t}\n+\n+\t# If no status code is found, treat as transient error\n+\twarn \"SMTP generic error: $error\";\n+\treturn 1;\n+}\n+\n sub ssl_verify_params {\n \teval {\n \t\trequire IO::Socket::SSL;\n-- \n2.49.0\n\n"}]}