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

Unable to register extension "nystudio107\seomatic\twigextensions\SeomaticTwigExtension #937

Closed
baileydoestech opened this issue Aug 2, 2021 · 12 comments
Labels

Comments

@baileydoestech
Copy link

baileydoestech commented Aug 2, 2021

Describe the bug

When explicitly throwing a 403 error in Craft (preventing access to the admin panel), we are seeing an internal server error that appears to be caused by the following:

[01-Aug-2021 21:18:23 Europe/London] An Error occurred while handling another error:
LogicException: Unable to register extension "nystudio107\seomatic\twigextensions\SeomaticTwigExtension" as extensions have already been initialized. in /var/app/current/vendor/twig/twig/src/ExtensionSet.php:145

Note we are only seeing this when DEV mode is disabled and logged out of the CP. This previously worked without issue (probably before Craft 3.7).

To reproduce

Steps to reproduce the behaviour:

  1. Disable DEV mode and log out of the control panel
  2. Intercept Craft CP Request in custom module (Craft::$app->getRequest()->isCpRequest)
  3. Throw a 403 error ( throw new HttpException(403, "Access to the control panel is restricted")
  4. Observe an internal server error with the exception Unable to register extension

Expected behaviour

Craft's standard error template should be displayed

Screenshots

This is shown on the front end
image

This is the error in the logs
image

This should shown on the front end:
image

Versions

  • Plugin version: 3.3.46
  • Craft version: 3.7.7
@baileydoestech
Copy link
Author

Confirmed that disabling SEOMatic resolves the issue

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

Exactly how are you intercepting the request?

SEOmatic doesn't do anything particularly fancy, it just registers the Twig extension here: https://github.com/nystudio107/craft-seomatic/blob/v3/src/Seomatic.php#L491

@baileydoestech
Copy link
Author

baileydoestech commented Aug 2, 2021

In a custom module to verify IP addresses to the control panel, essentially a simplified version of this: https://github.com/sjelfull/craft3-restrict

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

Can you show me some code though? Are you doing it inside of an event or such?

It's possible you might want to wrap it in:

        // Handler: Plugins::EVENT_AFTER_LOAD_PLUGINS
        Event::on(
            Plugins::class,
            Plugins::EVENT_AFTER_LOAD_PLUGINS,
            function () {
                // Do stuff here
            }
        );

...or some other event?

@baileydoestech
Copy link
Author

baileydoestech commented Aug 2, 2021

Thanks that did the trick, wrapping our security service call to check IP addresses in the above works as expected.

Not working:

public function init (): void
    {
        parent::init();
        self::$plugin = $this;

        $this->setComponents([
            'security'=> SecurityService::class,
        ]);

        if (Craft::$app->getRequest()->isCpRequest) {
            $this->security->checkIpAddress();
        }
}

Working:

public function init (): void
    {
        parent::init();
        self::$plugin = $this;

        $this->setComponents([
            'security'=> SecurityService::class,
        ]);

        Event::on(
            Plugins::class,
            Plugins::EVENT_AFTER_LOAD_PLUGINS,
            function () {
                if (Craft::$app->getRequest()->isCpRequest && !Craft::$app->getConfig()->general->devMode) {
                    $this->security->checkIpAddress();
                }
            }
        );
}

Is there any obvious reason why this was working previously (again I assume around the release of Craft 3.7) - it seems that SEO Matic would try and reinitialise its extensions on Craft's standard error pages (like a 403 to the admin panel).

Also is the general consensus to intercept CP requests after plugins have loaded regardless if they actually affect CP requests - in this instance why do SEO templates need to be injected into the CP at all given we don't want SEO there?

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

It could be a regression in 3.7, I don't know -- but SEOmatic isn't doing anything particularly fancy here. I'd imagine other plugins that register Twig extensions would cause the same issue.

That said, I have not run into this error on Craft 3.7 myself yet. If you're able to reproduce it with another plugin that registers a Twig extension, you might consider filing an issue with P&T

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

What's likely happening is:

  1. Your plugin loads before SEOmatic
  2. Your plugin throws an exception
  3. The exception rendering code uses Twig to do its thing, so Twig is initialized
  4. Plugin loading continues, and SEOmatic tries to register its Twig extension
  5. Twig has already been initialized, so extensions can't be registered, and an error is thrown

So any plugin that loads after yours, and also registers a Twig extension should throw this error.

In general, you shouldn't do anything that will cause Twig to render until after all plugins have loaded because of this. I've filed an issue before with P&T about formalizing the loading process to provide information on such things previously.

As for not wanting SEO, SEOmatic doesn't inject any SEO information in the CP, nor does it inject SEO tags on non-200 result code pages. But it does need to load in order to facilitate previews, etc.

@baileydoestech
Copy link
Author

Makes sense thanks, happy to close this with provided work around

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

The only thing I'm unclear on is how/why the plugin loading process would continue if a frontend exception was thrown. Perhaps you need to terminate the request yourself?

If you did that, you could avoid listening to the event.

@baileydoestech
Copy link
Author

Terminating the request doesn't resolve the issue, I prefer having Craft (or any framework we use) handle the intialisation process so as long as your insights are confirmed by P&T then this looks good.

Do you have a GH issue or discussion to reference (in case anyone else comes looking)

@khalwat
Copy link
Collaborator

khalwat commented Aug 2, 2021

It was so long ago, I can't remember honestly. It also might have been just a conversation with Brandon. I can't recall.

@baileydoestech
Copy link
Author

craftcms/cms#9671

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

No branches or pull requests

2 participants