* [PATCH 10/13] lei_input: always close single `eml' inputs
2023-11-09 10:09 7% [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
@ 2023-11-09 10:09 6% ` Eric Wong
0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
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;
^ permalink raw reply related [relevance 6%]
* [PATCH 00/13] misc error handling stuff and simplifications
@ 2023-11-09 10:09 7% Eric Wong
2023-11-09 10:09 6% ` [PATCH 10/13] lei_input: always close single `eml' inputs Eric Wong
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2023-11-09 10:09 UTC (permalink / raw)
To: meta
1-4 were things I noticed while chasing the lei SIGPIPE handling
fix. 5-7 are things I noticed while testing on Dragonfly and
NetBSD. 8 was noticed randomly while working on a new mirror,
and the last few complete the work which let me get rid of tied
IO handles in favor of using the PublicInbox::IO subclass.
Eric Wong (13):
lei_xsearch: put query in process title for debugging
lei: use cached $daemon_pid when possible
lei: reuse FDs atfork and close explicitly
lei_up: use v5.12
net_nntp_socks: more comments around how it works
lei ls-mail-source: gracefully handle network failures
net: retry on EINTR and check for {quit} flag
lei_mirror: note missing local manifests are non-fatal
ipc: simplify partial sendmsg fallback
lei_input: always close single `eml' inputs
xapcmd: get rid of scalar wantarray popen_rd
lei: get rid of autoreap usage
spawn: get rid of wantarray popen_rd/popen_wr
lib/PublicInbox/IPC.pm | 13 ++------
lib/PublicInbox/LEI.pm | 11 ++++---
lib/PublicInbox/LeiInput.pm | 26 +++++++--------
lib/PublicInbox/LeiLsMailSource.pm | 6 ++--
lib/PublicInbox/LeiMirror.pm | 5 +--
lib/PublicInbox/LeiRemote.pm | 14 ++++----
lib/PublicInbox/LeiUp.pm | 10 +++---
lib/PublicInbox/LeiXSearch.pm | 27 ++++++++-------
lib/PublicInbox/NetNNTPSocks.pm | 12 ++++---
lib/PublicInbox/NetReader.pm | 53 +++++++++++++++++++++---------
lib/PublicInbox/Spawn.pm | 6 ++--
lib/PublicInbox/TestCommon.pm | 23 ++++++++++++-
lib/PublicInbox/Watch.pm | 2 +-
lib/PublicInbox/Xapcmd.pm | 12 +++----
t/io.t | 8 +----
t/ipc.t | 7 ++++
t/lei-import.t | 27 +++++++++++++++
17 files changed, 163 insertions(+), 99 deletions(-)
^ permalink raw reply [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-11-09 10:09 7% [PATCH 00/13] misc error handling stuff and simplifications Eric Wong
2023-11-09 10:09 6% ` [PATCH 10/13] lei_input: always close single `eml' inputs Eric Wong
Code repositories for project(s) associated with this public 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).