Skip to content

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

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

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

nicolas-grekas
Copy link
Member

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
Copy link
Member Author

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 hc-discovery-error branch January 7, 2020 13:12
@nicolas-grekas nicolas-grekas restored the hc-discovery-error branch January 7, 2020 13:56
@nicolas-grekas nicolas-grekas reopened this Jan 7, 2020
@nicolas-grekas
Copy link
Member Author

Reopening, the message is still better.

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this message is better and more helpful.

@fabpot
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
@nicolas-grekas nicolas-grekas deleted the hc-discovery-error branch January 13, 2020 14:17
This was referenced Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants