contrib/echo: wrap appsec error in echo.HTTPError

[Hellzy]

What does this PR do?

This change ensures that the error returned by the appsec echo middleware is always an echo.HTTPError (or nil).
This is needed so that the top level APM code doesn't override the response status span tag with 500, which
happens if it encounters any other error type than echo.HTTPError

Motivation

This fixes a bug spotted while enabling user blocking system-tests.

Reviewer's Checklist

• If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
• Changed code has unit tests for its functionality.
• If this interacts with the agent in a new way, a system test has been added.

[eliottness]

Can we have a link to the test/job failing because of this maybe ?

[Julio-Guerra]

Glad you caught this before release 👏
It still needs a test to reproduce and confirm the fix please, if you could add it 🙏
Also, I think this simple handler isn't handled properly by you post-handler error management:

func myHandler(ctx echo.Context) error {
    return errors.New("my error without any response")
}

where you might end up with the expected behaviour by chance but our monitored HTTP status code will be 0, while it will actually be 500 if I understood correctly echo's behaviour.

[pr-commenter]

Benchmarks

Comparing candidate commit b20d07f in PR branch francois.mazeau/user-blocking-error with baseline commit eae71da in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 6 cases.

[Hellzy]

Glad you caught this before release clap It still needs a test to reproduce and confirm the fix please, if you could add it pray Also, I think this simple handler isn't handled properly by you post-handler error management:

func myHandler(ctx echo.Context) error {
    return errors.New("my error without any response")
}

where you might end up with the expected behaviour by chance but our monitored HTTP status code will be 0, while it will actually be 500 if I understood correctly echo's behaviour.

What changed:

We now call the echo error handlers (echo.Context.Error()) when the middleware returns an error. This provides several benefits:

• The response status gets committed
• We can simplify our wrapper around the response writer to simply expose Status(). We don't short-circuit it anymore, we simply make sure that we can read the status code that gets committed.
• We are guaranteed to follow APM's behaviour when it comes to status code handling
• We don't get in the way of users that want echo to generate the response/status code for them and simply return custom errors (i.e we let echo handle this directly and just read the result).