unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 10/13] lei_input: always close single `eml' inputs
Date: Thu,  9 Nov 2023 10:09:43 +0000	[thread overview]
Message-ID: <20231109100946.1440611-11-e@80x24.org> (raw)
In-Reply-To: <20231109100946.1440611-1-e@80x24.org>

This matches the behavior we have for multi-message mbox files
since we rely on ->close to detect errors on bad mboxes.  This
ensures we'll notice errors reading single messages from stdin.

We'll also start relying more on strace error injection to test
error handling.
---
 lib/PublicInbox/LeiInput.pm   | 13 +++++++------
 lib/PublicInbox/TestCommon.pm | 23 ++++++++++++++++++++++-
 t/io.t                        |  8 +-------
 t/lei-import.t                | 27 +++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index 4cd18c09..adb356c9 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -81,9 +81,11 @@ sub input_fh {
 	my ($self, $ifmt, $fh, $name, @args) = @_;
 	if ($ifmt eq 'eml') {
 		my $buf = do { local $/; <$fh> };
-		(defined($buf) && eof($fh) && close($fh)) or
-			return $self->{lei}->child_error(0, <<"");
-error reading $name: $!
+		my $ok = defined($buf) ? 1 : 0;
+		++$ok if eof($fh);
+		++$ok if $fh->close;
+		$ok == 3 or return $self->{lei}->child_error($?, <<"");
+error reading $name: $! (\$?=$?)
 
 		PublicInbox::Eml::strip_from($buf);
 
@@ -246,9 +248,8 @@ sub input_path_url {
 			my $rdr = { 2 => $lei->{2} };
 			my $fh = popen_rd($fp, undef, $rdr);
 			eval { $self->input_fh('eml', $fh, $input, @args) };
-			my @err = ($@ ? $@ : ());
-			$fh->close or push @err, "\$?=$?";
-			$lei->child_error($?, "@$fp failed: @err") if @err;
+			my $err = $@ ? ": $@" : '';
+			$lei->child_error($?, "@$fp failed$err") if $err || $?;
 		} else {
 			$self->folder_missing("$ifmt:$input");
 		}
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 83e99b42..46e6a538 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -28,7 +28,8 @@ BEGIN {
 		quit_waiter_pipe wait_for_eof
 		tcp_host_port test_lei lei lei_ok $lei_out $lei_err $lei_opt
 		test_httpd xbail require_cmd is_xdeeply tail_f
-		ignore_inline_c_missing no_pollerfd no_coredump cfg_new);
+		ignore_inline_c_missing no_pollerfd no_coredump cfg_new
+		strace strace_inject);
 	require Test::More;
 	my @methods = grep(!/\W/, @Test::More::EXPORT);
 	eval(join('', map { "*$_=\\&Test::More::$_;" } @methods));
@@ -933,6 +934,26 @@ sub cfg_new ($;@) {
 	PublicInbox::Config->new($f);
 }
 
+our $strace_cmd;
+sub strace () {
+	skip 'linux only test' if $^O ne 'linux';
+	require_cmd('strace', 1);
+}
+
+sub strace_inject () {
+	my $cmd = strace;
+	state $ver = do {
+		require PublicInbox::Spawn;
+		my $v = PublicInbox::Spawn::run_qx([$cmd, '--version']);
+		$v =~ m!version\s+([1-9]+\.[0-9]+)! or
+				xbail "no strace --version: $v";
+		eval("v$1");
+	};
+	$ver ge v4.16 or skip "$cmd too old for syscall injection (".
+				sprintf('v%vd', $ver). ' < v4.16)';
+	$cmd
+}
+
 package PublicInbox::TestCommon::InboxWakeup;
 use strict;
 sub on_inbox_unlock { ${$_[0]}->($_[1]) }
diff --git a/t/io.t b/t/io.t
index 4c7a97a3..3ea00ed8 100644
--- a/t/io.t
+++ b/t/io.t
@@ -9,13 +9,7 @@ use PublicInbox::Spawn qw(which run_qx);
 
 # only test failures
 SKIP: {
-skip 'linux only test' if $^O ne 'linux';
-my $strace = which('strace') or skip 'strace missing for test';
-my $v = run_qx([$strace, '--version']);
-$v =~ m!version\s+([1-9]+\.[0-9]+)! or xbail "no strace --version: $v";
-$v = eval("v$1");
-$v ge v4.16 or skip "$strace too old for syscall injection (".
-		sprintf('v%vd', $v). ' < v4.16)';
+my $strace = strace_inject;
 my $env = { PERL5LIB => join(':', @INC) };
 my $opt = { 1 => \my $out, 2 => \my $err };
 my $dst = "$tmpdir/dst";
diff --git a/t/lei-import.t b/t/lei-import.t
index b2c1de9b..6ad4c97b 100644
--- a/t/lei-import.t
+++ b/t/lei-import.t
@@ -154,6 +154,33 @@ do {
 } until (!lei('ls-label') || $lei_out =~ /\bbin\b/ || now > $end);
 like($lei_out, qr/\bbin\b/, 'commit-delay eventually commits');
 
+SKIP: {
+	my $strace = strace_inject; # skips if strace is old or non-Linux
+	my $tmpdir = tmpdir;
+	my $tr = "$tmpdir/tr";
+	my $cmd = [ $strace, "-o$tr", '-f',
+		"-P", File::Spec->rel2abs('t/plack-qp.eml'),
+		'-e', 'inject=readv,read:error=EIO'];
+	lei_ok qw(daemon-pid);
+	chomp(my $daemon_pid = $lei_out);
+	push @$cmd, '-p', $daemon_pid;
+	my $strace_opt = { 1 => \my $out, 2 => \my $err };
+	require PublicInbox::Spawn;
+	require PublicInbox::AutoReap;
+	my $pid = PublicInbox::Spawn::spawn($cmd, \%ENV, $strace_opt);
+	my $ar = PublicInbox::AutoReap->new($pid);
+	tick; # wait for strace to attach
+	ok(!lei(qw(import -F eml t/plack-qp.eml)),
+		'-F eml import fails on pathname error injection');
+	like($lei_err, qr!error reading t/plack-qp\.eml: Input/output error!,
+		'EIO noted in stderr');
+	open $fh, '<', 't/plack-qp.eml';
+	ok(!lei(qw(import -F eml -), undef, { %$lei_opt, 0 => $fh }),
+		'-F eml import fails on stdin error injection');
+	like($lei_err, qr!error reading .*?: Input/output error!,
+		'EIO noted in stderr');
+}
+
 # see t/lei_to_mail.t for "import -F mbox*"
 });
 done_testing;

  parent reply	other threads:[~2023-11-09 10:09 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-09 10:09 [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
2023-11-09 10:09 ` [PATCH 01/13] lei_xsearch: put query in process title for debugging Eric Wong
2023-11-09 10:09 ` [PATCH 02/13] lei: use cached $daemon_pid when possible Eric Wong
2023-11-09 10:09 ` [PATCH 03/13] lei: reuse FDs atfork and close explicitly Eric Wong
2023-11-09 10:09 ` [PATCH 04/13] lei_up: use v5.12 Eric Wong
2023-11-09 10:09 ` [PATCH 05/13] net_nntp_socks: more comments around how it works Eric Wong
2023-11-09 10:09 ` [PATCH 06/13] lei ls-mail-source: gracefully handle network failures Eric Wong
2023-11-09 10:09 ` [PATCH 07/13] net: retry on EINTR and check for {quit} flag Eric Wong
2023-11-09 10:09 ` [PATCH 08/13] lei_mirror: note missing local manifests are non-fatal Eric Wong
2023-11-09 10:09 ` [PATCH 09/13] ipc: simplify partial sendmsg fallback Eric Wong
2023-11-09 10:09 ` Eric Wong [this message]
2023-11-09 10:09 ` [PATCH 11/13] xapcmd: get rid of scalar wantarray popen_rd Eric Wong
2023-11-09 10:09 ` [PATCH 12/13] lei: get rid of autoreap usage Eric Wong
2023-11-09 10:09 ` [PATCH 13/13] spawn: get rid of wantarray popen_rd/popen_wr 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=20231109100946.1440611-11-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).