Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format string methods return undef #64

Closed
vshekun opened this issue Nov 23, 2017 · 6 comments
Closed

Format string methods return undef #64

vshekun opened this issue Nov 23, 2017 · 6 comments

Comments

@vshekun
Copy link

vshekun commented Nov 23, 2017

Format string methods return undef

my $x1 = $log->error('text'); # $x = 'text'
my $x2 = $log->errorf('text'); # $x = undef
@vshekun
Copy link
Author

vshekun commented Nov 23, 2017

#Sorry, this issue was dublicate #53

@vshekun vshekun closed this as completed Nov 23, 2017
@preaction
Copy link
Owner

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

@preaction preaction reopened this Nov 28, 2017
@preaction
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

vshekun commented Nov 28, 2017

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 added a commit that referenced this issue Nov 29, 2017
    [Fixed]

    - Fixed log format methods (errorf, warnf, infof, etc...) not
      returning the formatted message sometimes. Thanks @vshekun!
      [Github #64]
@preaction
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants