search

bitpay / php-bitpay-client

PHP implementation for the BitPay cryptographically secure RESTful API
MIT License
165 stars 149 forks source link

Our production client died due to breaks in this client. #269

Open hopeseekr opened 6 years ago

hopeseekr commented 6 years ago

The Problem

  1. php-bitpay-client throws an Exception (on production) for mere Notices. This is a huge risk to end-clients. bitpay/php-client/src/Bitpay/Client/Client.php line
  2. Our server logs started filling up with [2018-03-31 02:11:53] production.ERROR: Could not generate a Bitpay Invoice:Undefined index: btcPrice
  3. The source of the problem is bitpay/php-client/src/Bitpay/Client/Client.php.
  4. The Bitpay API apparently broke backward compatibility and no longer sends the btcPrice nor btcPaid.

The Solution

Ideally, bitpay's API wouldn't break backward compatibility, at least without notice.

Barring that, you must tweak the following code in Client.php:

        if (false !== $error_message) {
            throw new \Exception($error_message);
        }

This will definitely throw exceptions on mere notices, due to any array element from the Bitpay API being different / missing from what you expect.

Next, you must not use the base \Exception for throwing any errors, anywhere in the library. It makes it well-nigh impossible for end-developers to adequately handle cases where your client isn't working correctly.

My Workaround Hack

To save production, I had to manually edit vendor/bitpay/php-client/src/Bitpay/Client/Client.php and add the following lines in createInvoice(): Above the $invoice->setToken() on line 177:

$data['btcPrice'] = $data['btcPrice'] ?? 7000;
$data['btcPaid'] = $data['btcPaid'] ?? 0;

This is absolutely NOT ideal and I would greatly appreciate your assistance on properly rectifying this.

abixalmon commented 6 years ago

Mine too. This is really unprofessional!

pieterpoorthuis commented 6 years ago

I'm working on a v4 update, which resolves these issues.

Can you elaborate on this comment:

php-bitpay-client throws an Exception (on production) for mere Notices. This is a huge risk to end-clients. bitpay/php-client/src/Bitpay/Client/Client.php line"

Are you referring to the if (false !== $error_message) line? An error message in the BitPay response to create invoice always refers to a failure of invoice creation, so it's not a mere notice. I've currently updated the lines to: if (false !== $error_message) { throw new \Bitpay\Client\BitpayException($error_message); }

If you have other / better ideas, please let me know

hopeseekr commented 6 years ago

I've already forked your project and I am working to resolve this on my end.

I incoricate you to definitely merge my upcoming PR(s).