Skip to content
This repository has been archived by the owner on Sep 29, 2023. It is now read-only.

PHP-VCR with SDK causes issue from non-optional SDK debugging. #1057

Open
johnschmuff opened this issue Feb 8, 2018 · 2 comments
Open

PHP-VCR with SDK causes issue from non-optional SDK debugging. #1057

johnschmuff opened this issue Feb 8, 2018 · 2 comments
Labels

Comments

@johnschmuff
Copy link

PayPal-PHP-SDK 1.13.0
Sandbox
PHP 7.2.1

Writing the create payment code using this SDK worked fine, until I started writing unit tests around
my code to that uses the SDK. I use PHP-VCR to capture the responses into fixtures. I noticed as soon
as I implemented PHP-VCR the SDK was complaining. Below is the error I received in terminal:

call_user_func() expects parameter 1 to be a valid callback, cannot access protected method PayPal\Core\PayPalHttpConnection::parseResponseHeaders()

After digging around I realized this is because I am using in the exception handler $exception->getMessage();

To my surprise I found that weird cause that is basic PHP's way on showing the developer the problem that just occurred. Then I thought okie I am just not catching the SDK's correct exception class. I tried all of them and no that isn't the case here. So I found that method inside the PayPalHttpConnection class and made it public.

Ahh finally I can now see what is going on, now that it is public I can see what getMessage() has inside of it. Below is the error I am getting:

Not implemented: CURLSSLOPT_NO_REVOKE (2)

After looking that up, I found that I am not the only person to ever have this problem with VCR and this SDK. So I dug deeper, I noticed the method I turned public specifically states "Parses the response headers for debugging". So I found all the spots this was being called and commented them out. Since it is for debugging which isn't what I need, especially when this goes live.

Just to be clear lines #163 and #195 I commented out. Now with those lines commented out I returned the method back to its original state of protected. Cool I am stilling getting the correct exception error message and not the original error I posted in the beginning of this. After commenting out line #195 I noticed more debug code being called above it, that only is being used in $this->logger->debug(...) i.e. $requestHeaders = curl_getinfo(...) So I commented out those lines #191 - #195.

I reran my test again, now it is green it passed I check and VCR created my fixture files correctly as it should. I look at the responses it received and I got 201 created as one of them so I know the API call actually worked correctly.

P.S. I also set the API Context to mode = live this doesn't resolve the issue since this SDK is forcefully doing debugging in all environments with no way to toggle that off, which doesn't allow me to use a package as helpful as VCR is for writing tests around my code that uses this SDK.

@xiaoleih41
Copy link
Contributor

Thank you for bringing this up. We are looking into it.

@younkim
Copy link

younkim commented Aug 2, 2018

@johnschmuff what did you end up doing to resolve this issue with the SDK?

I believe there are actually two problems with the php-vcr library, not Paypal SDK.

  1. ...cannot access protected method...

VCR library hooks into curl_setopt and tries to invoke parseResponseHeaders (protected) outside of the class it belongs in, causing the error. I think I would prefer for php-vc to employ modifying visibility when it's trying to invoke something it can't. Forking php-vcr and tailoring to your need would probably be your best option.

  1. Not implemented: CURLSSLOPT_NO_REVOKE (2)

This is actually an incorrect error output. First of all, when php-vcr hooks into curl_getinfo for CURLINFO_HEADER_OUT, it doesn't know how to handle it. It needs to be updated to return the header information when this option is used, instead of reaching the exception. Secondly, when it does reach the exception, it tries to look up which method call caused the issue by looking up constants from get_defined_constants. The problem with this is that it aggregates all curl constants into one namespace. Both CURLINFO_HEADER_OUT and CURLSSLOPT_NO_REVOKE both are constants 2, but they are used for different curl functions.

I haven't tested all this, but I think if these are fixed in php-vcr, it should make recording possible. Hope this helps someone. I might be willing to write a fix for these, if anybody needs them. FYI @xiaoleih41

alnorth added a commit to alnorth/php-vcr that referenced this issue Nov 19, 2018
… functions. This should solve an issue encountered when using php-vcr with the PayPal SDK (paypal/PayPal-PHP-SDK#1057).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants