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
Undocumented error triggered when using Discovery with Symfony when php-http/httplug not installed #201
Comments
we trigger_error an E_USER_WARNING, not a E_USER_ERROR. the intention was not that consumers would need to handle an exception. but i see this is a problem if you convert warnings to errors, e.g. during ci runs. from how i understand #170, the aim was to not consider the symfony client if httplug is not available, as it won't be possible to instantiate it. we do the warning to help people who are not aware that they need to httplug to make the symfony client viable as psr 18 client. would reducing the warning to a notice be better? i guess an even better solution would be to somehow just collect the fact that symfony would be present but is not viable, but only tell that in the regular NotFoundException in case we find no viable client at all, and otherwise silently ignore it. it avoids warnings, at the cost of people maybe being confused why the symfony client is not preferred. |
All user warnings/errors are converted to exceptions in PHP 7+ I believe (they get converted to an
Yes; I think this would be better; if this |
i investigated the code a bit and we are missing one check to know if we can load the symfony client. can you check if #202 fixes the issue for you? @Nyholm what is your opinion on the general topic of the E_USER_WARNINGS? i don't see how we can easily change our code to track things. we could have some state based collection of exceptions to only output if no client was found, but that feels not very elegant either. |
I am using
Psr18ClientDiscovery::find()
in a library I'm writing, and I wrap thefind()
call in a try/catch to capture\Http\Discovery\Exception\NotFoundException
. I used the library to do a manual test in a Symfony application, but instead of seeing any exception, I see the followingE_USER_WARNING
is raised:The message itself is fine, but I expect the library to throw a
\Http\Discovery\Exception\NotFoundException
, not trigger an error. I traced the change into the change #170 which catches the\LogicException
thrown in Symfony, but instead of letting the exception bubble up, or wrapping it in a\Http\Discovery\Exception\NotFoundException
, it converts it to a runtime error usingtrigger_error
which was not expected or documented:discovery/src/Psr18ClientDiscovery.php
Lines 15 to 22 in de90ab2
As a workaround, one can catch
\ErrorException
as well as\Http\Discovery\Exception\NotFoundException
(or just catch\Throwable
), but the fact remains that this is confusing and undocumented behaviour.I think rather than converting the
\LogicException
into atrigger_error
, the exception should be caught and converted to outwardly throw aNotFoundException
, which is documented behaviour.The text was updated successfully, but these errors were encountered: