Zheng Yuting <05zyt30@xxxxxxxxx> writes:
> + if ($error) {
This block is full of overly long lines. Would it make sense to
turn it into a helper sub and do just this here in the block
return handle_smtp_error($error);
And then the remainder of the block would become a single helper
function that is called only from here, losing 2 levels of
indentation.
> + # check if an error was captured
> + # parse SMTP status code from error message in:
> + # https://www.rfc-editor.org/rfc/rfc5321.html
> + if ($error =~ /\b(\d{3})\b/) {
> + my $status_code = $1;
> + if ($status_code =~ /^4/) {
> + # 4yz: Transient Negative Completion reply
> + warn "SMTP temporary error (status code $status_code): $error";
> + return 1;
> + } elsif ($status_code =~ /^5/) {
> + # 5yz: Permanent Negative Completion reply
> + warn "SMTP permanent error (status code $status_code): $error";
> + return 0;
> + } else {
> + # if no recognized status code is found, treat as transient error
> + warn "SMTP unknown error: $error. Treating as permanent failure.";
The comment and warning message say different things. I suspect
that the warning message is wrong, as the branch returns 1 to signal
the caller to do the same thing a the 4yz transient case?
> + return 1;
> + }
> + } else {
> + # if no status code is found, treat as transient error
> + warn "SMTP generic error: $error";
> + return 1;
> + }
> + } elsif (!defined $result) {
> + # if no error and no result is returned, treat as transient error
> + warn "SMTP no result error: $error";
> + return 1;
> + }
> + else {
> + return $result ? 1 : 0;
> + }
> });
>
> return $auth;
