Event#refresh doesn't work

[nightpool]

[1] pry(main)> RUBY_VERSION
=> "2.6.3"
[3] pry(main)> Stripe::VERSION
=> "5.22.0"
[4] pry(main)> Stripe.api_version
=> "2020-03-02"

The .refresh resource method gives an error when used on events.

event = Stripe::Event.construct_from(webhook_params)
event.refresh # throws ArgumentError

You can see the stack-trace below

[2] pry(#<StripeController>)> event.refresh
ArgumentError: wrong number of arguments (given 3, expected 0)
from gems/stripe-5.22.0/lib/stripe/stripe_object.rb:325:in `block (3 levels) in add_accessors'
[3] pry(#<StripeController>)> wtf!
Exception: ArgumentError: wrong number of arguments (given 3, expected 0)
--
0: gems/stripe-5.22.0/lib/stripe/stripe_object.rb:325:in `block (3 levels) in add_accessors'
1: gems/stripe-5.22.0/lib/stripe/api_resource.rb:96:in `refresh'
2: (pry):2:in `webhook'

I believe this is because the Event API object has a request property, which overrides the .request method provided by https://github.com/stripe/stripe-ruby/blob/master/lib/stripe/api_operations/request.rb, which refresh uses to fetch a new copy of the object:

stripe-ruby/lib/stripe/api_resource.rb

Line 96 in 403f9b2

resp, opts = request(:get, resource_url, @retrieve_params)

[remi-stripe]

@nightpool Thanks for the report. Can you clarify why you are using event.refresh in your code? I don't get why you would need to do that.

[nightpool]

@remi-stripe Hi Remi, nice to meet you! Thanks for the quick response.

We occasionally get out-of-order webhooks from Stripe. For example, about 5-10% of the time, we get payment_intent.amount_capturable_updated before we receive payment_intent.created. If we updated the state of our database based on the information in these webhooks alone, we'd run into a situation where a payment intent would show in our database as requires_payment_method with capturable_amout: 0 when Stripe shows it as capture_required with the correct capturable amount.

Ideally, we'd like to use some sort of monotonically increasing lock_version from Stripe to guard against this, but as Stripe doesn't seem to provide this, we decided that a "good enough for now" way to prevent this problem would be to refetch the state of the object from Stripe before processing any webhook.

Now that I spell that all out, it's possible that Event#refresh wouldn't give us what we want anyway, since the state of the included object might be "frozen" in the Event at the time it's created. The Stripe API docs say "In each case, the data hash will have an attribute called object and its value will be the same as retrieving the same object directly from the API", which doesn't provide clarity on whether the data hash is created at the time of fetch or the time of the event's creation.

[remi-stripe]

I see, so yeah in that case refresh would never work. Events are immutable and can not be transformed. They are not even affected by API versions in the call since they are generated based on your endpoint's or account's default API version and will never change.

What you should do here is to retrieve the object associated with this event instead. So if you get a payment_intent.* event you would call the Retrieve Payment Intent API with event.data.id for example.

[nightpool]

@remi-stripe yep! just hoping for something a little cleaner then event.data.object.refresh. Either way, from a library perspective, events should either implement this method or they shouldn't, it probably shouldn't error out confusingly :P

(P.S i think you probably mean event.data.object.id?)

[remi-stripe]

I would recommend just not using refresh at all here. Instead, explicitly retrieve the object yourself separately. I don't think we expect usage of refresh() this way to be honest.

But I agree on the confusing error, and we'll get this fixed!

[nightpool]

@remi-stripe isn't refresh identical to retrieving the object? What would the concern be about using it here?

[remi-stripe]

I personally discourage anyone from mutating objects that way. I don't think it's as explicit, you're also mutating the content of Event which is not possible and could lead to issue (such as trying to serialize the object in the middle of an error), etc.

Obviously it works, but since you're asking (and for future readers) I would recommend always having your code be explicit about what you are doing so that it's easier to find all the call sites of a given method in the future.

[nightpool]

Ah, so the concern is about mutation? I'm using it like this:

order.update!(provider_payload: event.data.object.refresh)

Which seems pretty clear and unambiguous to me (especially since we don't use event later in the function) but i'm open to suggestions.

[nightpool]

I could probably add a dup in there to make it even more obvious, but ideally refresh would have mutating and non-mutating versions, refresh and refresh!. Unfortunately it doesn't seem to have those currently :(

[brandur-stripe]

(Just a note that we have a fix for the specific error in #936. I was able to repro this exception and confirm the fix.)

[brandur-stripe]

Thanks for the report @nightpool. We just released a fix for the ArgumentError problem as part of 5.23.1. Sorry this was delayed for so long.