Log::Log4perl::Catalyst abort not working as expected.

[ScottONeal]

I think there is already a PR for attempting to fix abort w/ Log::Log4perl::Catalyst. Albeit, I wanted to create an issue item for discussion, prior to a merge. This issue is also on the heel of using Catalyst::Plugin::Static::Simple.

It would appear, that abort does not work in the expected fashion of Catalyst::Log, rather it clears all of the appender's buffers when abort is called, when it should be at time = _flush(). Since there is no checking of $self->{abort} in flush, flush could have buffers which were populated after abort was called. This functionally seems to ignore abort's expected outcome. Currently, abort is more akin to a 'clear_buffers' type of function.

If I'm understanding the module correctly, would it be best to make a call to abort more of a setter, which flush then checks and operates on?

I'm thinking something like:

##################################################
sub _flush {
##################################################
    my ($self) = @_;

    for my $appender (values %Log::Log4perl::Logger::APPENDER_BY_NAME) {
        next if $appender->{name} !~ /_$CATALYST_APPENDER_SUFFIX$/;

        if ($self->{abort}) {
            $appender->{appender}{buffer} = [];
        }
        else {
            $appender->flush();
        }
    }

    $self->abort(undef);
}

##################################################
sub abort {
##################################################
    my $self = shift;

    $self->{abort} = $_[0] if @_;

    return $self->{abort};
}

Thanks, @mschilli for producing this wonder module.
Pinging @bokutin

[mschilli]

Hi @ScottONeal, thanks for that, I just merged @bokutin's PR. Can you please check if it works as expected?

[ScottONeal]

Everything is looking good on my end. Thanks, @mschilli!

[bokutin]

Thanks, @mschilli @ScottONeal !!

[mschilli]

Hooray, thanks everyone! :)

[ScottONeal]

Prost! 🍻