From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 405631FA01 for ; Wed, 13 Oct 2021 07:00:39 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/5] treewide: use warn() or carp() instead of env->{psgi.errors} Date: Wed, 13 Oct 2021 07:00:36 +0000 Message-Id: <20211013070038.6414-4-e@80x24.org> In-Reply-To: <20211013070038.6414-1-e@80x24.org> References: <20211013070038.6414-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Large chunks of our codebase and 3rd-party dependencies do not use ->{psgi.errors}, so trying to standardize on it was a fruitless endeavor. Since warn() and carp() are standard mechanism within Perl, just use that instead and simplify a bunch of existing code. --- lib/PublicInbox/GitHTTPBackend.pm | 39 +++++++------------------------ lib/PublicInbox/GzipFilter.pm | 3 +-- lib/PublicInbox/HTTP.pm | 16 +++++-------- lib/PublicInbox/LeiBlob.pm | 1 - lib/PublicInbox/LeiRediff.pm | 1 - lib/PublicInbox/LeiXSearch.pm | 19 +++++++-------- lib/PublicInbox/Qspawn.pm | 13 ++++------- lib/PublicInbox/SearchView.pm | 7 ++---- lib/PublicInbox/Unsubscribe.pm | 5 +--- lib/PublicInbox/ViewVCS.pm | 6 ++--- lib/PublicInbox/WwwAltId.pm | 3 +-- lib/PublicInbox/WwwHighlight.pm | 2 +- 12 files changed, 36 insertions(+), 79 deletions(-) diff --git a/lib/PublicInbox/GitHTTPBackend.pm b/lib/PublicInbox/GitHTTPBackend.pm index b9a3e4f5b93e..ba3a8f208949 100644 --- a/lib/PublicInbox/GitHTTPBackend.pm +++ b/lib/PublicInbox/GitHTTPBackend.pm @@ -5,13 +5,14 @@ # or smart HTTP. This is our wrapper for git-http-backend(1) package PublicInbox::GitHTTPBackend; use strict; -use warnings; +use v5.10.1; use Fcntl qw(:seek); use IO::Handle; # ->flush use HTTP::Date qw(time2str); use PublicInbox::Qspawn; use PublicInbox::Tmpfile; use PublicInbox::WwwStatic qw(r @NO_CACHE); +use Carp (); # 32 is same as the git-daemon connection limit my $default_limiter = PublicInbox::Qspawn::Limiter->new(32); @@ -45,10 +46,7 @@ sub serve { serve_dumb($env, $git, $path); } -sub err ($@) { - my ($env, @msg) = @_; - $env->{'psgi.errors'}->print(@msg, "\n"); -} +sub ucarp { Carp::carp(@_); undef } my $prev = 0; my $exp; @@ -118,37 +116,18 @@ sub input_prepare { my $input = $env->{'psgi.input'}; my $fd = eval { fileno($input) }; - if (defined $fd && $fd >= 0) { - return { 0 => $fd }; - } + return { 0 => $fd } if (defined $fd && $fd >= 0); my $id = "git-http.input.$env->{REMOTE_ADDR}:$env->{REMOTE_PORT}"; - my $in = tmpfile($id); - unless (defined $in) { - err($env, "could not open temporary file: $!"); - return; - } + my $in = tmpfile($id) // return ucarp("tmpfile: $!"); my $buf; while (1) { - my $r = $input->read($buf, 8192); - unless (defined $r) { - err($env, "error reading input: $!"); - return; - } + my $r = $input->read($buf, 8192) // return ucarp("read $!"); last if $r == 0; - unless (print $in $buf) { - err($env, "error writing temporary file: $!"); - return; - } + print $in $buf // return ucarp("print: $!"); } # ensure it's visible to git-http-backend(1): - unless ($in->flush) { - err($env, "error writing temporary file: $!"); - return; - } - unless (defined(sysseek($in, 0, SEEK_SET))) { - err($env, "error seeking temporary file: $!"); - return; - } + $in->flush // return ucarp("flush: $!"); + sysseek($in, 0, SEEK_SET) // return ucarp($env, "seek: $!"); { 0 => $in }; } diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm index 624c2ed3418c..c62161710725 100644 --- a/lib/PublicInbox/GzipFilter.pm +++ b/lib/PublicInbox/GzipFilter.pm @@ -154,8 +154,7 @@ sub close { sub bail { my $self = shift; if (my $env = $self->{env}) { - eval { $env->{'psgi.errors'}->print(@_, "\n") }; - warn("E: error printing to psgi.errors: $@", @_) if $@; + warn @_, "\n"; my $http = $env->{'psgix.io'} or return; # client abort eval { $http->close }; # should hit our close warn "E: error in http->close: $@" if $@; diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm index 82c2b200c4dd..8a89dd73b031 100644 --- a/lib/PublicInbox/HTTP.pm +++ b/lib/PublicInbox/HTTP.pm @@ -171,7 +171,7 @@ sub app_dispatch { } }; if ($@) { - err($self, "response_write error: $@"); + warn "response_write error: $@"; $self->close; } } @@ -285,14 +285,14 @@ sub getline_pull { return; # likely } } elsif ($@) { - err($self, "response ->getline error: $@"); + warn "response ->getline error: $@"; $self->close; } # avoid recursion if (delete $self->{forward}) { eval { $forward->close }; if ($@) { - err($self, "response ->close error: $@"); + warn "response ->close error: $@"; $self->close; # idempotent } } @@ -360,15 +360,11 @@ sub input_prepare { sub env_chunked { ($_[0]->{HTTP_TRANSFER_ENCODING} // '') =~ /\Achunked\z/i } -sub err ($$) { - eval { $_[0]->{httpd}->{env}->{'psgi.errors'}->print($_[1]."\n") }; -} - sub write_err { my ($self, $len) = @_; my $msg = $! || '(zero write)'; $msg .= " ($len bytes remaining)" if defined $len; - err($self, "error buffering to input: $msg"); + warn "error buffering to input: $msg"; quit($self, 500); } @@ -377,7 +373,7 @@ sub recv_err { if ($! == EAGAIN) { # epoll/kevent watch already set by do_read $self->{input_left} = $len; } else { - err($self, "error reading input: $! ($len bytes remaining)"); + warn "error reading input: $! ($len bytes remaining)"; } } @@ -458,7 +454,7 @@ sub close { my $self = $_[0]; if (my $forward = delete $self->{forward}) { eval { $forward->close }; - err($self, "forward ->close error: $@") if $@; + warn "forward ->close error: $@" if $@; } $self->SUPER::close; # PublicInbox::DS::close } diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm index a3ddbbcec211..b6a62d246657 100644 --- a/lib/PublicInbox/LeiBlob.pm +++ b/lib/PublicInbox/LeiBlob.pm @@ -73,7 +73,6 @@ sub do_solve_blob { # via wq_do # -cur_di, -qsp, -msg => temporary fields for Qspawn callbacks inboxes => [ $self->{lxs}->locals, @rmt ], }, 'PublicInbox::SolverGit'; - $lei->{env}->{'psgi.errors'} = $lei->{2}; # ugh... local $PublicInbox::DS::in_loop = 0; # waitpid synchronously $solver->solve($lei->{env}, $log, $self->{oid_b}, $hints); } diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm index 740cbcee334f..56c457fc4693 100644 --- a/lib/PublicInbox/LeiRediff.pm +++ b/lib/PublicInbox/LeiRediff.pm @@ -303,7 +303,6 @@ sub ipc_atfork_child { $self->{gits} = [ map { PublicInbox::Git->new($lei->rel2abs($_)) } @{$self->{lei}->{opt}->{'git-dir'}} ]; - $lei->{env}->{'psgi.errors'} = $lei->{2}; # ugh... $lei->{env}->{TMPDIR} = $self->{rdtmp}->dirname; if (my $nr = ($lei->{opt}->{drq} || $lei->{opt}->{'dequote-only'})) { my $re = '\s*> ' x $nr; diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm index fee1b859f9c6..ee9216feeb23 100644 --- a/lib/PublicInbox/LeiXSearch.pm +++ b/lib/PublicInbox/LeiXSearch.pm @@ -33,7 +33,7 @@ sub new { sub attach_external { my ($self, $ibxish) = @_; # ibxish = ExtSearch or Inbox my $desc = $ibxish->{inboxdir} // $ibxish->{topdir}; - my $srch = $ibxish->search or + my $srch = $ibxish->search // return warn("$desc not indexed for Xapian ($@ $!)\n"); my @shards = $srch->xdb_shards_flat or return warn("$desc has no Xapian shards\n"); @@ -184,11 +184,10 @@ sub query_one_mset { # for --threads and l2m w/o sort my $maxk = "external.$dir.maxuid"; my $stop_at = $lss ? $lss->{-cfg}->{$maxk} : undef; if (defined $stop_at) { - die "$maxk=$stop_at has multiple values" if ref $stop_at; - my @e; - local $SIG{__WARN__} = sub { push @e, @_ }; - $stop_at += 0; - return warn("$maxk=$stop_at: @e") if @e; + ref($stop_at) and + return warn("$maxk=$stop_at has multiple values\n"); + ($stop_at =~ /[^0-9]/) and + return warn("$maxk=$stop_at not numeric\n"); } my $first_ids; do { @@ -392,12 +391,11 @@ sub query_remote_mboxrd { } $err = ''; if (-s $cerr) { - seek($cerr, 0, SEEK_SET) or + seek($cerr, 0, SEEK_SET) // warn "seek($cmd stderr): $!"; $err = do { local $/; <$cerr> } // warn "read($cmd stderr): $!"; - truncate($cerr, 0) or - warn "truncate($cmd stderr): $!"; + truncate($cerr, 0) // warn "truncate($cmd stderr): $!"; } next if (($? >> 8) == 22 && $err =~ /\b404\b/); $uri->query_form(q => $qstr); @@ -423,9 +421,8 @@ sub query_done { # EOF callback for main daemon if (my $lxs = delete $lei->{lxs}) { $lxs->wq_wait_old(\&xsearch_done_wait, $lei); } - if ($lei->{opt}->{'mail-sync'} && !$lei->{sto}) { + ($lei->{opt}->{'mail-sync'} && !$lei->{sto}) and warn "BUG: {sto} missing with --mail-sync"; - } $lei->sto_done_request if $lei->{sto}; my $wait = $lei->{v2w} ? $lei->{v2w}->wq_do('done') : undef; $lei->{ovv}->ovv_end($lei); diff --git a/lib/PublicInbox/Qspawn.pm b/lib/PublicInbox/Qspawn.pm index b1285eda4a83..a1ff65b65324 100644 --- a/lib/PublicInbox/Qspawn.pm +++ b/lib/PublicInbox/Qspawn.pm @@ -76,11 +76,6 @@ sub child_err ($) { $msg; } -sub log_err ($$) { - my ($env, $msg) = @_; - $env->{'psgi.errors'}->print($msg, "\n"); -} - sub finalize ($$) { my ($self, $err) = @_; @@ -104,7 +99,7 @@ sub finalize ($$) { $self->{err} = $err; } if ($env && $self->{cmd}) { - log_err($env, join(' ', @{$self->{cmd}}) . ": $err"); + warn join(' ', @{$self->{cmd}}) . ": $err"; } } if ($qx_cb) { @@ -188,7 +183,7 @@ sub psgi_qx { # PSGI servers. sub event_step { my ($self, $err) = @_; # $err: $! - log_err($self->{psgi_env}, "psgi_{return,qx} $err") if defined($err); + warn "psgi_{return,qx} $err" if defined($err); finish($self); my ($fh, $qx_fh) = delete(@$self{qw(fh qx_fh)}); $fh->close if $fh; # async-only (psgi_return) @@ -210,14 +205,14 @@ sub rd_hdr ($) { $total_rd += $r; eval { $ret = $ph_cb->($total_rd, $hdr_buf, $ph_arg) }; if ($@) { - log_err($self->{psgi_env}, "parse_hdr: $@"); + warn "parse_hdr: $@"; $ret = [ 500, [], [ "Internal error\n" ] ]; } } else { # caller should notify us when it's ready: return if $! == EAGAIN; next if $! == EINTR; # immediate retry - log_err($self->{psgi_env}, "error reading header: $!"); + warn "error reading header: $!"; $ret = [ 500, [], [ "Internal error\n" ] ]; } } until (defined $ret); diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm index e74ddb9056c8..a42867c5f577 100644 --- a/lib/PublicInbox/SearchView.pm +++ b/lib/PublicInbox/SearchView.pm @@ -102,11 +102,8 @@ sub mset_summary { foreach my $m ($mset->items) { my $num = shift @nums; - my $smsg = delete($num2msg{$num}) or do { - eval { - $m = "$m $num expired\n"; - $ctx->{env}->{'psgi.errors'}->print($m); - }; + my $smsg = delete($num2msg{$num}) // do { + warn "$m $num expired\n"; next; }; my $mid = $smsg->{mid}; diff --git a/lib/PublicInbox/Unsubscribe.pm b/lib/PublicInbox/Unsubscribe.pm index d583b9c9831b..ddbd7a2e52cc 100644 --- a/lib/PublicInbox/Unsubscribe.pm +++ b/lib/PublicInbox/Unsubscribe.pm @@ -81,10 +81,7 @@ sub _user_list_addr { } my $user = eval { $self->{cipher}->decrypt(decode_base64url($u)) }; if (!defined $user || index($user, '@') < 1) { - my $err = quotemeta($@); - my $errors = $env->{'psgi.errors'}; - $errors->print("error decrypting: $u\n"); - $errors->print("$_\n") for split("\n", $err); + warn "error decrypting: $u: ", ($@ ? quotemeta($@) : ()); $u = Plack::Util::encode_html($u); return r($self, 400, 'Bad request', "Failed to decrypt: $u"); } diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm index 6365f04547bc..021b70cf2180 100644 --- a/lib/PublicInbox/ViewVCS.pm +++ b/lib/PublicInbox/ViewVCS.pm @@ -54,7 +54,7 @@ sub stream_blob_parse_hdr { # {parse_hdr} for Qspawn 'text/plain; charset=UTF-8', @cl ] ]; } if ($r == 0) { - warn "premature EOF on $oid $$logref\n"; + warn "premature EOF on $oid $$logref"; return html_page($ctx, 500, $logref); } @$ctx{qw(-res -logref)} = ($res, $logref); @@ -111,7 +111,7 @@ sub solve_result { my ($log, $hints, $fn) = delete @$ctx{qw(log hints fn)}; unless (seek($log, 0, 0)) { - $ctx->{env}->{'psgi.errors'}->print("seek(log): $!\n"); + warn "seek(log): $!"; return html_page($ctx, 500, \'seek error'); } $log = do { local $/; <$log> }; @@ -138,7 +138,7 @@ sub solve_result { my $blob = $git->cat_file($oid); if (!$blob) { # WTF? my $e = "Failed to retrieve generated blob ($oid)"; - $ctx->{env}->{'psgi.errors'}->print("$e ($git->{git_dir})\n"); + warn "$e ($git->{git_dir})"; $log = "
$e
" . $log; return html_page($ctx, 500, \$log); } diff --git a/lib/PublicInbox/WwwAltId.pm b/lib/PublicInbox/WwwAltId.pm index bf8519848cc5..e107dfe06eaf 100644 --- a/lib/PublicInbox/WwwAltId.pm +++ b/lib/PublicInbox/WwwAltId.pm @@ -15,8 +15,7 @@ sub check_output { my ($r, $bref, $ctx) = @_; return html_oneshot($ctx, 500) if !defined($r); if ($r == 0) { - my $err = eval { $ctx->{env}->{'psgi.errors'} } // \*STDERR; - $err->print("unexpected EOF from sqlite3\n"); + warn 'unexpected EOF from sqlite3'; return html_oneshot($ctx, 501); } [200, [ qw(Content-Type application/gzip), 'Content-Disposition', diff --git a/lib/PublicInbox/WwwHighlight.pm b/lib/PublicInbox/WwwHighlight.pm index 3593c2d49e3f..75338806a449 100644 --- a/lib/PublicInbox/WwwHighlight.pm +++ b/lib/PublicInbox/WwwHighlight.pm @@ -46,7 +46,7 @@ sub read_in_full ($) { return \$buf if $r == 0; $off += $r; } - $env->{'psgi.errors'}->print("input read error: $!\n"); + warn "input read error: $!"; undef; }