XeroAPI / xero-php-oauth2

Xero PHP SDK for oAuth 2 generated from Xero API OpenAPI Spec 3.0
MIT License
90 stars 64 forks source link

Strict enforcement of values causes changes in Xero to break the SDK #291

Closed IanSimpson closed 1 year ago

IanSimpson commented 2 years ago

OK - this is a bit of a curly one.

Certain values within models are enforced. In my instance this happened in the Organisation model, with the "Class" being enforced, but I suspect this is a wider issue and may need some rethinking of how the SDK behaves.

SDK you're using (please complete the following information):

Describe the bug

One of the changes in 2.13.0 was this one in Models/Accounting/Organisation.php, adding MODEL_CLASS_ULTIMATE as a constant, and adding this to the allowable values.

My customer yesterday received fatal errors when connecting an application. I was still running version 2.12.1 of the SDK. I traced it to this line of code:

$organisations = $x->getInstance()->getOrganisations($result->getTenantId())->getOrganisations();

Which is pretty innocuous. It turns out the customer had upgraded to the Ultimate plan, which version 2.12.1 of the SDK didn't know about. This then triggers an exception, which in turn triggered a fatal error.

The easy solution here is to blame me for not having the latest version of the SDK, however I have a couple of issues with that:

I also concede I didn't catch the error, but that doesn't really solve the problem - I'm just trying to get the Organisation Name here, and even if I had caught and handled the error the best we could've done is displayed an error to the end user saying we couldn't fetch the organisation name - essentially turning a fatal error into a warning.

This is sort of a problem. Essentially the way the SDK is written leads to changes in the Xero platform breaking it. We expect to have breaking changes in the SDK announced in the release notes, we don't expect breaking changes to just happen.

I expect the same thing would happen if any of these enum-esque values in the API are added to, and there's a lot of them. We can safely assume that as Xero continues to develop their product that this type of error will happen again.

To Reproduce Steps to reproduce the behavior:

  1. Set up an application with the latest version of the SDK
  2. Wait for Xero to release a new pricing plan and upgrade your company to this plan
  3. Call the "getOrganisations" function for that connection

Expected behavior Ideally, that we wouldn't get an exception thrown. I'm really not sure what the solution is here. If the data comes from the API (instead of from the User) we should probably accept that it's valid (albeit unexpected). Maybe the SDK could trigger a warning instead of throwing an exception?

The other option I guess is that this SDK (and I guess the API spec) is updated and released well before any updates to the Product come through, and that those upcoming breaking changes in the Product are announced through a mailing list or something. The Product team probably won't want to delay every release to accommodate though, and presumably also don't want to "leak" new features in the API docs before they're officially announced.

Screenshots image