Class property reflection issues

[m1waterman88]

We're seeing the following errors after upgrading from 6.0.2111 to 7.1.0, on line 673 in JsonMapper::createInstance. It's failing on what starts as a Client::getInvoice call using the merchant facade. Our code hasn't changed.

Class "\BitPaySDK\Model\Invoice\$overpaidAmount" not found

Class "\BitPaySDK\Model\Invoice\$underpaidAmount" not found

I'm thinking there's a reflection issue in there since these are properties---or seemingly method parameters given the missing underscore---we don't use on our side, and it appears the SDK code has getters and setters for it without actually using them outside of tests.

bitpay/sdk: 7.1.0
PHP: 8.1

Can you let me know if there's something we need to change on our end that you can surmise from this? Of course, let me know if you need any more information.

[bobbrodie]

Hi there @m1waterman88 ! We will take a look at this tonight, thank you so much for reporting.

[bobbrodie]

@m1waterman88 we're checking this out. JsonMapper checks types both in the objects and the annotations. In 7.1.0, the annotation doesn't have a type specified, so we added it in one of the recent pull requests that cleans up documentation, so in master we have tagged each as int:

    /**
     * Sets overpaidAmount
     *
     * This parameter will be returned on the invoice object
     * if the invoice was overpaid ("exceptionStatus": "paidOver").
     * It equals to the absolute difference between amountPaid
     * and paymentTotals for the corresponding transactionCurrency used.
     *
     * @param int $overpaidAmount the overpaid amount
     */
    public function setOverpaidAmount($overpaidAmount)
    {
        $this->_overpaidAmount = $overpaidAmount;
    }

And:

    /**
     * Sets underpaidAmount
     *
     * This parameter will be returned on the invoice object
     * if the invoice was underpaid ("exceptionStatus": "paidPartial").
     * It equals to the absolute difference between amountPaid
     * and paymentTotals for the corresponding transactionCurrency used.
     *
     * @param int $underpaidAmount the underpaid amount
     */
    public function setUnderpaidAmount($underpaidAmount)
    {
        $this->_underpaidAmount = $underpaidAmount;
    }

We are checking the rest of the annotations and running a few tests before we do a release of 7.2.0. If you could, would you be able to share a simple replication method? That'll ensure we're covering your scenario. Thanks!

[doliveira-v]

Hey @bobbrodie thanks for checking this so quickly, the information we pass to the sdk is really just an invoice ID (not sure if they're safe to share, so I cropped the image) to bitpay's getInvoice method and no other parameters.

[bobbrodie]

Wonderful, thank you so much! We’re testing a change and will keep everyone posted 🙂

[bobbrodie]

Hi @m1waterman88 & @doliveira-v! we're running through some manual testing and wanted to check if you could share an example of a JSON response (just the non-sensitive data) you've received from the API that throws the error. Thanks!

[doliveira-v]

@bobbrodie please let me know if that's enough information, that's what we're getting in our sentry log

[bobbrodie]

Thank you so much @doliveira-v, this is where it gets interesting. The response doesn't have overpaidAmount or underpaidAmount in it, which is typical. We've tested creating an invoice and getting an invoice with PHP 8.1 and 8.2 with SDK 7.1.0 and so far have been unsuccessful in replicating (even in the new version we're building that has integration tests).

That said, we've adjusted the annotations in master, so if you could try the following locally or in a test environment (some non-disruptive place) we'd be grateful:

1. Update your composer.json to use dev-master for the SDK version
2. Just to be safe, remove your vendor/ directory and composer.lock file and run composer install OR run composer update bitpay/sdk
3. Re-run your test

Thanks again for all the feedback - since we're not in your application it is extremely helpful!

[doliveira-v]

@bobbrodie alright, I see your point, we're setting up test cases in our dev environment currently, in the meanwhile can you please provide your next estimated release date for 7.2?

[bobbrodie]

Hi @doliveira-v we're working on our manual testing and should be releasing this week.

[doliveira-v]

Hi @bobbrodie just checking, are we on track for 7.2 this week or would you say most likely next week?

[bobbrodie]

Hi @doliveira-v we are working on the release this week (should be over the weekend), but would like to confirm if testing against dev-master resolved your issue? We haven't been able to replicate in a real-world scenario but have updated the annotations, so we want to ensure that the changes have addressed it for you. Thanks!

[doliveira-v]

@bobbrodie thanks for replying, test assertions work fine in local for dev-master , but we can't upgrade production to a dependency flagged as dev-master maybe @m1waterman88 can be able to reproduce the error somehow in a unit test in his local/dev but ideally we would just like to update to v7.2 and hopefully that'd fix the issue in prod. I will keep an eye on it over the weekend, thank you!

[bobbrodie]

Hey everyone, we've released 7.2.0. We'll close this for now, but please let us know if we need to reopen. Thanks!