Allow BYO http impl #377
Allow BYO http impl #377
Conversation
diehuxx
requested review from
KendallWeihe,
nitro-neal,
shamilovtim,
frankhinek and
leordev
as code owners
TBD Spec Test Vectors Report (web5-rs)
❌ Missing Vectors (13)These are test vectors without any test cases.
Automatically generated at: 2024-10-03T17:03:47.684Z |
TBD Spec Test Vectors Report (web5-core-kt)
❌ Missing Vectors (15)These are test vectors without any test cases.
Automatically generated at: 2024-10-03T17:07:53.362Z |
|
Looks like you may have accidentally deleted |
| pub struct HttpResponse { | ||
| pub status_code: u16, | ||
| pub headers: HashMap<String, String>, | ||
| pub body: Vec<u8>, |
There was a problem hiding this comment.
Can we change this to pub body: Option<Vec<u8>>?
| let headers: HashMap<String, String> = HashMap::from([ | ||
| ("Host".to_string(), "{}".to_string()), | ||
| ("Connection".to_string(), "close".to_string()), | ||
| ("Accept".to_string(), "application/json".to_string()), | ||
| ]); |
There was a problem hiding this comment.
do you think all of these headers are necessary? I've noticed you have added them throughout the various http calls. I'm guessing it's slightly more proper this way? just checking this isn'y copy/pasta from our roll-our-own http client code
| pub mod jose; | ||
| pub mod json; | ||
|
|
||
| pub use http::set_http_client; |
There was a problem hiding this comment.
don't we also need to expose the HttpClient trait in order for developers to bring their own?
IMO we should simply pub mod http; everything, otherwise in tbdex-rs we're going to have to copy/pasta all the http code for it's own BYO functionality (which over there is where the hard requirement kicks in because we have to implement a foreign fetch over the wasm ffi)
If you turn on the feature flag "http_reqwest", web5 crate will supply its own http implementation that uses
reqwest.