]> Sergey Matveev's repositories - public-inbox.git/commitdiff
lei: propagate curl errors, improve internal consistency
authorEric Wong <e@80x24.org>
Wed, 3 Feb 2021 08:11:37 +0000 (22:11 -1000)
committerEric Wong <e@80x24.org>
Thu, 4 Feb 2021 01:37:09 +0000 (01:37 +0000)
IO::Uncompress::Gunzip seems to be losing $? when closing
PublicInbox::ProcessPipe.  To workaround this, do a synchronous
waitpid ourselves to force proper $? reporting update tests to
use the new --only feature for testing invalid URLs.

This improves internal code consistency by having {pkt_op}
parse the same ASCII-only protocol script/lei understands.

We no longer pass {sock} to worker processes at all,
further reducing FD pressure on per-user limits.

lib/PublicInbox/LEI.pm
lib/PublicInbox/LeiOverview.pm
lib/PublicInbox/LeiXSearch.pm
lib/PublicInbox/PktOp.pm
t/lei.t

index 9b4d4e0ba7aa2d68909cfe83cec6db4c8bcf9a89..05a39cadffe3f4c08393d4b078e7241f2b230a94 100644 (file)
@@ -285,8 +285,8 @@ sub x_it ($$) {
        # make sure client sees stdout before exit
        $self->{1}->autoflush(1) if $self->{1};
        dump_and_clear_log();
-       if (my $sock = $self->{sock}) {
-               send($sock, "x_it $code", MSG_EOR);
+       if (my $s = $self->{pkt_op} // $self->{sock}) {
+               send($s, "x_it $code", MSG_EOR);
        } elsif ($self->{oneshot}) {
                # don't want to end up using $? from child processes
                for my $f (qw(lxs l2m)) {
@@ -339,9 +339,10 @@ sub puts ($;@) { out(shift, map { "$_\n" } @_) }
 
 sub child_error { # passes non-fatal curl exit codes to user
        my ($self, $child_error) = @_; # child_error is $?
-       if (my $sock = $self->{sock}) { # send to lei(1) client
-               send($sock, "child_error $child_error", MSG_EOR);
-       } elsif ($self->{oneshot}) {
+       if (my $s = $self->{pkt_op} // $self->{sock}) {
+               # send to the parent lei-daemon or to lei(1) client
+               send($s, "child_error $child_error", MSG_EOR);
+       } elsif (!$PublicInbox::DS::in_loop) {
                $self->{child_error} = $child_error;
        } # else noop if client disconnected
 }
@@ -420,9 +421,9 @@ sub atfork_parent_wq {
                $lei->{$f} = $wq->deep_clone($tmp);
        }
        $self->{env} = $env;
-       delete @$lei{qw(3 -lei_store cfg old_1 pgr lxs)}; # keep l2m
+       delete @$lei{qw(sock 3 -lei_store cfg old_1 pgr lxs)}; # keep l2m
        my @io = (delete(@$lei{qw(0 1 2)}),
-                       io_extract($lei, qw(sock pkt_op startq)));
+                       io_extract($lei, qw(pkt_op startq)));
        my $l2m = $lei->{l2m};
        if ($l2m && $l2m != $wq) { # $wq == lxs
                if (my $wq_s1 = $l2m->{-wq_s1}) {
index 366af8b2768d8c35d281f098a280396285938600..88034adadaed4e5d9551199964203d4ae0f5d401 100644 (file)
@@ -216,9 +216,7 @@ sub ovv_each_smsg_cb { # runs in wq worker usually
                        $wcb->(undef, $smsg, $eml);
                };
        } elsif ($l2m && $l2m->{-wq_s1}) {
-               my $sock = delete $lei->{sock}; # lei2mail doesn't need it
                my ($lei_ipc, @io) = $lei->atfork_parent_wq($l2m);
-               $lei->{sock} = $sock if $sock;
                # $io[0] becomes a notification pipe that triggers EOF
                # in this wq worker when all outstanding ->write_mail
                # calls are complete
index 23a9c020fb01b4ecbf11854ec60d8373af9fc976..d33064bb852f73d8143cd663febbdfca3cc0b00e 100644 (file)
@@ -113,8 +113,7 @@ sub mset_progress {
        if ($lei->{pkt_op}) { # called via pkt_op/pkt_do from workers
                pkt_do($lei->{pkt_op}, 'mset_progress', @_);
        } else { # single lei-daemon consumer
-               my @args = ref($_[-1]) eq 'ARRAY' ? @{$_[-1]} : @_;
-               my ($desc, $mset_size, $mset_total_est) = @args;
+               my ($desc, $mset_size, $mset_total_est) = @_;
                $lei->{-mset_total} += $mset_size;
                $lei->err("# $desc $mset_size/$mset_total_est");
        }
@@ -264,14 +263,11 @@ sub query_remote_mboxrd {
                shift(@$cmd) if !$cmd->[0];
 
                $lei->err("# @$cmd") if $verbose;
-               $? = 0;
-               my $fh = popen_rd($cmd, $env, $rdr);
+               my ($fh, $pid) = popen_rd($cmd, $env, $rdr);
                $fh = IO::Uncompress::Gunzip->new($fh);
-               eval {
-                       PublicInbox::MboxReader->mboxrd($fh, \&each_eml, $self,
-                                                       $lei, $each_smsg);
-               };
-               return $lei->fail("E: @$cmd: $@") if $@;
+               PublicInbox::MboxReader->mboxrd($fh, \&each_eml, $self,
+                                               $lei, $each_smsg);
+               waitpid($pid, 0) == $pid or die "BUG: waitpid (curl): $!";
                if ($? == 0) {
                        my $nr = $lei->{-nr_remote_eml};
                        mset_progress($lei, $lei->{-current_url}, $nr, $nr);
@@ -420,6 +416,8 @@ sub do_query {
                '.' => [ \&do_post_augment, $lei, $zpipe, $au_done ],
                '' => [ \&query_done, $lei ],
                'mset_progress' => [ \&mset_progress, $lei ],
+               'x_it' => [ $lei->can('x_it'), $lei ],
+               'child_error' => [ $lei->can('child_error'), $lei ],
        };
        (my $op, $lei->{pkt_op}) = PublicInbox::PktOp->pair($ops);
        my ($lei_ipc, @io) = $lei->atfork_parent_wq($self);
index 40c7262a3848d01dd222b9e0b715d7b982eb0e0a..10d76da0536b18064fcbf4874698f5e76da9ea7e 100644 (file)
@@ -4,8 +4,7 @@
 # op dispatch socket, reads a message, runs a sub
 # There may be multiple producers, but (for now) only one consumer
 # Used for lei_xsearch and maybe other things
-# "literal" => [ sub, @operands ]
-# /regexp/ => [ sub, @operands ]
+# "command" => [ $sub, @fixed_operands ]
 package PublicInbox::PktOp;
 use strict;
 use v5.10.1;
@@ -57,11 +56,19 @@ sub event_step {
                        $self->close;
                        die "recv: $!";
                }
-               my ($cmd, $pargs) = split(/\0/, $msg, 2);
+               my ($cmd, @pargs);
+               if (index($msg, "\0") > 0) {
+                       ($cmd, my $pargs) = split(/\0/, $msg, 2);
+                       @pargs = @{ipc_thaw($pargs)};
+               } else {
+                       # for compatibility with the script/lei in client mode,
+                       # it doesn't load Sereal||Storable for startup speed
+                       ($cmd, @pargs) = split(/ /, $msg);
+               }
                my $op = $self->{ops}->{$cmd //= $msg};
                die "BUG: unknown message: `$cmd'" unless $op;
                my ($sub, @args) = @$op;
-               $sub->(@args, $pargs ? ipc_thaw($pargs) : ());
+               $sub->(@args, @pargs);
                return $self->close if $msg eq ''; # close on EOF
        } while (1);
 }
diff --git a/t/lei.t b/t/lei.t
index 33f47ae4b082a406930ca420f19eacc6efd8bb91..461669a8813b47aece9a00d26e4114fafbe4a26f 100644 (file)
--- a/t/lei.t
+++ b/t/lei.t
@@ -14,6 +14,7 @@ require_mods(qw(json DBD::SQLite Search::Xapian));
 my $opt = { 1 => \(my $out = ''), 2 => \(my $err = '') };
 my ($home, $for_destroy) = tmpdir();
 my $err_filter;
+my $curl = which('curl');
 my @onions = qw(http://hjrcffqmbrq6wope.onion/meta/
        http://czquwvybam4bgbro.onion/meta/
        http://ou63pmih66umazou.onion/meta/);
@@ -39,7 +40,7 @@ local $ENV{XDG_RUNTIME_DIR} = "$home/xdg_run";
 local $ENV{HOME} = $home;
 local $ENV{FOO} = 'BAR';
 mkdir "$home/xdg_run", 0700 or BAIL_OUT "mkdir: $!";
-my $home_trash = [ "$home/.local", "$home/.config" ];
+my $home_trash = [ "$home/.local", "$home/.config", "$home/junk" ];
 my $cleanup = sub { rmtree([@$home_trash, @_]) };
 my $config_file = "$home/.config/lei/config";
 my $store_dir = "$home/.local/share/lei";
@@ -162,26 +163,19 @@ my $setup_publicinboxes = sub {
 my $test_external_remote = sub {
        my ($url, $k) = @_;
 SKIP: {
-       my $nr = 4;
+       my $nr = 5;
        skip "$k unset", $nr if !$url;
-       which('curl') or skip 'no curl', $nr;
+       $curl or skip 'no curl', $nr;
        which('torsocks') or skip 'no torsocks', $nr if $url =~ m!\.onion/!;
-       $lei->('ls-external');
-       for my $e (split(/^/ms, $out)) {
-               $e =~ s/\s+boost.*//s;
-               $lei->('forget-external', '-q', $e) or
-                       fail "error forgetting $e: $err"
-       }
-       $lei->('add-external', $url);
        my $mid = '20140421094015.GA8962@dcvr.yhbt.net';
-       ok($lei->('q', '-q', "m:$mid"), "query $url");
+       my @cmd = ('q', '--only', $url, '-q', "m:$mid");
+       ok($lei->(@cmd), "query $url");
        is($err, '', "no errors on $url");
        my $res = $json->decode($out);
        is($res->[0]->{'m'}, "<$mid>", "got expected mid from $url");
-       ok($lei->('q', '-q', "m:$mid", 'd:..20101002'), 'no results, no error');
+       ok($lei->(@cmd, 'd:..20101002'), 'no results, no error');
        is($err, '', 'no output on 404, matching local FS behavior');
        is($out, "[null]\n", 'got null results');
-       $lei->('forget-external', $url);
 } # /SKIP
 }; # /sub
 
@@ -355,12 +349,21 @@ my $test_completion = sub {
        }
 };
 
+my $test_fail = sub {
+       $lei->(qw(q --only http://127.0.0.1:99999/bogus/ t:m));
+       is($? >> 8, 3, 'got curl exit for bogus URL');
+       $lei->(qw(q --only http://127.0.0.1:99999/bogus/ t:m -o), "$home/junk");
+       is($? >> 8, 3, 'got curl exit for bogus URL with Maildir');
+       is($out, '', 'no output');
+};
+
 my $test_lei_common = sub {
        $test_help->();
        $test_config->();
        $test_init->();
        $test_external->();
        $test_completion->();
+       $test_fail->();
 };
 
 if ($ENV{TEST_LEI_ONESHOT}) {