WebApiThrottle
WebApiThrottle copied to clipboard
Don't use OnSendingHeaders
As we've decided to terminate the middleware stack and initiate creating the response, we shouldn't use OnSendingHeaders. Particularly to set the status code as middleware further down the stack can't see what response we've decided to return.
I appreciate this might be a breaking change for some people but I think it's the 'right' answer - so hopefully you might consider accepting this change.
To avoid a breaking change you could add an optional param to the middleware constructor and use a if to exclude the OnSendingHeaders section.
Hi, thanks for considering my PR.
To me that seems like a shame - to introduce the complexity of a flag to switch a behaviour that I don't believe is correct. To me the current behaviour feels like a bug - and this is a fix to that bug.
So maybe this change could wait for a major release - where breaking changes might be allowable?
I will concede that making this change breaks any code reliant on the last-minute mutation of the response, but...
The currently implemented behavior:
- does not match that of the
ThrottlingHandlerwhich has an immediate mutation - mutates the response outside of the middleware's execution scope, preventing any middleware that runs after the fact from making accurate decisions based on the response and any further mutations would be discarded
Based on those two things, to me it appears as though the currently released implementation is invalid and a bug, and that the last-minute mutation behavior just should not be permitted (and can be achieved by attaching this middleware last if it is truly desired).
In short -- I agree with JoachimC.