Setting headers in error response about huma HOT 6 CLOSED

Hey @danielgtaylor, I hit a similar limitation trying to send headers for errors and found this issue.

I'm implementing a route-specific secondary rate limiter (after a general IP-based limiter) that exponentially increases the required time between password resets based on the POSTed email address.

It seems that:

• the middleware approach won't work because it hasn't unmarshalled and validated the request body at that point.
• the handler approach won't work because I can't send rate limit headers back with a 429 status.
• transformers would split up my rate limiting logic between the handler and transformer with a custom error in between.

I've reviewed the wrapping solution in #387 and it would work well for me.

from huma.

@danielgtaylor the provided approach looks great! I took a close look at it over the weekend. I just don't feel it works in a "Huma-way" because all the rest is so well thought and designed that it feels we are missing something here. But I just can't put my finger on it. For now I would say "let's go with it" and we can always think of a new approach down the road.

from huma.

@victoraugustolls that's true, though you can use middleware to set some headers (e.g. if you never want caching, or to set rate limits, etc).

Take a look at #387 which is a proposal for enabling headers on error responses.

from huma.

I took a quick look in the cellphone (at the gym right now) and it got me thinking: wouldn't it be more natural for the user to define "header" tags in an error as an error response than to create a new error type? This would follow existing request approach. WDYT?

from huma.

@victoraugustolls sort of, and I like the desire for consistency of response headers. It's actually a bit complicated for a few reasons:

1. We want a single error type shared between all operations and error responses for consistency, and I'm not sure how to define the headers while also letting you use stuff like huma.Error404NotFound utilities.
2. Only some operations may set the headers, so documenting them for all operations seems wrong.
3. Errors in Go are weird... error is just a small interface that says nothing about how it'll be used or marshaled, and the idiomatic way in modern Go to augment errors with additional information is by wrapping and then using errors.Is or errors.As.
4. Using error was in itself kind of a compromise to enable handlers to return one success response and any number of error responses in a sane way that's also idiomatic.

For my prototype implementation in #387 I went with the wrapping approach, but it's not perfect either. As you pointed out, it's different from the existing non-error response headers, and it also means error headers are not automatically documented in the OpenAPI. I'm not too concerned about that as you can still manually document them.

Anyway I'm happy to have people chime in on alternative ideas/implementations and the pros/cons. I'll hold off on merging my PR for a bit until we can get some kind of consensus.

from huma.

@victoraugustolls any additional thoughts on this? Should I continue to hold off on the implementation?

from huma.