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

Already on GitHub? Sign in to your account

Dancer2::Plugin::_get_dsl() returns undef #418

Closed
melo opened this Issue Aug 30, 2013 · 15 comments

Comments

Projects
None yet
5 participants
Contributor

melo commented Aug 30, 2013

Hi,

$Dancer2::Plugin::VERSION = '0.08';

After I start my app (currently using Server::Starter and Starman), the first request gets a 500 error. The message is:

[Mush::Admin::Web:28798] error @2013-08-30 17:25:49> Route exception: Can't call method "dancer_app" on an undefined value at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin.pm line 127. in (eval 134) l. 2

I've updated _get_dsl() to print a Carp::longmess in case the final $dsl is null.

The stack trace I get is this:

_get_dsl() => undef at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin.pm line 125.
    Dancer2::Plugin::plugin_setting() called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin/Auth/Tiny.pm line 40
    Dancer2::Plugin::Auth::Tiny::__ANON__('Dancer2::Core::Context=HASH(0x27fdf38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Route.pm line 148
    Dancer2::Core::Route::execute('Dancer2::Core::Route=HASH(0x5f2e6c8)', 'Dancer2::Core::Context=HASH(0x27fdf38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 87
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 87
    Dancer2::Core::Dispatcher::dispatch('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)', undef, undef) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 141
    Dancer2::Core::Dispatcher::__ANON__('CODE(0x291c2b8)', 'Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at (eval 133) line 1
    Dancer2::Core::Dispatcher::__ANON__('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at (eval 134) line 2
    Dancer2::Core::Dispatcher::dispatch('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Role/Server.pm line 76
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Role/Server.pm line 76
    Dancer2::Core::Role::Server::__ANON__('HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Util.pm line 142
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Util.pm line 142
    Plack::Util::run_app('CODE(0x2aca3c0)', 'HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Starman/Server.pm line 273
    Starman::Server::process_request('Starman::Server=HASH(0x1c76a28)', 'Net::Server::Proto::TCP=GLOB(0x61c3f40)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server.pm line 75
    Net::Server::run_client_connection('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 229
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 229
    Net::Server::PreFork::run_child('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 187
    Net::Server::PreFork::run_n_children('Starman::Server=HASH(0x1c76a28)', 2) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 111
    Net::Server::PreFork::loop('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server.pm line 61
    Net::Server::run('Starman::Server=HASH(0x1c76a28)', 'port', 'ARRAY(0x5f55b18)', 'host', '*', 'proto', 'tcp', 'serialize', 'none', ...) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Starman/Server.pm line 84
    Starman::Server::run('Starman::Server=HASH(0x1c76a28)', 'CODE(0x2aca3c0)', 'HASH(0x61c1a98)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Handler/Starman.pm line 18
    Plack::Handler::Starman::run('Plack::Handler::Starman=HASH(0x1b3d8c8)', 'CODE(0x2aca3c0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Loader.pm line 84
    Plack::Loader::run('Plack::Loader=HASH(0x1b4f8e0)', 'Plack::Handler::Starman=HASH(0x1b3d8c8)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Runner.pm line 277
    Plack::Runner::run('Plack::Runner=HASH(0x18f5e38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/bin/starman line 38
_get_dsl() => undef at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin.pm line 125.
    Dancer2::Plugin::plugin_setting() called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin/Auth/Tiny.pm line 40
    Dancer2::Plugin::Auth::Tiny::__ANON__('Dancer2::Core::Context=HASH(0x27fdf38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Route.pm line 148
    Dancer2::Core::Route::execute('Dancer2::Core::Route=HASH(0x5f2e6c8)', 'Dancer2::Core::Context=HASH(0x27fdf38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 87
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 87
    Dancer2::Core::Dispatcher::dispatch('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)', undef, undef) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Dispatcher.pm line 141
    Dancer2::Core::Dispatcher::__ANON__('CODE(0x291c2b8)', 'Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at (eval 133) line 1
    Dancer2::Core::Dispatcher::__ANON__('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at (eval 134) line 2
    Dancer2::Core::Dispatcher::dispatch('Dancer2::Core::Dispatcher=HASH(0x29341e8)', 'HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Role/Server.pm line 76
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Core/Role/Server.pm line 76
    Dancer2::Core::Role::Server::__ANON__('HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Util.pm line 142
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Util.pm line 142
    Plack::Util::run_app('CODE(0x2aca3c0)', 'HASH(0x61c42a0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Starman/Server.pm line 273
    Starman::Server::process_request('Starman::Server=HASH(0x1c76a28)', 'Net::Server::Proto::TCP=GLOB(0x61c3f40)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server.pm line 75
    Net::Server::run_client_connection('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 229
    eval {...} called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 229
    Net::Server::PreFork::run_child('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 187
    Net::Server::PreFork::run_n_children('Starman::Server=HASH(0x1c76a28)', 2) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server/PreFork.pm line 111
    Net::Server::PreFork::loop('Starman::Server=HASH(0x1c76a28)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Net/Server.pm line 61
    Net::Server::run('Starman::Server=HASH(0x1c76a28)', 'port', 'ARRAY(0x5f55b18)', 'host', '*', 'proto', 'tcp', 'serialize', 'none', ...) called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Starman/Server.pm line 84
    Starman::Server::run('Starman::Server=HASH(0x1c76a28)', 'CODE(0x2aca3c0)', 'HASH(0x61c1a98)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Handler/Starman.pm line 18
    Plack::Handler::Starman::run('Plack::Handler::Starman=HASH(0x1b3d8c8)', 'CODE(0x2aca3c0)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Loader.pm line 84
    Plack::Loader::run('Plack::Loader=HASH(0x1b4f8e0)', 'Plack::Handler::Starman=HASH(0x1b3d8c8)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Plack/Runner.pm line 277
    Plack::Runner::run('Plack::Runner=HASH(0x18f5e38)') called at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/bin/starman line 38
[Mush::Admin::Web:29222] error @2013-08-30 17:36:36> Route exception: Can't call method "dancer_app" on an undefined value at /opt/mariapp/.perlbrew/libs/perl-5.16.3@release/lib/perl5/Dancer2/Plugin.pm line 127. in (eval 134) l. 2

I'm still trying to track this down, but I'm leaving this here so other can find it if they run into the same issue. I'll try to track this down.

Thanks

Contributor

melo commented Sep 16, 2013

I can reproduce this with my own app now, I'll try to reduce it further later on. The magic combination is Starman + Dancer2::Plugin::Auth::Tiny. With the standard server this works just fine.

I've added debug information to Dancer2::Plugin::_get_dsl(), that's the function that fails to find the $dsl object under this situation.

When my app is starting under starman, this method is called while the app loads, and he can find the dsl fine because my main App class is in the call stack (with the default server he finds main...), but when a request comes in, none of the classes in the caller stack have a dsl() method.

I really have no idea what's the next step to fix this... I'll try to reduce this a bit more with a smaller app.

In the meantime, guidance would be most appreciated \cc @ambs @xsawyerx @sukria

Contributor

melo commented Sep 16, 2013

A single line change in Auth::Tiny avoids the problem, but I'm not sure what other problems it causes. On this code:

sub _build_login {
    my ( $dsl, $coderef ) = @_;
    return sub {
        $conf ||= { _default_conf(), %{ plugin_setting() } }; # lazy
        my $request = $dsl->app->context->request;
        if ( $dsl->app->session( $conf->{logged_in_key} ) ) {
            goto $coderef;
        }
        else {
            my $params = $request->params;
            my $data =
              { $conf->{callback_key} => $request->uri_for( $request->path, $params->{query} ) };
            for my $k ( @{ $conf->{passthrough} } ) {
                $data->{$k} = $params->{$k} if $params->{$k};
            }
            return $dsl->app->context->redirect( $request->uri_for( $conf->{login_route}, $data ) );
        }
    };
}

If we take the line $conf ||= { _default_conf(), %{ plugin_setting() } }; # lazy and move it outside the sub {} it starts to work. It's the plugin_setting() call that will fail later if we don't do this.

I'll ask the Auth::Tiny author about this.

@melo melo referenced this issue in PerlDancer/Dancer2-Plugin-Auth-Tiny Sep 17, 2013

Closed

Move configuration management to _conf_for(): #2

Contributor

melo commented Sep 17, 2013

Added a PR for @dagolden to fix the issue from Auth::Tiny perspective, still needs tests though.

But I think the Dancer2::Plugin API should also be updated. We should allow the caller to provide us a $dsl object instead of relying only on _get_dsl() because some new keyword might need to call some of our methods later, and they will have a $dsl around, but not the call stack we need.

Time permits, I'll add a PR for this.

For now, I think the culprit is Tiny::Auth and how it uses the Dancer2::Plugin API.

@melo melo closed this Sep 17, 2013

Contributor

melo commented Sep 17, 2013

Yeah, this Dancer2::Plugin API is killing me :)

I get this error now:

DEPRECATED: Dancer2::Plugin::Auth::Tiny calls 'dsl' instead of '$dsl->dsl'. at /Users/melo/Documents/work/ISI/mush/app/elib/dancer2-plugin-auth-tiny/lib/Dancer2/Plugin/Auth/Tiny.pm line 61.

(the offending line is this one https://github.com/melo/dancer2-plugin-auth-tiny/blob/master/lib/Dancer2/Plugin/Auth/Tiny.pm#L61)

The problem is that my call to plugin_settings() during setup will find the legacy dsl() import and trigger the warning. I believe this line https://github.com/PerlDancer/Dancer2/blob/master/lib/Dancer2/Plugin.pm#L380 and this one https://github.com/PerlDancer/Dancer2/blob/master/lib/Dancer2/Plugin.pm#L394 are there to prevent this exact situation but they use a single global scalar, so it only works for a single plugin.

I can patch it to support multiple, by making $dsl_deprecation_wrapper a hash and use caller() class as the key, but the correct way is to just stop importing all those DSL symbols.

Ideally we should just get rid of them, but some old plugins might be depending on this to work. So I've created this PR: #469

It gives you a :no_dsl import tag to bypass that import.

With that, and using it on ::Auth::Tiny, everything seems to be working. More testing is forthcoming.

@melo melo reopened this Sep 17, 2013

Owner

ambs commented Sep 19, 2013

I would be happy to request $dsl-> on all calls from Plugins. Probably create a deprectation warning for modules that do not use it, and remove it completely some time later.

Contributor

melo commented Sep 20, 2013

That is not the problem. Or the problem is before we get to the $dsl call.

Dancer2::Plugin implements an Export-based API: you call plugin_settings() as a function, not as a method, so it needs to find the context via caller and that is were _get_dsl() comes into play.

I would need to dig deeper into how the DSL exports his symbols into the app, and how we could get the Plugin instance for the app before we have an answer on how to find the $dsl you need.

But from my POV, my latest comment is the important one. We need the :no_dsl to move towards decent modern plugins without the burden of the legacy DSL exports.

Owner

ambs commented Sep 20, 2013

@melo, yeah, I know. If plugin_settings gets called with the $dsl things get easier.

Given that there is no compatibility between Dancer1 and Dancer2 plugins I can't see a good reason to keep that functionality active (that is known to be buggy). Just deprecate somehow the call to plugin_settings() and any other relevant without being as a method, and in a future release just remove it.

Contributor

melo commented Sep 20, 2013

@ambs :), the entire Dancer2::Plugin API is based on functions, not methods... You would need to deprecate the entire API.

Is not that I disagree, I don't like the current API at all, but it might be a bit brusk without coming up with an alternative.

Contributor

cym0n commented Nov 2, 2013

Thank you to this thread I solved a problem with a plugin of mine!
However, there's still something I cant' understand. Why this problem didn't happen on my developement server but just deploying the code on the NGINX+Plack/Starman?

Contributor

melo commented Nov 4, 2013

My memory fails me, I investigated this a long time ago, but if I recall correctly, when you use the builtin server, you will have the main app in your caller tree, but not when you use Starman or Starlet. This is due to the way the two systems (the standard HTTP::Server server and the other two) startup.

It's very easy to check: edit your local Dancer2 copy, and add a Carp confess to _get_dsl if he is ready to return undef, and compare the two stack traces, one on Starman and the other on the default server. You'll see they diverge on the main app.pl. The dance() call does different things: on the default server it does not return, but on the others, it does, it returns the app.

Again, this is from memory, I might have some details wrong...

We still need this patch to have some plugins running on 0.10 BTW, I'm sticking with 0.08 for now...

Bye,

Owner

sukria commented Nov 4, 2013

The :no_dsl flag is a good idea for us to move forward, I'll do that.

@melo melo closed this Nov 4, 2013

Contributor

melo commented Nov 4, 2013

Sorry, closed by mistake...

@sukria I have this pull request #469, tell me what you need on top of that, I might have some time to add it...

@melo melo reopened this Nov 4, 2013

Owner

sukria commented Nov 4, 2013

#469 is merged now, thanks.

@satmovi satmovi referenced this issue in ironcamel/Dancer2-Plugin-DBIC May 13, 2014

Closed

DEPRECATED: Dancer2::Plugin::DBIC calls #5

Owner

xsawyerx commented May 24, 2014

Every time I read this issue I get a headache. @melo, is it possible to summarize this issue? Is it resolved? Should we start a new issue that can summarize whatever isn't handled here?

@xsawyerx xsawyerx removed this from the 0.11 milestone May 24, 2014

Owner

xsawyerx commented Nov 26, 2014

I believe this issue is resolved. If someone feels this needs more attention and wasn't resolved, reopen.

@xsawyerx xsawyerx closed this Nov 26, 2014

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