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

[HttpClient] fix exception in case of PSR17 discovery failure #35245

Merged
merged 1 commit into from Jan 7, 2020

Conversation

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 7, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

On symfony.com, we need to use HTTPlug for 3rd party libs. When nyholm/psr7 is not installed, we currently see an exception saying No HTTPlug clients found. from HttpClientDiscovery.

This fixes the message by correctly suggesting nyholm/psr7 instead, since there is an HTTPlug client: HttplugClient from our HttpClient component.

It's quite unfortunate that guzzle/psr7 provides no PSR17 factory yet, because that would have solved some part of this deps mess. /cc @Nyholm @sagikazarmark FYI
Note that https://packagist.org/providers/psr/http-factory-implementation lists guzzle/psr7 but this is a wrong solution: no tagged release of it is PSR17-compatible, which means installing it doesn't solve the issue.

@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Jan 7, 2020

Hum, actually the message says No PSR-17 response factory found. Install a package from this list: https://packagist.org/providers/psr/http-factory-implementation.
The one about No HTTPlug clients found. is from another dep that calls Http\Discovery\HttpClientDiscovery::find().

This is a mess, sorry :)

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:hc-discovery-error branch Jan 7, 2020
@nicolas-grekas nicolas-grekas restored the nicolas-grekas:hc-discovery-error branch Jan 7, 2020
@nicolas-grekas nicolas-grekas reopened this Jan 7, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member Author

nicolas-grekas commented Jan 7, 2020

Reopening, the message is still better.

@Nyholm
Nyholm approved these changes Jan 7, 2020
Copy link
Member

Nyholm left a comment

I agree that this message is better and more helpful.

@fabpot
fabpot approved these changes Jan 7, 2020
@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Jan 7, 2020

Thank you @nicolas-grekas.

fabpot added a commit that referenced this pull request Jan 7, 2020
…ure (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] fix exception in case of PSR17 discovery failure

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

On symfony.com, we need to use HTTPlug for 3rd party libs. When `nyholm/psr7` is not installed, we currently see an exception saying `No HTTPlug clients found.` from `HttpClientDiscovery`.

This fixes the message by correctly suggesting `nyholm/psr7` instead, since there *is* an HTTPlug client: `HttplugClient` from our HttpClient component.

It's quite unfortunate that `guzzle/psr7` provides no PSR17 factory yet, because that would have solved some part of this deps mess. /cc @Nyholm @sagikazarmark FYI
Note that https://packagist.org/providers/psr/http-factory-implementation lists `guzzle/psr7` but this is a wrong solution: no tagged release of it is PSR17-compatible, which means installing it doesn't solve the issue.

Commits
-------

96e70a4 [HttpClient] fix exception in case of PSR17 discovery failure
@fabpot fabpot merged commit 96e70a4 into symfony:4.4 Jan 7, 2020
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details
@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:hc-discovery-error branch Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.