Allow Log to print undef values

[buralien]

Print "undef" when an undef value is provided to Log, instead of throwing an error.

[kraih]

What's the use case for this? And how much of a performance regression does it cause?

[buralien]

What's the use case for this?

In my experience, causing an error when an undef variable is provided to Log is not very user friendly and quite easy to achieve when communicating with external models.

my $val = $third_party_model->some_method();
$c->log->debug("GOT:", $val); # errors out if $val is undef

And how much of a performance regression does it cause?

I haven't found any performance tests in the suite. Both map and defined are core and are only evaluated in case the message is logged with high enough level. I assume that typically the Log methods are not called with very large array of args. What kind of test would be sufficient to answer this?

[jhthorsen]

I don't think it's worth it. Doesn't seem to difficult to avoid it on the way in.

[guest20]

@jhthorsen every caller could map not defined ? "undef" : $_, @to_log to prevent the warning, but does that make web development fun?

[jhthorsen]

@jhthorsen every caller could map not defined ? "undef" : $_, @to_log to prevent the warning, but does that make web development fun?

I'm not sure how often you really need that though. I made https://metacpan.org/pod/Mojolicious::Plugin::Logf for this reason (and some others), but I don't use it that often in my own projects, since it's often overkill imo.

[guest20]

@jhthorsen

Mojolicious::Plugin::Logf [is] often overkill imo.

Definitely.

However with Mojo::Log logging to STDERR, the same handle that perl warnings end up on, it can be a bit of a pita to consume log lines from an app where a call to the logger can cause warnings in a second format that come from within the logger.

Additionally a useful undef warning would refer to the call site of log, not in the guts of Mojo::Log::_short where the join happens. As it turns out, only way for a dev to fix a warning about an undef in _short is to submit a PR like this one...

[buralien]

I don't think it's worth it. Doesn't seem to difficult to avoid it on the way in.

In adition to the comments by guest20 (which I fully agree with), current implementation of Log does not even allow for clean subclassing to override the behavior in an app. Writing an additional if statement every time I want to log some value across an entire application, while not "difficult" is still quite cumbersome and makes for poor code readability.

I was not aware of Logf, so thanks for the tip. In case this PR is not approved, I'll use iot instead of patching Log in my CI pipeline (which is what I do right now).