Missing fields after composer update

[kevinpapst]

Hi Stripe Team!

After upgrading my Stripe-PHP package from 7.29.0 to 7.92.0 I am running into missing fields issue.

My CI breaks with phpstan warnings, so I had to revert to the old API.

After checking dozends of "Multiple API changes" PRs I was finally able to find the offending one:
https://github.com/stripe/stripe-php/pull/1005/files

I would like to ask for clarification (how to replace the fields if they are gone) or a new release with fixed docs for eg:
Where did all the fields go to that were removed in this PR, eg:

• Subscription::$plan
• Subscription::$tax_percent
• Subscription::$quantity
• Invoice::$tax_percent
• Invoice::$unified_proration
• InvoiceLineItem::$unified_proration

Please note, I did not check if they came back with a later release, my CI is at least is missing two of them. Here are more details.

[remi-stripe]

@kevinpapst The properties didn't disappear from the API, just from the docstrings we give in the library for classes. We consider those docstrings indicative as they are used for tests but not canonical for your code, since they are only valid for a specific API version and wouldn't match if your code used a different one.

Changing those docstrings is not something we'd change with a major version since it'd require a new major version every time an API version is released for example. In the future, if we introduced real types, like we do for go/dotnet/java, then in that case the library would be pinned to a specific API version and would get a major version every time we pin to a new API version (since it'd be a real API breaking change).

Does that make sense?

[kevinpapst]

Yes, and no (as most often) 😁 I do understand what you are saying. I am talking with a specific Stripe API version. It will have these fields, no matter which client I use. Not my first API. Probably I cannot make my point clear, written communication in a foreign language... I'll do my best to express my thoughts in english, not always easy, sorry.

Please try to see it from my perspective as a consumer of your library (not endpoint). These docblocks are the API that you give us right now. As long as we have to work with these unfortunate magic strings, I consider this your public API. And I consider it to be a reliable source of truth.

I don't see a difference between having these attributes "in real in the class" or only "faked via docblock". The only difference is on your end: if you take them out of your BC policy, its easy to say "here is a new release, find out yourself if the attributes exist or not".
If you don't consider these attributes as a binding contract, then my wish would be: remove the attributes completely. Having something that is unreliable is worse than not having anything at all. Or better: create major version updates for API changes (as mentioned in the other issue). Having a table that explains which library major version is compatible with a specific API version is totally fine. I don't expect updated docblocks for a two year old API version, but I expect the library to be stable and testable and that it documents the available attributes. And that composer updates on minor versions don't break anything.

API versioning is a tough topic, without doubt. But don't put that burden on the consumer of your library if avoidable.

If that sounds grumpy, sorry. I am trying to explain a real world issue here. I cannot deploy without CI approval and now I have to either stick with an outdated lib or exclude specific lines from the code analysis inside the heart of a SAAS business (subscription and payment workflows) just to get that approval => which is an unnecessary business risk.

[remi-stripe]

First off all, thanks a lot for the thoughtful feedback. We do really appreciate conversations like this one as they help us improve the library and consider future potential improvements and policy changes. That's how we ended up adding PHPDocs in the first place or how we moved to the client/service pattern for example after discussing the changes here with other developers. Don't worry about sounding grumpy as you just raised great points!

Please try to see it from my perspective as a consumer of your library (not endpoint). These docblocks are the API that you give us right now. As long as we have to work with these unfortunate magic strings, I consider this your public API. And I consider it to be a reliable source of truth.

While that's fair to think that, it's unfortunately not how our API versioning works today. Every time we release a breaking change in the API, we release a new API version. This can be because we rename a common parameter or property, because we change the behaviour of an API or the format/type of a property returned. For example, we might rename statement_description to statement_descriptor (we did many years go). When we do this, the latest API version will not have statement_descriptor as a property anymore and if our PHPDocs said it existed, it'd be incorrect and confusing. For that reason, the library would be updated and have the newer PHPDocs were the one for statement_description disappears, and the one for statement_descriptor appears in its place.

It's important to understand that, because you'd have the same issue if we didnt update the PHPDocs but your code used a different API version.

Or better: create major version updates for API changes (as mentioned in the other issue). Having a table that explains which library major version is compatible with a specific API version is totally fine. I don't expect updated docblocks for a two year old API version, but I expect the library to be stable and testable and that it documents the available attributes. And that composer updates on minor versions don't break anything.

This makes sense. That's the approach we took for our statically typed libraries (go, dotnet and java). It comes with the benefits you're describing where you know exactly what is available and the format. It also comes with downsides though. For example if we release a new property on Customer today and your code was built a year ago, to access it you will need the latest version of the library. Just to access that new string property, you now have to deal with potentially multiple API version changes that could drastically affect your entire code, because the newer property is only added to the latest major.

As we take backwards compatibility really seriously, we make sure that a large portion of our features are available to everyone, independent of the API version. This means that the new property on Customer would work even on a 2 or 5 years old API version. This is a huge upside of stripe-php and the other non statically types libraries. With your current code, you can just access that new property, despite the missing PHPDoc and it'd work

I don't see a difference between having these attributes "in real in the class" or only "faked via docblock".
There is a real difference here though. In the class, they'd represent what the class can actually do and you couldn't access properties that are not defined or pass a parameter that doesn't exist.
The PHPDocs are indicative, not a contract. They help developers in their IDE with auto-completion and code discovery, and they help with static code analysis or test suite, but they don't impact the runtime itself.

While it's another language, in Node.js we shipped Typescript definitions 18 months ago. At the time we talked to the community about this as we wanted the ability to release breaking changes in types without a major version to avoid fragmenting the install base of libraries and make it easier to upgrade. We confirmed with other developers that since it impacted the build and not the runtime it was common to release those type changes in minor versions.

If you don't consider these attributes as a binding contract, then my wish would be: remove the attributes completely. Having something that is unreliable is worse than not having anything at all.

That's also a fair position! I held the same position when we discussed releasing PHPDocs and discussed it with the community. It's been a while though with real usage and developers were comfortable some disappearing or changing types as it wouldn't impact production code and help with discovery of changed features.

We do agree it's not perfect though and feedback like yours helps us improve the library over time. It's the first time this is raised which is also why it's not something we've explicitly revisited since then.

[remi-stripe]

I'm going to close this issue for now, but it's still something we're thinking about as a future improvement to the library!

[kevinpapst]

Closed as won't-fix is the truth ^^

You are just one of the biggest online payment provider in the world, who cares about API contracts 👎