plack / Plack

PSGI toolkit and server adapters
http://plackperl.org/
Other
486 stars 214 forks source link

Plack::App::URLMap: use psgix.logger #685

Closed XSven closed 2 years ago

XSven commented 2 years ago

I would suggest to replace the proprietary debug logging (PLACK_URLMAP_DEBUG) with a psgix.logger based logging.

miyagawa commented 2 years ago

PLACK_URLMAP_DEBUG is a flag to whether enabling the debug, while psgi.logger is a callback to where to print the logs to, and they're different things. Perhaps you meant to replace the warn with the logger, or to psgi.errors.

XSven commented 2 years ago

To be precise. I don't want to use yet another proprietary debugging output implementation. As a compromise I am kindly asking you to add after each of the 3 lines

DEBUG && warn "...\n";

a corresponding

$env->{ 'psgix.logger' }->( { level => 'debug', message => "..." } ) if exists $env->{ 'psgix.logger' };

line. I have chosen debug as the level because this fits to your classification (PLACKURLMAPDEBUG).

By the way it would be very helpful if the original PATH_INFO and SCRIPT_NAME values could be logged too. A corresponding log statement at line https://metacpan.org/dist/Plack/source/lib/Plack/App/URLMap.pm#L44 would be very much appreciated.

miyagawa commented 2 years ago

OK that makes a little sense. Though personally these debug lines are mostly only useful when developing and testing the app, rather than enabling it in production (for logger). Also, warn is a very standard debugging system that is built into perl and calling it "yet another proprietary" is very weird. I am not sure what you're trying to get after by logging each URLMap actions to the logger function?

If this is really what you want I believe you can do:

BEGIN  { $ENV{PLACK_URLMAP_DEBUG} = 1; }
use Plack::Builder;

my $app = sub { ... };

builder {
    enable "SomeLoggerMiddleware";
    enable sub {
        my $app = shift;
        sub {
            my $env = shift;
            local $SIG{__WARN__} = sub { $env->{'psgix.logger'}->({ level => 'debug', message => $_[0] }) };
            $app->($env);
        };
    };
    mount "/" => $app;
};

to capture the warn messages and pass them to logger. This is a universal solution that would work for any module that would emit debug messages to warn.

XSven commented 2 years ago

Maybe this is a more polite summary of what I a talking about.

I guess your workaround will work but my request is a feature request that should lead to a lean, clean, straight setup like this

my $urlmap = Plack::App::URLMap->new;
...
builder {
    enable "LogAny", category => "Plack::App::URLMap";
    $urlmap->to_app;
}