preaction / Log-Any

Simple, fast Perl logging API compatible with any logging system
Other
13 stars 19 forks source link

Format string methods return undef #64

Closed vshekun closed 6 years ago

vshekun commented 6 years ago

Format string methods return undef

my $x1 = $log->error('text'); # $x = 'text'
my $x2 = $log->errorf('text'); # $x = undef
vshekun commented 6 years ago

Sorry, this issue was dublicate #53

preaction commented 6 years ago

No, this is a different problem to #53. This is the formatting methods not returning anything at all, which is a bug.

preaction commented 6 years ago

From the other thread:

@vshekun: I solved this problem the following code

*{$namef} = sub {
   my ( $self, @args ) = @_;
   unless ($self->{adapter}->$is_realname) {
       my $text = shift @args;
       my $message = sprintf($text, @args);
       return $self->$name($message);
   }
   my $message =
     $self->{formatter}->( $self->{category}, $numeric, @args );
   return unless defined $message and length $message;
   return $self->$name($message);
};
preaction commented 6 years ago

Your code completely bypasses the $self->{formatter}, which is what does the sprintf. This doesn't allow people to override the formatter with their own formatter.

As for errorf not returning the formatted log text, this behavior is verified by the Log-Any tests: https://github.com/preaction/Log-Any/blob/master/t/proxy.t#L26

So, I'm going to need more information about the bug, including a reproduction case or a failing test.

vshekun commented 6 years ago

In these tests there is Log::Any::Adapter 'Test'. I use logging in libraries through the following code:

confess $log->errorf( .. );

I have exceptions without description, when program not set Log::Any::Adapter.

My solution does not restrict $self->{formatter}, it fix the issue.

Example problem code

#!/usr/bin/perl
use Log::Any qw($log);
printf "-%s-", $log->infof("ran sub");

Test

$ cat t/proxy-#64.t 
use strict;
use warnings;
use Test::More;

use Log::Any qw( $log );

plan tests => 1;

my $out = $log->infof("ran sub");
is $out, 'ran sub', 'log message built is returned';

$ prove t/proxy-#64.t 
t/proxy-#64.t .. 1/1 
#   Failed test 'log message built is returned'
#   at t/proxy-#64.t line 10.
#          got: undef
#     expected: 'ran sub'
# Looks like you failed 1 test of 1.
t/proxy-#64.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 
preaction commented 6 years ago

Your code goes into the unless block when the adapter isn't logging at a certain level. That block uses sprintf directly, which isn't the formatter that is configured on the proxy object. The proxy must use $self->{formatter} to format the log lines.

It looks like the problem is because Log::Any::Proxy wasn't correctly returning the formatted log string in the case where the log level of the adapter is higher than the current log message. This is always the case when using the Null adapter. I've added a couple tests and made sure that if someone is using the return value from the log method, it is returned.

Thanks for the report and the help reproducing! This is fixed in v1.703.