Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Naming conflict for "ErrorHandler" which actually is a middleware and not a RequestHandler #190

Open
boesing opened this issue Aug 13, 2019 · 3 comments

Comments

@boesing
Copy link
Member

boesing commented Aug 13, 2019

Hey there,

I am giving some trainings regarding zend-expressive, e.g. for some colleagues.
One thing, which always came back to me as a question was:

Why is the ErrorHandler called ErrorHandler like all those request handlers and not ErrorMiddleware which actually suits better as its a middleware per definition.

Would like to see the next major version where the ErrorHandler is renamed to ErrorMiddleware.

Any thoughts on this?

@weierophinney
Copy link
Member

We can do this by doing the following:

  • Introduce a new minor version that creates the new class, and then has ErrorHandler extend it. The constructor can then emit an E_USER_DEPRECATION message indicating the new name, and how to configure it.
  • The new major version would then remove it.

Please feel free to submit a PR against develop to accomplish the first step.

@boesing
Copy link
Member Author

boesing commented Aug 19, 2019

I like the idea of providing that new ErrorMiddleware.
I am not sure if triggering a deprecation message is a good idea.
As of zend-expressive, the ErrorHandler is being automatically set to the configuration.
So after updating zend-stratigility, every zend-expressive project will receive deprecation messages.

I would rather implement just a @deprecated tag, so that the handler can be safely removed in v4 instead.
A migration guide should probably mention that the registered delegators should attach the listeners to the new middleware, as just "aliasing" wont be enough. (Some more details can be found in this comment zendframework/zend-mvc#294 (comment))

I will provide a PR shortly with the changes.

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-stratigility; a new issue has been opened at laminas/laminas-stratigility#3.

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

No branches or pull requests

2 participants