Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

guzzlehttp/psr7 1.x broken in Slim 4.9.0 #3121

Closed
sapphirecat opened this issue Oct 15, 2021 · 7 comments
Closed

guzzlehttp/psr7 1.x broken in Slim 4.9.0 #3121

sapphirecat opened this issue Oct 15, 2021 · 7 comments
Labels

Comments

@sapphirecat
Copy link

sapphirecat commented Oct 15, 2021

I did a composer update and my app broke, due to #3106.

If this is an intentional side-effect, then the composer.json should probably indicate a conflict for guzzlehttp/psr7 <2.0. [edit: actually... maybe not. That would preclude anyone from providing their own factory, which might be a better solution for my situation.]

If unintended, then I believe there should be another factory class, to retain support for http-interop/http-factory-guzzle. That's the package I have been using to plug everything together with aws/aws-sdk-php (currently locked at 3.198.3 FWIW) which requires guzzlehttp/psr7: ^1.7.

I have updated my composer.json to require slim/slim: ^4.0, <4.9.0 to work around the issue on my end.

@l0gicgate
Copy link
Member

l0gicgate commented Oct 16, 2021

I suspect it would be difficult to support both since they share the same namespace. We'd have to break more things to make this happen. I'm open to getting a solution if there is one that doesn't break more things.

@sapphirecat
Copy link
Author

In order to accept new Slim versions and bug fixes, I reverted my composer.json, and plugged in my own class:

<?php
class GuzzlePsr7v1Factory extends \Slim\Factory\Psr17\GuzzlePsr17Factory
{
	protected static $responseFactoryClass = 'Http\Factory\Guzzle\ResponseFactory';
	protected static $streamFactoryClass = 'Http\Factory\Guzzle\StreamFactory';
}

As you can see, this simply restores the http-interop class names as used in 4.8.1's GuzzlePsr17Factory.

This is used during the main app building process (which, for historical reasons, uses php-di, but I have also tested with the AppBuilder):

<?php
use Slim\Factory\Psr17\Psr17FactoryProvider;

// ...
Psr17FactoryProvider::addFactory(GuzzlePsr7v1Factory::class);

// Create the app
$container = ContainerBuilder::build($configuration);
$app = $container->get(App::class);

// ...

To fix it, it seems like this class could be implemented within Slim under a different name from GuzzlePsr17Factory (GuzzleLegacyPsr17Factory?), then be added to the list of default factories.

Otherwise, docs should probably be clear that guzzlehttp/psr7 version 2.x is supported. It can be confusing to see "pick one of these things, we support guzzlehttp/psr7", when I clearly have the package installed (but the wrong version.) In fact, as I write, the installation docs and error message link give instructions for installing http-interop/http-factory-guzzle to use Guzzle, although this no longer necessarily works.

@t0mmy742
Copy link
Contributor

t0mmy742 commented Oct 17, 2021

We do not have any conflict with guzzlehttp/psr7 <2.0. We just do not support http-interop/http-factory-guzzle anymore.
guzzlehttp/psr7 ^2.0 is supported version since #3099 (it does not means guzzlehttp/psr7 <2.0 CAN NOT work, it means guzzlehttp/psr7 ^2.0 MUST work)
http-interop/http-factory-guzzle is officially unsupported since #3106 since its author deprecated it.

Otherwise, docs should probably be clear that guzzlehttp/psr7 version 2.x is supported. It can be confusing to see "pick one of these things, we support guzzlehttp/psr7", when I clearly have the package installed (but the wrong version.) In fact, as I write, the installation docs and error message link give instructions for installing http-interop/http-factory-guzzle to use Guzzle, although this no longer necessarily works.

I can admit we could specify which versions are tested and officially supported, based on require-dev from composer.json file (for example guzzlehttp/psr7 ^2.0).
Documentation website is not really updated after each change, it could be error prone.
We already removed mentions to http-interop/http-factory-guzzle on README file.

About your request, I think Slim framework should not support legacy versions of PSR-7 and PSR-17 providers, but at least always support last major version. Other versions (as it is already the case for other PSR-7 providers) can still be added manually.

@l0gicgate
Copy link
Member

I think the best solution here is to clarify the docs and also document the workaround that OP is using for legacy support.

I agree with @t0mmy742 that we shouldn’t support deprecated packages as it becomes a nightmare to maintain over time.

@sapphirecat
Copy link
Author

Upon further reflection, I have decided to publish the workaround as a reusable library, and have released it as sapphirecat/slim4-http-interop-adapter.

AIUI, all that's left here for the Slim project would be documentation updates.

@l0gicgate
Copy link
Member

@sapphirecat we can include your package in the docs if you want to do the update!

sapphirecat added a commit to sapphirecat/Slim-Website that referenced this issue Oct 18, 2021
Removes the Guzzle HTTP Factory from the base requirements, as it is no
longer needed with the latest Slim version.

Adds instructions for a new package that supports Guzzle PSR-7 version 1
on Slim 4.9.

See slimphp/Slim#3121
sapphirecat added a commit to sapphirecat/Slim-Website that referenced this issue Oct 20, 2021
Removes the Guzzle HTTP Factory from the base requirements, as it is no
longer needed with the latest Slim version.

Adds instructions for a new package that supports Guzzle PSR-7 version 1
on Slim 4.9.

See slimphp/Slim#3121
@l0gicgate
Copy link
Member

Closing this as resolved

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants