diff --git a/lib/Qpsmtpd.pm b/lib/Qpsmtpd.pm index 39363b6..a17f242 100644 --- a/lib/Qpsmtpd.pm +++ b/lib/Qpsmtpd.pm @@ -292,6 +292,7 @@ sub run_continuation { my $tran = $self->transaction; eval { @r = $code->{code}->($self, $tran, @$args); }; if ($@) { + chomp $@; $self->log(LOGCRIT, "FATAL PLUGIN ERROR [$name]: ", $@); next; } diff --git a/t/Test/Qpsmtpd.pm b/t/Test/Qpsmtpd.pm index f36864c..0254c49 100644 --- a/t/Test/Qpsmtpd.pm +++ b/t/Test/Qpsmtpd.pm @@ -96,7 +96,11 @@ sub log { my ($self, $trace, $hook, $plugin, @log) = @_; my $level = Qpsmtpd::TRACE_LEVEL() || 5; $level = $self->init_logger if !defined $level; - print("# " . join(' ', $$, @log) . "\n") if $trace <= $level; + return if $trace > $level; + print("# " . join(' ', $$, @log) . "\n"); + ( undef, undef, my @record_args ) = @_; + push @{ $self->{_logged} }, log_level($trace) . ":" + . join '', grep { defined } @record_args; } sub varlog { diff --git a/t/qpsmtpd.t b/t/qpsmtpd.t index e6b50bb..4c81d8c 100644 --- a/t/qpsmtpd.t +++ b/t/qpsmtpd.t @@ -148,19 +148,36 @@ sub __run_continuation { disconnected => 0, descr => 'DECLINED -> DENY', }, + # TODO: ignore invalid return codes rather than treating them like OK + #{ + # hooks => [ [123456,undef], [DENY, 'goaway'] ], + # expected_response => '550/goaway', + # disconnected => 0, + # descr => 'INVALID -> DENY', + #}, { - hooks => [ [123456,undef], [DENY, 'goaway'] ], - expected_response => '550/goaway', + hooks => [ sub { die "dead\n" }, [DENY, 'begone'] ], + expected_response => '550/begone', disconnected => 0, - descr => 'INVALID -> DENY', + logged => 'LOGCRIT:FATAL PLUGIN ERROR [___FakeHook___]: dead', + descr => 'fatal error -> DENY', + }, + { + hooks => [ [undef], [DENY, 'nm'] ], + expected_response => '550/nm', + disconnected => 0, + logged => 'LOGERROR:Plugin ___FakeHook___, hook helo returned undef!', + descr => 'undef -> DENY', }, ); for my $t (@test_data) { - for my $h ( @{ $t->{hooks} } ) { - $smtpd->fake_hook( 'helo', sub { return @$h } ); + for my $h ( reverse @{ $t->{hooks} } ) { + my $sub = ( ref $h eq 'ARRAY' ? sub { return @$h } : $h ); + $smtpd->fake_hook( 'helo', $sub ); } $smtpd->{_continuation} = [ 'helo', ['somearg'], @{ $smtpd->hooks->{helo} } ]; delete $smtpd->{_response}; + delete $smtpd->{_logged}; $smtpd->connection->notes( disconnected => undef ); $smtpd->run_continuation; my $response = join '/', @{ $smtpd->{_response} || [] }; @@ -174,8 +191,12 @@ sub __run_continuation { ok( ! $smtpd->connection->notes('disconnected'), "run_continuation() does not disconnect on $t->{descr}" ); } + if ( $t->{logged} ) { + is( join("\n", @{ $smtpd->{_logged} || [] }), $t->{logged}, + "run_continuation() logging on $t->{descr}" ); + } + $smtpd->unfake_hook('helo'); } - $smtpd->unfake_hook('helo'); } sub __hook_responder {