symfony httplug adapter needs php-http httplug to be installed #202
Conversation
dbu
force-pushed
the
fix-symfony-client-strategy
branch
from
dd33789
to
56af60c
Compare
|
i don't have time to do #200 - we should convert from travis-ci to github actions to have the tests actually run in this repository. |
| ['class' => Guzzle7::class, 'condition' => Guzzle7::class], | ||
| ['class' => Guzzle6::class, 'condition' => Guzzle6::class], | ||
| ['class' => Curl::class, 'condition' => Curl::class], | ||
| ['class' => React::class, 'condition' => React::class], | ||
| ], | ||
| HttpClient::class => [ | ||
| ['class' => SymfonyHttplug::class, 'condition' => [SymfonyHttplug::class, RequestFactory::class, [self::class, 'isPsr17FactoryInstalled']]], | ||
| ['class' => SymfonyHttplug::class, 'condition' => [SymfonyHttplug::class, HttpClient::class, RequestFactory::class, [self::class, 'isPsr17FactoryInstalled']]], |
There was a problem hiding this comment.
Unfortunately this does not seem to change anything. Symfony throws a \LogicException outside of the class structure, so as soon as the file is loaded, and php-http/httplug is missing, the \LogicException is thrown.
Then in \Http\Discovery\Strategy\CommonClassesStrategy::getPsr18Candidates, that \LogicException is caught, and converted to a trigger_error:
discovery/src/Strategy/CommonClassesStrategy.php
Lines 139 to 145 in 98ead1f
In trying to debug this, the only ways I found this was working was to turn that try/catch into silence (not ideal...):
try {
if (is_subclass_of($c['class'], Psr18Client::class)) {
$candidates[] = $c;
}
} catch (\Throwable $e) {
//trigger_error(sprintf('Got exception "%s (%s)" while checking if a PSR-18 Client is available', get_class($e), $e->getMessage()), E_USER_WARNING);
}but ALSO you would need to add HttpClient::class as a condition to symfonyPsr18Instantiate under the Psr18Client::class definitions:
Psr18Client::class => [
[
'class' => [self::class, 'symfonyPsr18Instantiate'],
'condition' => [HttpClient::class, SymfonyPsr18::class, Psr17RequestFactory::class],
],Silencing the exception feels very wrong, but with a clear comment explaining why, it might be OK, as it doesn't look like there is a logger around to log a warning to.
There was a problem hiding this comment.
right the problem occurs before conditions are even checked. i realized that we need to use our safe class exists here as well. changed to use that and in my small reproducer that makes the trigger_error go away.
dbu
force-pushed
the
fix-symfony-client-strategy
branch
3 times, most recently
from
b373504
to
9ff61d3
Compare
|
@asgrim old issue... i re-read the thing and updated the PR to something that should actually solve the problem you encountered and is cleaner |
|
Thank you @dbu ! |
e.g. symfony httplug adapter triggers a warning if the php-http/httplug is not available https://github.com/symfony/http-client/blob/290eb481973b4984eb59377585f6afbc65a0f645/HttplugClient.php#L44
dbu
force-pushed
the
fix-symfony-client-strategy
branch
from
9ff61d3
to
94ddf9e
Compare
if we don't check for the httplug client interface, we trigger a warning:
https://github.com/symfony/http-client/blob/290eb481973b4984eb59377585f6afbc65a0f645/HttplugClient.php#L44
What's in this PR?
avoid having symfony trigger a warning when symfony httplug client is not usable due to missing dependencies.