Make GrpcErrorsAsFailures more flexible

[davidpdrsn]

Previously GrpcErrorsAsFailures would only consider Ok as a success. However for servers it might be useful to also consider Not Found or Invalid Arguments as success (or at least not consider them failures) since they're likely caused by the client.

This PR adds GrcpErrorsAsFailures::success_codes which takes a bitmask of the codes that are considered successes.

Example usage:

// Response classifier that doesn't consider `Ok`, `Invalid Argument`, or `Not Found` as
// failures
let classifier = GrpcErrorsAsFailures::new()
    .success_codes(GrpcCodeBitmask::INVALID_ARGUMENT | GrpcCodeBitmask::NOT_FOUND);

let service = ServiceBuilder::new()
    .layer(TraceLayer::new(SharedClassifier::new(classifier)))
    .service(service);

I went with the name GrpcCodeBitmask because I think it communicates quite somewhat clearly what it does. At least to people who know what a bitmask is. At first I considered just calling it GrpcCode but that might be confusing since its used to match multiple codes and doesn't just represent a single code.

Maybe the name GrpcErrorsAsFailures isn't very accurate anymore 🤔 Maybe just GrpcClassifier would be better. Although its more broad.

It does add a new public dependency on bitflags but I'm okay with that. The crate is maintained by people from the Rust team, doesn't have dependencies, and has been 1.0 for over 4 years. axum also has a public dependency on it.

Fixes #126

[davidpdrsn]

@tamuhey you filed the original issue about this. What do you think about this API?

[davidpdrsn]

@hawkw you were quite involved in the original design of tower-http's response classification stuff. Wanna give this a look?

[seanmonstar]

One option so as to not expose the bitmask is just to keep that internal. Allow people to call .success_code() multiple times with different codes, and build up the flags internally.

I'm a fan of this because I also feel like bit flags aren't the most intuitive thing to use for many programmers. At least I've been confused a few times whether I want to use | or &.

[davidpdrsn]

Yeah thats true 🤔 Although that would require an enum with each variants for each, although that wouldn't be too bad.

I'm personally fine with bitflags but I've also used it a bunch in axum so maybe I'm just used to it.

[seanmonstar]

No, you could use bit flags internally, just let the user pass in a Code and you can do the bit fiddling internally.

Like for a user:

layer
    .with_success(Code::OK)
    .with_success(Code::CANCELED)

[Nehliin]

Nice! I personally think the exposed bitflags actually clearer but that's just me. Actually now that I think about it & would just be a fotgun if the bitflags are exposed since its impossible for a status to match two different codes.

[davidpdrsn]

No, you could use bit flags internally, just let the user pass in a Code and you can do the bit fiddling internally.

Like for a user:

layer
    .with_success(Code::OK)
    .with_success(Code::CANCELED)

Yep that also what I meant. I'll play around with the API a bit today.

[davidpdrsn]

I've changed it and the bitmask is no longer public. API is now:

use tower_http::classify::{GrpcErrorsAsFailures, GrpcCode};

let classifier = GrpcErrorsAsFailures::new()
    .with_success(GrpcCode::InvalidArgument)
    .with_success(GrpcCode::NotFound);

[tamuhey]

@davidpdrsn Looks perfect for me, thanks!