Add idempotency support to post

[gorup]

Hey!

I need idempotency support [1], and I hacked this together. I manually tested the post_form_options and the idempotency works. Unsure of what you all think about placing Options where it is, naming, doc-comments, etc. Also, this should/could be added to the other functions like post, delete, delete_query, etc.

It appears that Stripe clients use the model of having another method that just allows for options to be passed in, e.g. javadoc. I think this model works, especially given that it is backwards compatible.

Thanks!

[1] https://stripe.com/docs/api/idempotent_requests

[kestred]

@gorup - To handle the same patterns in stripe-rs we use the Headers struct and Client::with_headers method:

• https://github.com/wyyerd/stripe-rs/blob/master/src/params.rs#L16
• https://github.com/wyyerd/stripe-rs/blob/master/src/client/async.rs#L73

And also sometimes a single named setter:

• https://github.com/wyyerd/stripe-rs/blob/master/src/client/async.rs#L73

Can you update the PR to use the Headers struct instead? (and perhaps either a with_idempotency_key and/or set_idempotency_key method...)

[gorup]

Got it, so the method takes in the Headers struct, add the idem key to this headers struct. Will do soon(TM)

[gorup]

So looking into this, one thing that I see being a problem is that this field needs to be unique per request. To use the headers struct and the with_headers, you need to clone the entire HTTP client per request. This will 'work', but again we're cloning everything on every request, including the HttpClient which has pooled connections.

If we wanted to have a with_idempotency_key method on the (stripe) Client as you mention, which sets the key, then we have a different problem: this requires us to have an &mut reference to the Client, which I for one would not be able to use as I pass around my client in an Arc, in addition to the fact that on a successful request, we would need to then remember to remove the idempotency key from the struct, or have it be automatically taken out of the struct per-request, both requiring &mut.

Looking at the Headers struct, all of the variables are things that are very unlikely needing to change per-request. Someone may have >1 client structs floating around for different accounts or something, but the fields in Headers likely never change, or do infrequently. The idempotency key on the other had must change per requests, unless you're specifically trying to retry a failed request, or are otherwise attempting to use the idempotency key feature (maybe you have a poorly coordinated workflow in which multiple requests are made to stripe that could be identical).

Personally, the best option going forward must have new functions added which allow you to pass fields/arguments per-request, that don't require mutating the Client struct in any way. There are two ways I see that happening. The first is with a new struct like the one in my PR as of now. I see how its not ideal that now there are Headers and Options structs. The alternative would be to add add VERB_with_headers functions. You pass in a &Headers, which will be used instead of the default client one. If we also add fn copy_existing_headers(&self) -> Headers, then the users could do something like this:

fn create_sub_idem(request: CreateSubscription, client: Arc<StripeClient>) -> Result<Subscription, &'static str> {
    let mut h = client.copy_existing_headers();
    h.idempotency_key = Some(Uuid::new_v4().to_string());
    let mut attempts_left = 3;
    while attempts_left >= 0 {
        attempts_left -= 1;
        match client.post_form_with_headers(request, &h).await {
            Ok(r) => return Ok(r);
            Err(e) => error!("Got an error from stripe while creating a sub numleft {} {:?} {:?}", attempts_left, request, e),
        }
        tokio::time::delay_for(DEFAULT_RETRY_DELAY).await;
     }
     Err("ran out of attempts")
}

let me know your thoughts! Thanks