AaronErhardt / actix-governor

A middleware for actix-web that provides rate-limiting backed by governor.
GNU General Public License v3.0
103 stars 21 forks source link

Specify the type of error content when the key cannot be extracted #29

Closed TheAwiteb closed 2 years ago

TheAwiteb commented 2 years ago

Providing the possibility to specify the type of error content when the key cannot be extracted would be great, because it would allow to return json, html or xml.

But I think that will generate breaking changes, how do you think we can add it?

https://github.com/AaronErhardt/actix-governor/blob/main/src/key_extractor.rs#L18-L21

TheAwiteb commented 2 years ago

Maybe we need to do something like this and remove response_error from KeyExtractor

use actix_web::http::{header::ContentType, StatusCode};

trait Foo {
    type Content: ToString;

    fn content(&self) -> (Self::Content, ContentType);

    fn status(&self) -> StatusCode;
}

trait KeyExtractor: Clone {
    type KeyExtractionError: Foo;

    // ...
}
AaronErhardt commented 2 years ago

I think we should eventually clean up the KeyExtractor trait and change the extract method to return Result<Self::Key, actic_web::HttpResponse instead of Result<Self::Key, Self::KeyExtractionError>. This allows you to have maximum flexibility for the response that is returned in case of an error. This would also allow us to remove the response_error method.

Also, response_error_content could simply get a HttpResponseBuilder and return a HttpResponse for more flexibility.