Compilation fails with no default features
Closed this issue · 3 comments
0.15.1 and 0.15.2 introduced feature changes/additions I don't necessarily agree with so I don't know what kind of PR to submit for this.
Having both a resolve-http and reqwest features that slightly differ in semantics makes no sense, since resolve-http includes the reqwest feature, it alone should be enough to enable the HTTP(S) schema resolution feature.
Also I believe exposing optional features of dependencies is just a way to pollute the features of this library and also leads to issues like this one. #343 could have been solved simply by suggesting the addition of reqwest = { version = "*", features = [ "rustls-tls" ] }, the current solution raises the question why only native-tls and rustls are exposed while reqwest has a handful more of TLS features and combinations.
Hi @tamasfe
First of all, thank you for opening this issue, as it makes clear to me that I didn't thoroughly think about those changes, and missed that they might lead to such consequences. Sorry for the complications 🙏
My intention was to avoid obscure errors like #343, and I got some inspiration from how Sentry handles their transports.
If there is no TLS feature in reqwest, then the error message from hyper isn't that helpful - error trying to connect: invalid URL, scheme is not http, and probably it would be better to improve the error message there instead of trying to guess the real cause from the message :( There is a closed issue in the Hyper repo that contains concerns about the error message, but those concerns remain unresolved.
I am inclined to think that for now this problem should be documented in the installation doc section, and those changes reverted, as they cause more harm than actually solve the problem. What do you think? Or is there any reliable way to prevent such cases as #343 upfront?
I am inclined to think that for now this problem should be documented in the installation doc section, and those changes reverted, as they cause more harm than actually solve the problem. What do you think? Or is there any reliable way to prevent such cases as #343 upfront?
I believe the best way to go here is really just documentation indeed. The misleading error message is unfortunate, but I still believe that libraries should not enable more features of dependencies than they need and should not re-export features unless they are affected by them.
If it was just a TLS on/off switch for reqwest, it could probably make sense to re-export it as a feature, but with the amount of TLS options it has, it's a lot easier to let the user configure it however they want instead.
Thank you for sharing your thoughts on this! I agree with you.
#360 should resolve this.