@jplatte feel free to review :)

I don' t hink we want Option to be the same as the normal extractor and just discard errors.

I think it would be better to surface the parsing errors and return None only if there weren't any. I believe that was the idea behind OptionalFromRequestParts, when you just want to discard errors, you should use Result<T, _>. Arguably the full extractor should also differentiate between missing and malformed headers so this might have to wait until the next breaking release.

Others' opinions might differ and we can go ahead with this in that case, if not I'm not sure where that would leave this PR. We could merge it and change it along with the other extractor or just wait with it.

My availability is limited for the next two weeks or so, but I'll get to reviewing it some time next month.

I don' think we want Option to be the same as the normal extractor and just discard errors.

@mladedav the Host extractor in its current state already swallows any parsing errors and just discards the header. the only rejection that it currently returns is FailedToResolveHost, which is equivalent to the None case, so this refactored implementation does not discard more errors than before due to this particular OptionalFromRequestParts implementation using type Rejection = Infallible.

see also #3177 (comment) 😉

Yeah, I get that it doesn't discard more errors than the other extractor, but that doesn't change that it does discard them. I appreciate we want the two to work as similarly as possible, but I'm not sure that this addition is really needed then.

The only reason I can see why people want this is because people are used to Option being just Result where all errors are conflated into None. Which I think we should not promote because it already is not the case for some extractors and it will only get more surprising. In the linked issue someone seemed genuinely surprised they can just extract Result to get basically the same thing so I think this is just an issue with education rather than users really wanting Options specifically.

I generally don't think we should have any OptionalFromRequestParts extractors where the implementation can be done with Result::ok. That doesn't mean we should have the optional extractor here, I just think it should mean something slightly different which is arguably also different from what the main extractor does right now.