HTTPErrorHandler is called multiple times from (built in) middlewares

[ForsakenHarmony]

Issue Description

It's unexpected to me that the echo error handler gets called multiple times for a single request (even if you can detect it). We log a message if a request was already handled because we expect the error handler to be the only place where error responses are sent.

A couple of middlewares, including the logger, bodydump and contrib prometheus middlewares will call echocontext.Error() to directly invoke the error handler, but then also return the error further down the stack. I understand the echocontext.Error function as a way to handle the error somewhere in the stack if you don't want to return it further.

https://github.com/labstack/echo/blob/master/middleware/body_dump.go#L82

Checklist

• [x] Dependencies installed
• [x] No typos
• [x] Searched existing issues and docs

Expected behaviour

Error handler is only called once for each request.

Actual behaviour

Error handler gets called multiple times for some requests.

Steps to reproduce

1. Write a custom error handler
2. include the body dump middleware
3. have an error in a request
4. -> body dump middleware will call the error handler directly via echocontext.Error() but also return the error, which leads to the error handler being called twice.

[arieroos]

This comment on the prometheus repo has workaround you can use: https://github.com/globocom/echo-prometheus/issues/5#issuecomment-671310135

[aldas]

or default error handler

https://github.com/labstack/echo/blob/b445958c3ce4cf34997a67ef73a30cd870170945/echo.go#L377-L381

some, if not all, of those middlewares can be changed to return that error to upstream middleware and not to use c.Error(err)

[markhildreth-gravity]

@aldas We recently ran into this as well. I believe there is a problem where there isn't a clear understanding of Committed amongst those implementing both middleware and custom HTTP error handlers. Thus, there aren't any checks being done for "Committed", which can cause problems.

This is partially a documentation issue. The documentation for middleware shows an example of calling the error handler without a Commited check. That example returns nil, but it's not obvious to a casual reader that if that middleware were to return the error like otelecho recently did, then the error handler might write the response twice.

Likewise, it's not obvious to the custom error handler implementers that they should check for committed, since the custom error handler documentation doesn't perform this check.

I feel that this needs to be clarified. Either:

1.) It's echo's responsibility to put this complexity into c.Error(), so that no third party needs to worry about this. 2.) It's the error handler's responsibility, in which case the documentation should be updated, or, 3.) It's the middleware's responsibility, in which case that documentation should be updated.