* [PATCH 1/5] extindex: show OID on bad blob failure
2021-10-13 7:00 [PATCH 0/5] warnings and such Eric Wong
@ 2021-10-13 7:00 ` Eric Wong
2021-10-13 7:00 ` [PATCH 2/5] lei: use standard warn() in more places Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-13 7:00 UTC (permalink / raw)
To: meta
AFAIK I've never hit these messages, but I might be glad
if I ever do.
---
lib/PublicInbox/ExtSearchIdx.pm | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index 40489eab4c66..ddb16241e23d 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -595,11 +595,11 @@ sub _reindex_oid { # git->cat_async callback
my $oidbin = pack('H*', $expect_oid);
my $remain = _unref_doc($sync, $docid, undef, undef, $oidbin);
if ($remain == 0) {
- warn "W: #$docid gone or corrupted\n";
+ warn "W: #$docid ($oid) gone or corrupt\n";
} elsif (my $next_oid = $req->{xr3r}->[++$req->{ix}]->[2]) {
$self->git->cat_async($next_oid, \&_reindex_oid, $req);
} else {
- warn "BUG: #$docid gone (UNEXPECTED)\n";
+ warn "BUG: #$docid ($oid) gone (UNEXPECTED)\n";
}
return;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/5] lei: use standard warn() in more places
2021-10-13 7:00 [PATCH 0/5] warnings and such Eric Wong
2021-10-13 7:00 ` [PATCH 1/5] extindex: show OID on bad blob failure Eric Wong
@ 2021-10-13 7:00 ` Eric Wong
2021-10-13 7:00 ` [PATCH 3/5] treewide: use warn() or carp() instead of env->{psgi.errors} Eric Wong
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-13 7:00 UTC (permalink / raw)
To: meta
warn() is easier to augment with context information, and
frankly unavoidable in the presence of 3rd-party libraries
we don't control.
---
lib/PublicInbox/Fetch.pm | 2 +-
lib/PublicInbox/LEI.pm | 18 +++++++++++++-----
lib/PublicInbox/LeiBlob.pm | 2 +-
lib/PublicInbox/LeiConvert.pm | 2 +-
lib/PublicInbox/LeiForgetExternal.pm | 5 ++---
lib/PublicInbox/LeiImport.pm | 7 +++----
lib/PublicInbox/LeiImportKw.pm | 2 +-
lib/PublicInbox/LeiInput.pm | 4 ++--
lib/PublicInbox/LeiMailSync.pm | 4 ++--
lib/PublicInbox/LeiMirror.pm | 8 ++++----
lib/PublicInbox/LeiP2q.pm | 2 +-
lib/PublicInbox/LeiRediff.pm | 2 +-
lib/PublicInbox/LeiRemote.pm | 2 +-
lib/PublicInbox/LeiViewText.pm | 2 +-
lib/PublicInbox/LeiXSearch.pm | 25 +++++++++++++++----------
15 files changed, 49 insertions(+), 38 deletions(-)
diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index 5ada1f49e4dc..e5756fb664e4 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -52,7 +52,7 @@ sub do_manifest ($$$) {
$m0 = eval {
PublicInbox::LeiMirror::decode_manifest($fh, $mf, $mf)
};
- $lei->err($@) if $@;
+ warn($@) if $@;
}
my ($bn) = ($fn =~ m!/([^/]+)\z!);
my $curl_cmd = $lei->{curl}->for_uri($lei, $muri, qw(-R -o), $bn);
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 183cb545fe55..bd8a6bef632b 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -471,6 +471,7 @@ sub _drop_wq {
# pronounced "exit": x_it(1 << 8) => exit(1); x_it(13) => SIGPIPE
sub x_it ($$) {
my ($self, $code) = @_;
+ local $current_lei = $self;
# make sure client sees stdout before exit
$self->{1}->autoflush(1) if $self->{1};
stop_pager($self);
@@ -504,6 +505,7 @@ sub qfin { # show message on finalization (LeiFinmsg)
sub fail_handler ($;$$) {
my ($lei, $code, $io) = @_;
+ local $current_lei = $lei;
close($io) if $io; # needed to avoid warnings on SIGPIPE
_drop_wq($lei);
x_it($lei, $code // (1 << 8));
@@ -521,6 +523,7 @@ sub sigint_reap {
sub fail ($$;$) {
my ($self, $buf, $exit_code) = @_;
+ local $current_lei = $self;
$self->{failed}++;
warn($buf, "\n") if defined $buf;
$self->{pkt_op_p}->pkt_do('fail_handler') if $self->{pkt_op_p};
@@ -541,6 +544,7 @@ sub puts ($;@) { out(shift, map { "$_\n" } @_) }
sub child_error { # passes non-fatal curl exit codes to user
my ($self, $child_error, $msg) = @_; # child_error is $?
+ local $current_lei = $self;
$child_error ||= 1 << 8;
warn($msg, "\n") if defined $msg;
if ($self->{pkt_op_p}) { # to top lei-daemon
@@ -1019,7 +1023,7 @@ sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
$cmd = [ Text::ParseWords::shellwords($cmd) ];
send($sock, exec_buf($cmd, {}), MSG_EOR) if $sock;
} else {
- err($self, "W: unsupported --alert=$op"); # non-fatal
+ warn("W: unsupported --alert=$op\n"); # non-fatal
}
}
}
@@ -1068,7 +1072,7 @@ sub start_pager {
# display a message for user before spawning full-screen $VISUAL
sub pgr_err {
my ($self, @msg) = @_;
- return $self->err(@msg) unless $self->{sock} && -t $self->{2};
+ return warn(@msg) unless $self->{sock} && -t $self->{2};
start_pager($self, { LESS => 'RX' }); # no 'F' so we prompt
print { $self->{2} } @msg;
$self->{2}->autoflush(1);
@@ -1118,6 +1122,7 @@ sub accept_dispatch { # Listener {post_accept} callback
sub dclose {
my ($self) = @_;
+ local $current_lei = $self;
delete $self->{-progress};
_drop_wq($self) if $self->{failed};
$self->close if $self->{-event_init_done}; # PublicInbox::DS::close
@@ -1369,6 +1374,7 @@ sub DESTROY {
sub wq_done_wait { # dwaitpid callback
my ($arg, $pid) = @_;
my ($wq, $lei) = @$arg;
+ local $current_lei = $lei;
my $err_type = $lei->{-err_type};
$? and $lei->child_error($?,
$err_type ? "$err_type errors during $lei->{cmd}" : ());
@@ -1383,6 +1389,7 @@ sub fchdir {
sub wq_eof { # EOF callback for main daemon
my ($lei) = @_;
+ local $current_lei = $lei;
my $wq1 = delete $lei->{wq1} // return $lei->fail; # already failed
$wq1->wq_wait_old($wq1->can('_wq_done_wait') // \&wq_done_wait, $lei);
}
@@ -1423,7 +1430,7 @@ sub refresh_watches {
$seen{$url} = undef;
my $state = $cfg->get_1("watch.$url.state");
if (!watch_state_ok($state)) {
- $lei->err("watch.$url.state=$state not supported");
+ warn("watch.$url.state=$state not supported\n");
next;
}
if ($url =~ /\Amaildir:(.+)/i) {
@@ -1492,6 +1499,7 @@ sub lms {
sub sto_done_request {
my ($lei, $sock) = @_;
+ local $current_lei = $lei;
eval {
if ($sock //= $lei->{sock}) { # issue, async wait
$lei->{sto}->wq_io_do('done', [ $sock ]);
@@ -1499,14 +1507,14 @@ sub sto_done_request {
my $wait = $lei->{sto}->wq_do('done');
}
};
- $lei->err($@) if $@;
+ warn($@) if $@;
}
sub cfg_dump ($$) {
my ($lei, $f) = @_;
my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
return $ret if !$@;
- $lei->err($@);
+ warn($@);
undef;
}
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index b94f67a029ff..a3ddbbcec211 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -40,7 +40,7 @@ sub solver_user_cb { # called by solver when done
# don't try to support all the git-show(1) options for non-blob,
# this is just a convenience:
$type ne 'blob' and
- $lei->err("# $oid is a $type of $size bytes in:\n#\t$gd");
+ warn "# $oid is a $type of $size bytes in:\n#\t$gd\n";
my $cmd = [ 'git', "--git-dir=$gd", 'show', $oid ];
my $rdr = { 1 => $lei->{1}, 2 => $lei->{2} };
diff --git a/lib/PublicInbox/LeiConvert.pm b/lib/PublicInbox/LeiConvert.pm
index 6550c242b1a2..9e98edc391a0 100644
--- a/lib/PublicInbox/LeiConvert.pm
+++ b/lib/PublicInbox/LeiConvert.pm
@@ -41,7 +41,7 @@ sub process_inputs { # via wq_do
delete $lei->{1};
delete $self->{wcb}; # commit
my $nr = delete($lei->{-nr_write}) // 0;
- $lei->err("# converted $nr messages") if $lei->{opt}->{verbose};
+ $lei->qerr("# converted $nr messages");
}
sub lei_convert { # the main "lei convert" method
diff --git a/lib/PublicInbox/LeiForgetExternal.pm b/lib/PublicInbox/LeiForgetExternal.pm
index 7a4bbcf80a3a..07f0ac803c88 100644
--- a/lib/PublicInbox/LeiForgetExternal.pm
+++ b/lib/PublicInbox/LeiForgetExternal.pm
@@ -20,10 +20,9 @@ sub lei_forget_external {
if ($? == 0) {
$lei->qerr("# $l forgotten ");
} elsif (($? >> 8) == 5) {
- $lei->err("# $l not found");
+ warn("# $l not found\n");
} else {
- $lei->err("# --unset $key error");
- return $lei->x_it($?);
+ $lei->child_error($?, "# --unset $key error");
}
}
}
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 69d63ab6b397..2f8fd6c64f69 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -38,7 +38,7 @@ sub pmdir_cb { # called via wq_io_do from LeiPmdir->each_mdir_fn
my $lse = $self->{lse} //= $self->{lei}->{sto}->search;
my $lms = $self->{-lms_ro} //= $self->{lei}->lms; # may be 0 or undef
my @oidbin = $lms ? $lms->name_oidbin($folder, $bn) : ();
- @oidbin > 1 and $self->{lei}->err("W: $folder/*/$$bn not unique:\n",
+ @oidbin > 1 and warn("W: $folder/*/$$bn not unique:\n",
map { "\t".unpack('H*', $_)."\n" } @oidbin);
my %seen;
my @docids = sort { $a <=> $b } grep { !$seen{$_}++ }
@@ -100,9 +100,8 @@ sub do_import_index ($$@) {
my $nproc = $self->detect_nproc;
$j = $nproc if $j > $nproc;
}
- if ($lei->{opt}->{'new-only'} && (!$net || !$net->{imap_order})) {
- $lei->err('# --new-only is only for IMAP');
- }
+ ($lei->{opt}->{'new-only'} && (!$net || !$net->{imap_order})) and
+ warn "# --new-only is only for IMAP\n";
my $ops = {};
$lei->{auth}->op_merge($ops, $self) if $lei->{auth};
$lei->{-eml_noisy} = 1;
diff --git a/lib/PublicInbox/LeiImportKw.pm b/lib/PublicInbox/LeiImportKw.pm
index c35c5c266c4b..8359f3386668 100644
--- a/lib/PublicInbox/LeiImportKw.pm
+++ b/lib/PublicInbox/LeiImportKw.pm
@@ -36,7 +36,7 @@ sub ck_update_kw { # via wq_io_do
my ($self, $url, $uid, $kw) = @_;
my @oidbin = $self->{-lms_ro}->num_oidbin($url, $uid);
my $uid_url = "$url/;UID=$uid";
- @oidbin > 1 and $self->{lei}->err("W: $uid_url not unique:\n",
+ @oidbin > 1 and warn("W: $uid_url not unique:\n",
map { "\t".unpack('H*', $_)."\n" } @oidbin);
my %seen;
my @docids = sort { $a <=> $b } grep { !$seen{$_}++ }
diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index 83479221000b..6a90e7e1e756 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -343,8 +343,8 @@ $input is `eml', not --in-format=$in_fmt
--mail-sync specified but no inputs support it
# non-fatal if some inputs support support sync
- $lei->err("# --mail-sync will only be used for @{$sync->{ok}}");
- $lei->err("# --mail-sync is not supported for: @{$sync->{no}}");
+ warn("# --mail-sync will only be used for @{$sync->{ok}}\n");
+ warn("# --mail-sync is not supported for: @{$sync->{no}}\n");
}
if ($net) {
$net->{-can_die} = 1;
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index f7e37ad9ca80..f2f1e3ed2658 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -466,7 +466,7 @@ sub arg2folder {
# using `$res' instead of `$orig'
EOM
} else {
- $lei->err($res) if defined $res;
+ warn($res, "\n") if defined $res;
push @no, $orig;
}
} elsif (m!\A(?:nntps?|s?news)://!i) {
@@ -478,7 +478,7 @@ EOM
# using `$res' instead of `$orig'
EOM
} else {
- $lei->err($res) if defined $res;
+ warn($res, "\n") if defined $res;
push @no, $orig;
}
} else {
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 4be8f70ae401..ec41bec6f16b 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -20,7 +20,7 @@ sub _wq_done_wait { # dwaitpid callback (via wq_eof)
if ($?) {
$lei->child_error($?);
} elsif (!unlink($f)) {
- $lei->err("unlink($f): $!") unless $!{ENOENT};
+ warn("unlink($f): $!\n") unless $!{ENOENT};
} else {
if ($lei->{cmd} ne 'public-inbox-clone') {
$lei->lazy_cb('add-external', '_finish_'
@@ -120,7 +120,7 @@ sub _try_config {
-d $dst or die "mkpath($dst): $!\n";
}
my $err = _get_txt($self, qw(_/text/config/raw inbox.config.example));
- return $self->{lei}->err($err) if $err;
+ return warn($err, "\n") if $err;
my $f = "$self->{dst}/inbox.config.example";
chmod((stat($f))[2] & 0444, $f) or die "chmod(a-w, $f): $!";
my $cfg = PublicInbox::Config->git_config_dump($f, $self->{lei}->{2});
@@ -151,7 +151,7 @@ sub index_cloned_inbox {
my ($self, $iv) = @_;
my $lei = $self->{lei};
my $err = _get_txt($self, qw(description description));
- $lei->err($err) if $err; # non fatal
+ warn($err, "\n") if $err; # non fatal
eval { set_description($self) };
warn $@ if $@;
@@ -380,7 +380,7 @@ sub try_manifest {
my ($path_pfx, $v1_path, @v2_epochs) = deduce_epochs($m, $path);
if (@v2_epochs) {
# It may be possible to have v1 + v2 in parallel someday:
- $lei->err(<<EOM) if defined $v1_path;
+ warn(<<EOM) if defined $v1_path;
# `$v1_path' appears to be a v1 inbox while v2 epochs exist:
# @v2_epochs
# ignoring $v1_path (use --inbox-version=1 to force v1 instead)
diff --git a/lib/PublicInbox/LeiP2q.pm b/lib/PublicInbox/LeiP2q.pm
index c0c4563d6fbb..5c2ce0a13dfc 100644
--- a/lib/PublicInbox/LeiP2q.pm
+++ b/lib/PublicInbox/LeiP2q.pm
@@ -135,7 +135,7 @@ sub do_p2q { # via wq_do
if ($lei->{opt}->{debug}) {
my $json = ref(PublicInbox::Config->json)->new;
$json->utf8->canonical->pretty;
- $lei->err($json->encode($lei->{qterms}));
+ print { $lei->{2} } $json->encode($lei->{qterms});
}
my (@q, %seen);
for my $pfx (@want) {
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index decb721b7a4c..740cbcee334f 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -30,7 +30,7 @@ sub rediff_user_cb { # called by solver when done
# don't try to support all the git-show(1) options for non-blob,
# this is just a convenience:
- $type ne 'blob' and return $lei->err(<<EOF);
+ $type ne 'blob' and return warn(<<EOF);
# $oid is a $type of $size bytes in:
# $git->{git_dir} (wanted: $oid_want)
EOF
diff --git a/lib/PublicInbox/LeiRemote.pm b/lib/PublicInbox/LeiRemote.pm
index 346aa6a40132..7782aa9dbfa1 100644
--- a/lib/PublicInbox/LeiRemote.pm
+++ b/lib/PublicInbox/LeiRemote.pm
@@ -75,7 +75,7 @@ sub smsg_eml {
if (my $bref = $self->{lei}->ale->git->cat_file($smsg->{blob})) {
return PublicInbox::Eml->new($bref);
}
- $self->{lei}->err("E: $self->{uri} $smsg->{blob} gone <$smsg->{mid}>");
+ warn("E: $self->{uri} $smsg->{blob} gone <$smsg->{mid}>\n");
undef;
}
diff --git a/lib/PublicInbox/LeiViewText.pm b/lib/PublicInbox/LeiViewText.pm
index c469d1ea5b1c..2dad3b780fa1 100644
--- a/lib/PublicInbox/LeiViewText.pm
+++ b/lib/PublicInbox/LeiViewText.pm
@@ -77,7 +77,7 @@ sub new {
my $cfg = PublicInbox::Config::config_fh_parse($r, "\0", "\n");
waitpid($pid, 0);
if ($?) {
- $lei->err("# git-config failed, no color (non-fatal)");
+ warn "# git-config failed, no color (non-fatal)\n";
return $self;
}
$self->{-colored} = \&my_colored;
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index ae9f5881676d..fee1b859f9c6 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -16,6 +16,7 @@ use PublicInbox::Spawn qw(popen_rd spawn which);
use PublicInbox::MID qw(mids);
use PublicInbox::Smsg;
use PublicInbox::Eml;
+use PublicInbox::LEI;
use Fcntl qw(SEEK_SET F_SETFL O_APPEND O_RDWR);
use PublicInbox::ContentHash qw(git_sha);
use POSIX qw(strftime);
@@ -392,11 +393,11 @@ sub query_remote_mboxrd {
$err = '';
if (-s $cerr) {
seek($cerr, 0, SEEK_SET) or
- $lei->err("seek($cmd stderr): $!");
+ warn "seek($cmd stderr): $!";
$err = do { local $/; <$cerr> } //
- "read($cmd stderr): $!";
+ warn "read($cmd stderr): $!";
truncate($cerr, 0) or
- $lei->err("truncate($cmd stderr): $!");
+ warn "truncate($cmd stderr): $!";
}
next if (($? >> 8) == 22 && $err =~ /\b404\b/);
$uri->query_form(q => $qstr);
@@ -416,6 +417,7 @@ sub xsearch_done_wait { # dwaitpid callback
sub query_done { # EOF callback for main daemon
my ($lei) = @_;
+ local $PublicInbox::LEI::current_lei = $lei;
my $l2m = delete $lei->{l2m};
$l2m->wq_wait_old(\&xsearch_done_wait, $lei) if $l2m;
if (my $lxs = delete $lei->{lxs}) {
@@ -462,6 +464,7 @@ Error closing $lei->{ovv}->{dst}: $!
sub do_post_augment {
my ($lei) = @_;
+ local $PublicInbox::LEI::current_lei = $lei;
my $l2m = $lei->{l2m} or return; # client disconnected
$lei->fchdir or return;
my $err;
@@ -497,9 +500,10 @@ sub concurrency {
$nl + $nr;
}
-sub start_query ($;$) { # always runs in main (lei-daemon) process
- my ($self, $l2m) = @_;
- if ($self->{opt_threads} || ($l2m && !$self->{opt_sort})) {
+sub start_query ($$) { # always runs in main (lei-daemon) process
+ my ($self, $lei) = @_;
+ local $PublicInbox::LEI::current_lei = $lei;
+ if ($self->{opt_threads} || ($lei->{l2m} && !$self->{opt_sort})) {
for my $ibxish (locals($self)) {
$self->wq_io_do('query_one_mset', [], $ibxish);
}
@@ -521,9 +525,10 @@ sub start_query ($;$) { # always runs in main (lei-daemon) process
}
sub incr_start_query { # called whenever an l2m shard starts do_post_auth
- my ($self, $l2m) = @_;
+ my ($self, $lei) = @_;
+ my $l2m = $lei->{l2m};
return if ++$self->{nr_start_query} != $l2m->{-wq_nr_workers};
- start_query($self, $l2m);
+ start_query($self, $lei);
}
sub ipc_atfork_child {
@@ -545,7 +550,7 @@ sub do_query {
'l2m_progress' => [ \&l2m_progress, $lei ],
'x_it' => [ $lei ],
'child_error' => [ $lei ],
- 'incr_start_query' => [ $self, $l2m ],
+ 'incr_start_query' => [ $self, $lei ],
};
$lei->{auth}->op_merge($ops, $l2m) if $l2m && $lei->{auth};
my $end = $lei->pkt_op_pair;
@@ -586,7 +591,7 @@ sub do_query {
if ($l2m) {
$l2m->net_merge_all_done($lei) unless $lei->{auth};
} else {
- start_query($self);
+ start_query($self, $lei);
}
$lei->event_step_init; # wait for shutdowns
$lei->wait_wq_events($op_c, $ops);
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/5] treewide: use warn() or carp() instead of env->{psgi.errors}
2021-10-13 7:00 [PATCH 0/5] warnings and such Eric Wong
2021-10-13 7:00 ` [PATCH 1/5] extindex: show OID on bad blob failure Eric Wong
2021-10-13 7:00 ` [PATCH 2/5] lei: use standard warn() in more places Eric Wong
@ 2021-10-13 7:00 ` Eric Wong
2021-10-13 7:00 ` [PATCH 4/5] extindex: set {current_info} in eidxq processing Eric Wong
2021-10-13 7:00 ` [PATCH 5/5] www: preload: load ExtSearch via ->ALL Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-13 7:00 UTC (permalink / raw)
To: meta
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 = "<pre><b>$e</b></pre>" . $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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/5] extindex: set {current_info} in eidxq processing
2021-10-13 7:00 [PATCH 0/5] warnings and such Eric Wong
` (2 preceding siblings ...)
2021-10-13 7:00 ` [PATCH 3/5] treewide: use warn() or carp() instead of env->{psgi.errors} Eric Wong
@ 2021-10-13 7:00 ` Eric Wong
2021-10-13 7:00 ` [PATCH 5/5] www: preload: load ExtSearch via ->ALL Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-13 7:00 UTC (permalink / raw)
To: meta
This gives context as to where warnings are coming from.
---
lib/PublicInbox/ExtSearchIdx.pm | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index ddb16241e23d..750ced5c0f45 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -754,9 +754,8 @@ sub prep_id2pos ($) {
sub eidxq_process ($$) { # for reindexing
my ($self, $sync) = @_;
- return unless $self->{cfg};
-
- return unless eidxq_lock_acquire($self);
+ local $self->{current_info} = 'eidxq process';
+ return unless ($self->{cfg} && eidxq_lock_acquire($self));
my $dbh = $self->{oidx}->dbh;
my $tot = $dbh->selectrow_array('SELECT COUNT(*) FROM eidxq') or return;
${$sync->{nr}} = 0;
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 5/5] www: preload: load ExtSearch via ->ALL
2021-10-13 7:00 [PATCH 0/5] warnings and such Eric Wong
` (3 preceding siblings ...)
2021-10-13 7:00 ` [PATCH 4/5] extindex: set {current_info} in eidxq processing Eric Wong
@ 2021-10-13 7:00 ` Eric Wong
4 siblings, 0 replies; 6+ messages in thread
From: Eric Wong @ 2021-10-13 7:00 UTC (permalink / raw)
To: meta
This ought to give us more CoW savings and fragmentation
avoidance in -httpd.
---
lib/PublicInbox/WWW.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 5e83f76915d5..b4db0582db79 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -170,6 +170,7 @@ sub preload {
if (defined($pi_cfg->{'publicinbox.cgitrc'})) {
$pi_cfg->limiter('-cgit');
}
+ $pi_cfg->ALL and require PublicInbox::Isearch;
$self->cgit;
$self->stylesheets_prepare($_) for ('', '../', '../../');
$self->news_www;
^ permalink raw reply related [flat|nested] 6+ messages in thread