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

with cake plugin load, the plugin seems not to work #36

Closed
o0h opened this issue May 9, 2020 · 5 comments
Closed

with cake plugin load, the plugin seems not to work #36

o0h opened this issue May 9, 2020 · 5 comments
Milestone

Comments

@o0h
Copy link
Contributor

o0h commented May 9, 2020

#29 (comment)

After bin/cake plugin load Connehito/CakeSentry in CakePHP4, Applicaiton.php is modified like below.

@@ -39,6 +39,8 @@
      */
     public function bootstrap(): void
     {
+        $this->addPlugin('Connehito/CakeSentry');
+
         // Call parent to load bootstrap from files.
         parent::bootstrap();
 

This does not apply the Error/Logger settings correctly

@o0h o0h added this to the 3.x milestone May 9, 2020
@o0h
Copy link
Contributor Author

o0h commented May 9, 2020

Does Server::bootstrap() call pluginBootstlap()?

https://github.com/cakephp/cakephp/blame/4.0.7/src/Http/Server.php#L112-L114

Based on that, Middleware's responsibility can be supplemented for errors that occur inside of Application::handle().

@o0h
Copy link
Contributor Author

o0h commented May 10, 2020

I would like to make it available without the need for manual rewriting by the user.
To do so, it's best if it can be completed by executing a single command.
If that's not possible, I'd like to at least have only one point to rewrite.
We need to register the Logger and register the ErrorHander.

What is the approach taken by the other plugins?
Here are a few picks from Awesome-cakephp. A brief survey is described below.

whoops error handler

https://github.com/dereuromark/cakephp-whoops

Instead of using "Plugin::load()", it is recommended to replace the simple way handlers are registered.
NOTE: This plugin doesn't need to register Logger.

database logger

https://github.com/dereuromark/CakePHP-DatabaseLog

This plugin instructs the user to run "plugin load" and set up the log by rewriting app.php.
FYI: The rewriting of "app.php" seems to be much simpler and less error-prone than the rewriting of bootstrap.

airbrake

https://github.com/chrisShick/AirbrakeCake

They take a similar approach to the whoops-plugin, with, said "You don't have to enable the Plugin because it uses an error handler. Therefore, all you have to do is replace this line in the app/Config/bootstrap.php".
In addition, they don't have bootstrap.php.

@o0h
Copy link
Contributor Author

o0h commented May 10, 2020

The simplest approach would be something like this.

  • Keep the configuration as it is
    • Encapsulate the configuration changes in bootstrap.php.
  • Remove the description that recommends loading plugins by command
    • Require a manual rewrite of Application.php. In doing so.
    • Make it clear that "addPlugin()" needs to be called after " parent::bootstrap();".

@o0h
Copy link
Contributor Author

o0h commented May 16, 2020

I was aware of an issue where the error wasn't properly passed to ErrorHandler and thought it was due to the timing of the bootstrapping.
However, that was a mistake, and it was eliminated by ef4487e

It is possible that ErrorHandler and Logger settings may be overridden due to compatibility with other plugins. Since the behavior is a matter of what the user is using, it may be enough to suggest the possibility without changing the basic method.

@o0h o0h mentioned this issue May 17, 2020
@o0h
Copy link
Contributor Author

o0h commented May 17, 2020

Fix in c8b2b74

@o0h o0h closed this as completed May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant