Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP7.4 & guzzle #289

Closed
janvernieuwe opened this issue Dec 2, 2019 · 24 comments · Fixed by #293
Closed

PHP7.4 & guzzle #289

janvernieuwe opened this issue Dec 2, 2019 · 24 comments · Fixed by #293
Labels
Bug Something isn't working

Comments

@janvernieuwe
Copy link
Contributor

janvernieuwe commented Dec 2, 2019

Since upgrading to php7.4 im getting

ParseError : syntax error, unexpected end of file, expecting :: (T_PAAMAYIM_NEKUDOTAYIM)
 vendor/guzzlehttp/guzzle/src/Handler/CurlMultiHandler.php:197

I have noticed this on multiple projects. I'll try to look into this later today, but as of now it's not really usable on php 7.4.

I only get this error when VCR is turned on.

@JapSeyz
Copy link

JapSeyz commented Dec 10, 2019

Did you dig further into this?

@janvernieuwe
Copy link
Contributor Author

I tried, but there is some really weird issue going on. It almost seems that files are not completely loaded after x lines. It's very strange. If I find something, I'll make it known here.

@volkyeth
Copy link

Maybe it's a regression on this php 7.4 bug: https://bugs.php.net/bug.php?id=78406

Tried finding the exact spot where it occurs with xdebug and it seems to happen when the file is included.
The CodeTransform works as expected, but somehow it seems that part of the stream is ignored, probably because the code transform increases the file size and the old file size is considered.

Looks like It's being reproduced on travis on the 7.4 test suite on #290:
https://travis-ci.org/php-vcr/php-vcr/jobs/619582578?utm_medium=notification&utm_source=github_status

Just by having this on the bootstrap it breaks the test suite:

\VCR\VCR::turnOn();
\VCR\VCR::turnOff();

When commenting it the tests run, breaking in other spots, and reporting several errors of this kind when composer tries to autoload some classes:

ParseError: syntax error, unexpected end of file in php-vcr/src/VCR/Util/HttpClient.php:35
Stack trace:

#0 php-vcr/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile(...

By commenting $this->enableLibraryHooks(); on \VCR\Videorecorder::turnOn() I get no more syntax errors. So it's most definitely a problem on the stream processing used by those hooks

It's either a bug on core or some undocumented change on streams that broke the stream processing.
The only related change I could find was: https://www.php.net/manual/en/migration74.incompatible.php#migration74.incompatible.core.stream-wrappers
But that doesn't seem to explain what we see going on here.

@morozov
Copy link
Contributor

morozov commented Dec 14, 2019

Bug #78406 is closed and according to https://3v4l.org/jEMEE is no longer reproducible although the underlying logic and the symptoms look very similar.

Could anyone who is better familiar with the VCR internals rework the code snippet mentioned above to reproduce the issue in isolation from the VCR itself? Then it could be reported and fixed upstream.

@janvernieuwe
Copy link
Contributor Author

janvernieuwe commented Dec 30, 2019

I have a feeling it is related to this issue goaop/framework#426
(which links to the bug already mentioned)
Unfortunately no fix found yet.

@morozov
Copy link
Contributor

morozov commented Dec 30, 2019

Unfortunately no fix found yet.

Could you please try reproducing the bug in isolation from the rest of the VCR code?

@janvernieuwe
Copy link
Contributor Author

janvernieuwe commented Dec 31, 2019

I tried to remove as much as possible.
https://github.com/janvernieuwe/php-vcr-debug

I'm now looking at the patchwork library (https://github.com/antecedent/patchwork), where the streamprocessor of vcr seems to be based off, to see if I can find a solution there.

@morozov
Copy link
Contributor

morozov commented Dec 31, 2019

@janvernieuwe this is good progress! At bugs.php.net, they accept a code sample that reproduces the problem. The same could be evaluated at 3v4l.org. Could you put all the code in one file and potentially remove some more irrelevant pieces?

Once it's reproducible in a single PHP script, we can file a bug.

@janvernieuwe
Copy link
Contributor Author

I'll take a look on monday, but it happens on include, so the file needs to to be separate in the testing.

@janvernieuwe
Copy link
Contributor Author

Gets even weirder now. When I add the classes into the file itself instead of including them, the error doesn't seem to be occurring.

I left the run.php in place and added a run2.php where the classes are included.
Weirdness.

@morozov
Copy link
Contributor

morozov commented Jan 6, 2020

I've filed the bug and referenced the repository above: https://bugs.php.net/bug.php?id=79072.

@janvernieuwe
Copy link
Contributor Author

Nice, seems like an easy enough fix!

@carusogabriel
Copy link
Contributor

@janvernieuwe @morozov I'll take a look into this during this week, I want first ensure that #292 and #256 don't get conflict with it :)

@carusogabriel carusogabriel added the Bug Something isn't working label Jan 7, 2020
@morozov
Copy link
Contributor

morozov commented Jan 7, 2020

Nice, seems like an easy enough fix!

Please see #293.

@morozov
Copy link
Contributor

morozov commented Jan 7, 2020

I'll take a look into this during this week, I want first ensure that #292 and #256 don't get conflict with it :)

Thanks, @carusogabriel. Since #292 is about bumping the minimal required PHP version, I'd like to make sure that the fix for this issue is released before the version is bumped so that PHPBrew can use it before it bumps its own PHP requirement.

@danielbecker
Copy link

I'd like to bump the issue as we're having trouble with it, as well.

@bomas13
Copy link

bomas13 commented Jan 30, 2020

Unfortunately for me this fixes the initial error like syntax error, unexpected end of file... but the test seems to get into an endless loop and runs for 60min without a timeout. Does anyone have a similar behaviour or better yet a solution for it?

PHP 7.4.1 VCR1.4 with fix from morozov

@carusogabriel
Copy link
Contributor

I'm on vacation until February 17th. Once I'm back, I'll take a look at this 👍

@bomas13
Copy link

bomas13 commented Jan 30, 2020

Ah ok. Have a nice holiday!

Just for your information: After updating PHP to 7.4.2. I'm getting the following error (incl. fix from morozov):

ErrorException : Trying to access array offset on value of type null
 /app/vendor/php-vcr/php-vcr/src/VCR/Storage/Yaml.php:61
 /app/vendor/php-vcr/php-vcr/src/VCR/Storage/Yaml.php:124
 /app/vendor/php-vcr/php-vcr/src/VCR/Cassette.php:71
 /app/vendor/php-vcr/php-vcr/src/VCR/Videorecorder.php:223
 /app/vendor/php-vcr/php-vcr/src/VCR/Videorecorder.php:291
 /app/vendor/php-vcr/php-vcr/src/VCR/LibraryHooks/CurlHook.php:204
 /app/vendor/php-vcr/php-vcr/src/VCR/LibraryHooks/CurlHook.php:154
 /app/vendor/guzzlehttp/guzzle/src/Handler/CurlHandler.php:40
 /app/vendor/guzzlehttp/guzzle/src/Handler/Proxy.php:28
 /app/vendor/guzzlehttp/guzzle/src/Handler/Proxy.php:51
 /app/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php:37
 /app/vendor/guzzlehttp/guzzle/src/Middleware.php:29
 /app/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php:70
 /app/vendor/guzzlehttp/guzzle/src/Middleware.php:59
 /app/vendor/guzzlehttp/guzzle/src/HandlerStack.php:71
 /app/vendor/guzzlehttp/guzzle/src/Client.php:361
 /app/vendor/guzzlehttp/guzzle/src/Client.php:163
 /app/vendor/guzzlehttp/guzzle/src/Client.php:183
 /app/vendor/guzzlehttp/guzzle/src/Client.php:96
 /app/tests/Feature/VcrTest.php:20

Without the fix and 7.4.2 it behaves like in the first comment

@bomas13
Copy link

bomas13 commented Jan 31, 2020

We could find a solution for the exception.
The problem with my endless loop was not vcr related.

@bomas13
Copy link

bomas13 commented Feb 17, 2020

after I switched from my vanilla curl usage to guzzle I run into issues that seem to related to the steam_wrapper issues again. My tests are all getting green, but after PHPUnit prints out the test summary, it won't quit the application successfully. If I run a couple of tests that do not use VCR this behavior doesn't come up. (See the Screenshot. The first test lines are aborted after several minutes of doing nothing [See the ^C]. The second line runs as expected and gets finished by itself correctly).

It's a little bit frustrating, but changing all tests from VCR to Mockery will take too much time as well.

Please please please get VCR working again. Thank you.

Screenshot

@danielbecker
Copy link

Hey guys, any update here? We could really need a working version for php 7.4. I'm happy to have a look myself if you could point me at least into the right direction?

@morozov
Copy link
Contributor

morozov commented Feb 20, 2020

@danielbecker please see #293 (comment).

@danielbecker
Copy link

Thanks for the response @morozov. As @bomas13 mentioned, phpunit doesn't terminate. Even with the patch applied.

roback added a commit to twingly/twingly-search-api-php that referenced this issue Sep 17, 2021
Guzzle 7.2.0 added support for PHP 8: https://github.com/guzzle/guzzle/blob/7008573787b430c1c1f650e3722d9bba59967628/CHANGELOG.md#720---2020-10-10

Ran into the following php-vcr related bug when updating guzzle,
so I had to update php-vcr too: php-vcr/php-vcr#289

PHP-VCR added the fix for the above issue in 1.4.5, but I chose the
latest minor version which is 1.5 (1.5.2 at time of writing).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants