TypeError - no implicit conversion of Hash into String when attempting to pay an invoice

[linkyndy]

Hello,

We're getting the following error when attempting to pay:

Stripe::Invoice.pay(id: stripe_subscription.latest_invoice, expand: ['payment_intent'])

an invoice:

TypeError - no implicit conversion of Hash into String:
	/Users/andrei/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/stripe-4.21.0/lib/stripe/api_resource.rb:73:in `escape'
	/Users/andrei/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/stripe-4.21.0/lib/stripe/api_resource.rb:73:in `block in custom_method'

I would assume this doesn't work because Stripe::Invoice.pay expects a positional argument for the ID, while that way, it cannot accept optional arguments, such as expand.

This works when retrieving a subscription, for example:

Stripe::Subscription.retrieve(id: 'whatever', expand: ['latest_invoice.payment_intent'])

[ob-stripe]

Hi @linkyndy. For historical / backwards compatibility reasons, the retrieve method has a different signature than other methods that affect a specific resource.

The retrieve method expects an id and an optional opts hash (for setting special headers like Stripe-Account, Stripe-Version, Idempotency-Key, etc.):

stripe-ruby/lib/stripe/api_resource.rb

Line 95 in 267eae5

def self.retrieve(id, opts = {})

Originally, id could only be a String. This worked because retrieve methods typically don't accept any other parameters. expand is an exception since it can be used in any API method. In order to add support for passing expand parameters in retrieve requests without breaking backwards compatibility, we made the method accept a Hash instead of a String for the id parameter.

Other API methods that affect a specific resource expect different arguments for the ID and for actual request parameters:

stripe-ruby/lib/stripe/api_resource.rb

Line 72 in 267eae5

define_singleton_method(name) do |id, params = {}, opts = {}|

So you need to change your call to Stripe::Invoice.pay as follows:

Stripe::Invoice.pay(stripe_subscription.latest_invoice, {expand: ['payment_intent']})

We should probably change the signature of retrieve methods to adopt the same pattern in a future major version. @brandur-stripe Do you agree?

[brandur-stripe]

Great analysis OB.

We should probably change the signature of retrieve methods to adopt the same pattern in a future major version. @brandur-stripe Do you agree?

I'm a little on the fence. On one hand, I'd certainly prefer that retrieve just take an ID like pay and other methods like it (the current system which automagically deconflates parameters and opts is too forgiving and is surely resulting in lots of inconsistent code), but on the other, this might require so many people to change code during the upgrade that it might not be worth it due to the sheer volume of pain.

Maybe we could introduce a deprecation message and then wait a long time before changing it to give people as much time to upgrade as possible, but even that might be painful in its own way.

So yeah, I'm not too sure :) Possibly if we do a big breaking change in the future we could pull this in, but I'd probably hold off until then.

[linkyndy]

Thank you both for your answers.

It may be a pain to change this interface, but keep in mind developers also feel pain and get confused by an inconsistent interface. I would make the id retrievable from within a Hash as well throughout the entire API, to address this inconsistency, and then indeed, add deprecation messages and fully address this on the next major gem version.

Thanks for your hard work!

[brandur-stripe]

@linkyndy Yeah, definitely fair enough on that point.

This isn't a full solution, but for now I'm going to try and make this incrementally better by improving the error message that's raised when passing the wrong type to one of these custom methods. See #810.