fix: fix some edge cases for HTTP integration#465
Conversation
There was a problem hiding this comment.
There are some improvements to the error mapping here. This is on the guest side, mapping from WASI to Python.
I was debugging #464 and only got error messages a la Err { value: () }" because some of the respective WASI methods return Result<..., ()>. While you can get the Python backtrace to find out which method was called, it still seemed rather bad as a debugging experience. So I've improved these error messages.
There was a problem hiding this comment.
Actual bug fix: try to peak into the reqwests::Error to map the error to a proper WASI error code.
There was a problem hiding this comment.
The change here looks bigger than it is due to a changed indentation. It really just introduces:
- IPv6 support
- two low-level failure modes that we can emulate
Both is now possible due to #453, showing that this "use home-grown test harness" approach pays off.
| }), | ||
| ..Default::default() | ||
| }, | ||
| // https://github.com/influxdata/datafusion-udf-wasm/issues/464 |
There was a problem hiding this comment.
Yeah, using IPv6 addresses directly in URLs seems to be broken at the moment. My guess is that this is a bug in urllib3 (i.e. the Python guest code). Need to debug this later.
| ErrorKind::TimedOut => { | ||
| return HttpErrorCode::ConnectionTimeout; | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
Note that you CANNOT match all kinds because that enum is non-exhaustive.
| "#; | ||
| const NUMBER_OF_IMPLEMENTATIONS: usize = 2; | ||
|
|
||
| let servers = BTreeMap::from([ |
There was a problem hiding this comment.
The integration test now uses multiple test servers because of all the different behaviors we need to emulate. There will be even more in the future when I get to #434. I think the flexibility of this is actually kinda nice.
There was a problem hiding this comment.
This is really nice, I like this 💯
| "#; | ||
| const NUMBER_OF_IMPLEMENTATIONS: usize = 2; | ||
|
|
||
| let servers = BTreeMap::from([ |
There was a problem hiding this comment.
This is really nice, I like this 💯
2 participants
Working towards #448.