unable to impl Sender due to constraint on elastic::client::private::Sealed

[softprops]

Very nice looking crate!

I'm evaluating this as an option to use aws's hosted elastic search. aws requires all requests to be signed which seems most doable with this crate if I implement a Sender that signs requests before sending them. However it seems impl Sender requires implementing a trait that's not visible outside of this crate, elastic::client::private::Sealed.

Do you have another recommendation on how I could accomplish request signing?

[KodrAus]

Hi @softprops thanks for reaching out!

tl;dr We don't accommodate this yet, but I have an idea of how we could reasonably quickly.

This is what the Sender trait is for in the long term, but unfortunately it isn't quite living up to expectations yet, because even if you implement Sender the rest of the client code is expecting there to be only 2 concrete Sender implementations. So most client methods wouldn't be available. That's a terrible experience, so I've added the Sealed trait to it until I get the rest of the client to a point where you can reasonably implement your own Sender.

It looks like right now I haven't really got anything in the API that would make this possible, which is a shame.

For cases like this where you just want a chance to inspect and tweak the raw request before it goes out I'd be on board with adding a function to the client that can be used to inspect and mutate it:

let client = AsyncClientBuilder::new()
    .pre_send_raw(|request| {
        // `request` is `reqwest::async::Request` or `RequestBuilder` here
        // sign the request and either return it or a totally new one
    })

I'd held off on this because it leaks the underlying http library, overlaps with Senders and is a big black box for messing with requests. But since it's just part of the default Senders I think it's reasonable, and quick to implement and convenient when you need it. Maybe I'll put it behind a feature gate.

What do you think?

[softprops]

That would be perfect. The ability to get a handle on the request before it's sent to compute a request signature is precisely what I need.

[KodrAus]

I've got an implementation of this on the vNext branch. You can see it here. This will make it in to the next breaking release, which I'm hoping to push out before Christmas.

[MrArnoldPalmer]

Hey @softprops wondering if you ever got aws request signing working using the pre_send_raw method on the client builder? I'm using rusoto for some other stuff and trying to use the existing credential provider and their SignedRequest logic to avoid writing all the signing logic myself but haven't had luck. Would love to see an example if you have one?

Also its been awhile since this was initially opened. Is opening the Sender trait for implementation still the plan @KodrAus ?

Fantastic crate btw. (shout out to softprops as well for all the rust + aws tools)

[KodrAus]

Hi @MrArnoldPalmer 👋

Even though it's been a long time, the state of the world hasn't actually moved that far since then. I've only just managed to ship a prerelease build onto crates.io that includes the pre_send_raw API.

It looks like pre_send_raw might be a little clumsy for that SignedRequest API. I'm guessing you'd need to copy the request data into a SignedRequest structure, sign it, then copy the relevant headers back. It's not ideal.

The Sender trait promises more than it can actually deliver, unfortunately because its concrete implementations are sprinkled around in impl blocks. So a new Sender would be missing most methods. I'd be up for reworking them though so you'd implement either a SyncSender or AsyncSender trait instead of Sender directly, and make our impl blocks that expect a concrete SyncSender or AsyncSender generic over those new traits:

trait SyncSender: Sender {
    ...
}

impl<T: SyncSender> Sender for T {}

trait AsyncSender: Sender {
    ...
}

// NOTE: This won't be coherent, but could work around it
impl<T: AsyncSender> Sender for T {}

Then we basically remove pre_send_raw in favour of writing your own Sender that can encapsulate whatever http stack you want to use.

[MrArnoldPalmer]

@KodrAus awesome, looks like it will make this much easier. I'll keep an eye out for this.

I got basic signing functionality working using pre_send_raw, wasn't as hard as I thought but definitely not the prettiest code.

[ColonelThirtyTwo]

Was hoping to use this in a WASM project by implementing a sender that uses JS functions but I guess that isn't happening.

[ColonelThirtyTwo]

Might take a stab at this, because I really want to use this crate in my web app.

My idea was to have the sender specify a type to wrap the results in; ex a plain Result for the sync API and a Future for the async API... unfortunately this needs generic associated types which isn't implemented yet.

[ColonelThirtyTwo]

So this will require a bit of a refactor around the Client, Sender, and SniffedNodes.

Basically:

• SyncSender would need to return a result,
• AsyncSender would need to return a futures::Future,
• and StdwebSender would need to return a Javascript future object.

So it's not as simple as having one sync sender and one async sender with multiple implementations. The receiving mechanism has to be generic too. In particular the code for refreshing SniffedNodes needs to be pulled into the client's implementation, since it's the only thing that knows when a response is ready.