refactor: convert httpclient provider to bindgen

[vados-cosmonic]

Feature or Problem

This PR converts the httpclient provider to use provider-wit-bindgen, utilizing a couple features and bugfixes from other PRs:

#963
#983

Those PRs should land before this one, but it's not absolutely required.

Related Issues

#743
#680

Release Information

Consumer Impact

Testing

Built on platform(s)

• x86_64-linux
• aarch64-linux
• x86_64-darwin
• aarch64-darwin
• x86_64-windows

Tested on platform(s)

• x86_64-linux
• aarch64-linux
• x86_64-darwin
• aarch64-darwin
• x86_64-windows

Unit Test(s)

Acceptance or Integration

Pending implementation of wasi:outgoing-handler in host

Manual Verification

Tested locally

[rvolosatovs]

requesting changes to prevent (accidental) merge until we have #680 done

Regarding the backwards-incompatibility, I expected we would serialize lists of tuples as maps (that could be a parameter to the macro invocation or we can just hardcode it for now)

[rvolosatovs]

As of 2023/10/13, this code doesn't work due to conversions between map (which doesn't exist in WIT) as expressed by the Smithy contract and the list<tuple<string, list<u8>>> that WIT has to take. The error looks like this:

{"error":"actor: sending req: Host send error failed to call `wasmcloud:bus/host.call`: provider call failed: Error when handling invocation: Error when deserializing invocation: Syntax(\"invalid type: map, expected a sequence\")"}

This should be resolved by #680, as the host will likely have to munge invocations and responses to translate maps into/out of what WIT can actually understand.

#680 is not expected to be anyhow WIT-aware, message format should be fully backwards-compatible, meaning that wasi:http/outgoing-handler will be sending msgpack maps, just like before. Bindgen, however, should be able to handle that and decode a msgpack map into a list of tuples and other way round

[vados-cosmonic]

meaning that wasi:http/outgoing-handler will be sending msgpack maps, just like before.

This seems to be a bit confusingly worded -- wasi:http/outgoing-handler cannot send out msgpack maps, because WIT doesn't have a map type. Is what you mean here is that #680 will make sure that the host intercepts wasi:http/outgoing-handler calls and converts them into objects that serialize as msgpack maps?

Bindgen, however, should be able to handle that and decode a msgpack map into a list of tuples and other way round

This is going to require some deeper changes to bindgen -- it's not clear that is safe to turn every list<tuple<string, list<u8>>> or any such type into something that serializes to a type that serializes to a msgpack map, and this would break normal WIT <-> WIT semantics.

This will likely have to be some kind of configurable option.

[rvolosatovs]

meaning that wasi:http/outgoing-handler will be sending msgpack maps, just like before.

This seems to be a bit confusingly worded -- wasi:http/outgoing-handler cannot send out msgpack maps, because WIT doesn't have a map type. Is what you mean here is that #680 will make sure that the host intercepts wasi:http/outgoing-handler calls and converts them into objects that serialize as msgpack maps?

yes

Bindgen, however, should be able to handle that and decode a msgpack map into a list of tuples and other way round

This is going to require some deeper changes to bindgen -- it's not clear that is safe to turn every list<tuple<string, list<u8>>> or any such type into something that serializes to a type that serializes to a msgpack map, and this would break normal WIT <-> WIT semantics.

This will likely have to be some kind of configurable option.

Yes, ideally it's a parameter to the macro, as outlined in my original comment #745 (review), but for now we can just hard-code it, because we are the only users of this crate

[vados-cosmonic]

Thanks for the clarifications -- I'm not too keen on hard-coding it, so I'll take a little time to think about a reasonable way to do it flexibly.

[rvolosatovs]

@vados-cosmonic #804 is merged, so this is now unblocked, please rebase on latest main

this is now unblocked

[rvolosatovs]

@vados-cosmonic any idea why the provider build artifacts might have changed in this PR?
Does e.g.

cargo clippy --release -j 2 --all-targets --workspace -- --deny warnings

work locally for you with this change set?

[vados-cosmonic]

Not sure, but it looks like the test-providers build in build.rs is failing to build after the rebase -- will look into it and see what changed.

[rvolosatovs]

looks like this is blocked on #869 , but I have no permissions to approve/merge, so we'll have to wait until someone wakes up in US

[rvolosatovs]

feel free to just commit the regenerated Cargo.lock

[brooksmtownsend]

@rvolosatovs / @vados-cosmonic merged that PR!

[brooksmtownsend]

@rvolosatovs This looks good to me at a high level! Let me know if you need an additional approval here to merge but leaving it to you since you've been following

[vados-cosmonic]

Hey @brooksmtownsend this isn't ready to merge yet -- I still haven't been able to get the tests working even after rebasing -- custom build isn't passing still even with the Cargo.lock changes.

Regardless of that, given that the host implementation changed there will likely be changes required here.

Going to put this back in Draft so people aren't reviewing it.

[EDIT] Builds are fine, Tests aren't, gotta squash the bugs, will update this and change it back to ready for review when done.

[connorsmith256]

One blocking request, and a few nits. I'm assuming you'll split out the bindgen code from this commit once #963 is merged?

[vados-cosmonic]

Ah sorry @connorsmith256 refactoring bug made it's way in there... hopefully you don't have to re-approve (I just rebased to one commit again)

[thomastaylor312]

@connorsmith256 asked me to merge this for him