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

Renaming ErrorHandler to ErrorHandlerMiddleware #7

Conversation

boesing
Copy link
Member

@boesing boesing commented Jan 2, 2020

This re-opens zendframework/zend-stratigility#191 and handles #2

Summary

As already mentioned in zendframework/zend-stratigility#190 (migrated to #3), there is a naming conflict in the ErrorHandler.

I had some feedback of trainees which asked me, why the ErrorHandler ain't a RequestHandler, as its name says.
That made me think about the naming and I realized, its just because of the missing Middleware postfix.

However, I'd like to rename the ErrorHandler to ErrorHandlerMiddleware to avoid future confusions about where the difference between RequestHandlerInterface and MiddlewareInterface is.

The `ErrorMiddleware` is just a drop-in replacement for the `ErrorHandler` to solve a naming conflict.
As the `ErrorHandler` is implementing the `MiddlewareInterface`, it supposed to be called `ErrorMiddleware` instead.
The `ErrorHandler` should be removed in v4.
By just extending the `ErrorMiddleware`, we can remove the dedicated unit test and the duplicated code for the `ErrorHandler`
Actually, its an error handling middleware.
Renaming to `ErrorHandlerMiddleware` is more beneficial as just `ErrorMiddleware`.
@Xerkus
Copy link
Member

Xerkus commented Jan 2, 2020

I disagree with the change. Name does not say that it is a request handler but that it is an error handler, it handles errors.

While Handler might seem somewhat ambiguous, none of the other middlewares follow proposed naming convention and namespace already clearly indicates that it is a middleware.

@boesing
Copy link
Member Author

boesing commented Jan 2, 2020

@Xerkus As I've already tried to explain in zendframework/zend-stratigility#190, its not about what we as the creators of this package think is "enough" but what my workshop attendees do.
I had multiple developers from juniors to seniors which asked me about the ErrorHandler and why the handler does not end the middleware pipeline as it states to be a Handler (and handlers break chains as they do not forward the request to the next handler).
If you use a use statement to simplify the namespace, what stays in your pipeline configuration is the ErrorHandler which (for me and many others) states to be a handler.

I understand why you think so but I still think this is a useful change to make things more clear.

Just a sidenote: the NotFoundHandler is definitely a handler which does not delegate the request to the next handler and still is in the Middleware namespace. So it has to be a middleware? 🤷‍♂
Fix for that is in #8

@geerteltink geerteltink changed the base branch from develop to 3.3.x September 11, 2020 12:34
@boesing boesing closed this Oct 20, 2020
@boesing boesing deleted the deprecation/error-handler-to-error-middleware branch August 11, 2021 22:39
@boesing boesing restored the deprecation/error-handler-to-error-middleware branch August 11, 2021 22:39
@boesing boesing deleted the deprecation/error-handler-to-error-middleware branch August 11, 2021 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants