amphp / http-client

An advanced async HTTP client library for PHP, enabling efficient, non-blocking, and concurrent requests and responses.
https://amphp.org/http-client
MIT License
704 stars 66 forks source link

Incompatible with CookieInterceptor since 4.2.1 #281

Closed Bilge closed 3 years ago

Bilge commented 4 years ago

A change introduced between releases 4.2.0 and 4.2.1 breaks the CookieInterceptor. May also affect other interceptors.

The specific usage scenario is: a PHPUnit test makes four HTTP calls in serial, one in each test (of the same test class). Then it makes an additional two HTTP calls in two different test case classes (one in each). Either one or both of these will typically fail (very rarely they might both pass, providing a false positive). The first four tests (in the same class) share the same connection pool and always pass. The next two tests create a new pool each, but share the same cookies, and often fail as described.

kelunik commented 4 years ago

Please share a simple script to reproduce the behavior you're seeing.

Bilge commented 4 years ago

Will be very tricky considering I cannot reproduce locally, only on Travis.

Bilge commented 4 years ago

For what it's worth, I validated my suspicion that disabling pooling avoids the problem. Although I cannot provide a contrived test case at this point, this confirms to me that there is an invalid interaction between pooling and interceptors since 4.2.1 running up to the latest (this was tested on 4.5.2, passing without pooling).

kelunik commented 3 years ago

@Bilge Do you use --static-backup in PHPUnit?

Bilge commented 3 years ago

No

kelunik commented 3 years ago

How do you ensure the cookies are shared correctly? Does one test create them and the other use them?

Bilge commented 3 years ago

Test base class statically initializes them.

kelunik commented 3 years ago

The issue is likely in your code somewhere, e.g. cloning the CookieJar could be problematic.

We can reopen the issue if you provide a simple reproduction case.