danielgtaylor / huma

Huma REST/HTTP API Framework for Golang with OpenAPI 3.1
https://huma.rocks/
MIT License
1.71k stars 134 forks source link

Setting headers in error response always appends #475

Open jeremybower opened 1 month ago

jeremybower commented 1 month ago

@danielgtaylor @victoraugustolls Thanks for the prototype solution in #386 and #387 for setting headers in error responses.

While it worked well initially, I've hit a case where I don't see solution. I'm happy to open a PR, but would appreciate your thoughts on the right approach to solve this specific issue. I know there are thoughts around a much more robust approach. At this point, I'm only trying to solve this narrow problem.

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

The issue is that, unlike headers in output structs, headers in errors are always appended. This means that I can't override the headers set by the general limiter with the headers set by the more restrictive limiter. The response with duplicate headers looks like this:

HTTP/1.1 429 Too Many Requests
Connection: close
Content-Type: application/problem+json
Link: </api/core/v1/schemas/ErrorModel.json>; rel="describedBy"
Retry-After: Mon, 10 Jun 2024 13:37:46 UTC
Retry-After: Mon, 10 Jun 2024 13:38:47 UTC
X-Ratelimit-Limit: 100
X-Ratelimit-Limit: 1
X-Ratelimit-Remaining: 98
X-Ratelimit-Remaining: 0
X-Ratelimit-Reset: 2024-06-10T13:37:47Z
X-Ratelimit-Reset: 2024-06-11T13:37:46Z
X-Ratelimit-Retry: 2024-06-10T13:37:46Z
X-Ratelimit-Retry: 2024-06-10T13:38:47Z

Ideally, in this case, I could do something like the array approach documented for output structs -- override if a single value and append when an array. But I understand that in the general case it makes more sense to alway append error headers to preserve information that might be useful when troubleshooting.

It seems that:

Other than a framework-level enhancement, the only things I can think of doing are:

Thoughts?

danielgtaylor commented 1 month ago

@jeremybower I'm still wrapping my head around this problem. Maybe you can make a small example like https://go.dev/play/p/xrlGxyVZBge to reproduce it?

That said, I wonder if you can use operation metadata to just tell the middleware when a handler will set its own rate limiting info into the response, something like https://go.dev/play/p/d_dj-_UMKM1

jeremybower commented 2 weeks ago

@danielgtaylor Here's an example of the primary and secondary rate limiter use case: https://go.dev/play/p/zOulOSxaFWL

Please let me know if I'm misunderstanding the suggestion about using operation metadata, but it seems this won't work because the primary rate limiter must always be enabled so that invalid requests that don't reach the secondary rate limiter still count against the rate limiting quota (as shown in the example with an invalid email address).

Another thing I'll point out about this example, which is purely subjective, is that there are three different ways to set the same header:

I can understand the middleware operating at a lower level with the Huma context, but it seems like the handler should be consistent about how headers are set. If I continue to pull on that thread, then it leads me to errors being similar to output structs where headers can be set in the same way.

Thanks again for your thoughts on this. I'm really enjoying using Huma.