arlyon / async-stripe

Async (and blocking!) Rust bindings for the Stripe API
https://payments.rs
Apache License 2.0
436 stars 127 forks source link

bug: error serializing or deserializing a request for response of stripe::Subscription::delete #461

Closed skeptrunedev closed 5 months ago

skeptrunedev commented 8 months ago

Describe the bug

When deleting a subscription like so - stripe::Subscription::delete(&stripe_client, &subscription_stripe_id) - the subscription properly deletes relative to stripe externally. Meaning that after this runs for a given subscription id, the subscription is properly marked as canceled on the stripe dashboard for me and I can see the webhook fire.

However, the error in the following scenario:

.map_err(|e| {
            log::error!("Failed to cancel stripe subscription: {}", e);
})

regarding the Rust code is error serializing or deserializing a request.

Something must be wrong regarding the way the return type is attempting to be deserialized.

To Reproduce

Use the stripe::Subscription::delete function and look at the result.

Expected behavior

The proper deserialized response is returned when a subscription is deleted for the the Subscription::delete function.

Code snippets

stripe::Subscription::delete(&stripe_client, &subscription_stripe_id)
        .await
        .map_err(|e| {
            log::error!("Failed to cancel stripe subscription: {}", e);
            DefaultError {
                message: "Request to stripe failed",
            }
        })?;


### OS

linux Ubuntu 22.04

### Rust version

stable-x86_64-unknown-linux-gnu

### Library version

0.26.0

### API version

2023-10-16

### Additional context

Not the biggest deal in the world because I am able to make this request manually instead, but it's not ideal. Would be great to use the crate here instead of hacking it. 
augustoccesar commented 8 months ago

Can confirm that this is an issue when using the Subscription::delete that is on the generated folder. The issue is that it expects a Deleted type to be returned, which is not true for this case. The correct way to "delete" a Stripe subscription is by canceling it with Subscription::cancel. Which is correctly implemented on subscription_ext.

So, to simply fix your case, you can just replace

stripe::Subscription::delete(&stripe_client, &subscription_stripe_id);

with

stripe::Subscription::cancel(&stripe_client, &subscription_stripe_id, CancelSubscription::new());

and you should get what you are looking for.

But I guess the ideal solution would to either fix the generated code for the subscription deletion or not export it as the Subscription::cancel is the correct one

augustoccesar commented 8 months ago

But I guess the ideal solution would to either fix the generated code for the subscription deletion or not export it as the Subscription::cancel is the correct one

I believe this will be fixed on the codegen revamp PR (https://github.com/arlyon/async-stripe/pull/452)

skeptrunedev commented 8 months ago

Can confirm that this is an issue when using the Subscription::delete that is on the generated folder. The issue is that it expects a Deleted type to be returned, which is not true for this case. The correct way to "delete" a Stripe subscription is by canceling it with Subscription::cancel. Which is correctly implemented on subscription_ext.

So, to simply fix your case, you can just replace

stripe::Subscription::delete(&stripe_client, &subscription_stripe_id);

with

stripe::Subscription::cancel(&stripe_client, &subscription_stripe_id, CancelSubscription::new());

and you should get what you are looking for.

But I guess the ideal solution would to either fix the generated code for the subscription deletion or not export it as the Subscription::cancel is the correct one

Got it, that makes a lot of sense.

mzeitlin11 commented 7 months ago

I believe this will be fixed on the codegen revamp PR (#452)

Yep - definitely will be fixed because the delete method won't exist anymore! (Right now it looks like cancel and delete hit the same API endpoint, but delete just has the wrong return signature).

skeptrunedev commented 7 months ago

We did switch over to using the cancel route now, so this is no longer an active issue for us. Thanks for your help @augustoccesar