user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: Eric Wong <e@80x24.org>
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	[thread overview]
Message-ID: <20211013070038.6414-4-e@80x24.org> (raw)
In-Reply-To: <20211013070038.6414-1-e@80x24.org>

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;
 }
 

  parent reply	other threads:[~2021-10-13  7:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211013070038.6414-4-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    --subject='Re: [PATCH 3/5] treewide: use warn() or carp() instead of env->{psgi.errors}' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).