]> git.ipfire.org Git - thirdparty/public-inbox.git/commitdiff
lei_input: always close single `eml' inputs
authorEric Wong <e@80x24.org>
Thu, 9 Nov 2023 10:09:43 +0000 (10:09 +0000)
committerEric Wong <e@80x24.org>
Thu, 9 Nov 2023 21:53:53 +0000 (21:53 +0000)
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
lib/PublicInbox/TestCommon.pm
t/io.t
t/lei-import.t

index 4cd18c093e978e08f8267335d7d5b960a85cfc64..adb356c975e4472dd239570336382f7bbda7a6e6 100644 (file)
@@ -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");
                }
index 83e99b42ba601ee5eb5ffb2f29e3b2a25e787428..46e6a538602a85b2311de427a407b6567180eac6 100644 (file)
@@ -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 4c7a97a3ac4fba413276c7c5ed952b3507d7e9c7..3ea00ed82e4fe1f1c631297b8762a2bfd5b432da 100644 (file)
--- 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";
index b2c1de9b9f9033f4bf366cfd021bd5866fdcc45e..6ad4c97b904819efc08650341ddcf47ad8540943 100644 (file)
@@ -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;