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

Dependency nyholm/psr7 missing in 1.4.0 #105

Closed
NickvdMeij opened this issue Mar 7, 2018 · 16 comments
Closed

Dependency nyholm/psr7 missing in 1.4.0 #105

NickvdMeij opened this issue Mar 7, 2018 · 16 comments

Comments

@NickvdMeij
Copy link

Q A
Bug? no
New Feature? no
Version 1.4.0

Actual Behavior

Due to the missing dependency, the sourcecode of nyholm/psr7 is never retrieved from packagist.org, resulting in the following bug:

[2018-03-07 15:33:57] main.ERROR: Source class "\Nyholm\Psr7\Factory\Stream" for "Nyholm\Psr7\Factory\StreamFactory" generation does not exist. [] []

Expected Behavior

No errors...

Steps to Reproduce

  1. git clone
  2. composer install
  3. run it?

Possible Solutions

specify nyholm/psr7 in the require section of composer.json

@Nyholm
Copy link
Member

Nyholm commented Mar 8, 2018

Thank you for this issue.

This library does NOT have a dependency on Nyholm/psr7, guzzlehttp/psr7 or any other client or PSR7 implementation.

The only purpose of this library is to find the classes you have installed. So this library does:
"Is Nyolm/psr7 installed? - If so, use it"
"Is Guzzle PSR7 installed? - If so, use it".

Could you give me some more details. What version of PHP are you running? Is Nyholm/psr7 installed in your application?

@NickvdMeij
Copy link
Author

NickvdMeij commented Mar 8, 2018

@Nyholm No, Nyholm/psr7 is not installed in our application. The problem is for us 3 levels deep in dependencies.

The problems lie in the fact that, in our dependency tree, Nyholm/psr7 is never installed. This results in the following error in Magento2:

[2018-03-07 15:33:57] main.ERROR: Source class "\Nyholm\Psr7\Factory\Stream" for "Nyholm\Psr7\Factory\StreamFactory" generation does not exist. [] []

This error is in that sense 'correct', due to the fact that we don't have these source classes since it's nowhere required in the dependency tree.
However, it is quite strange that this package is trying to use your classes, even though they don't exist. Like mentioned in mailgun/mailgun-php#452 (comment), if I revert php-http/discovery back to version 1.3.0, the problem is solved since your package is not yet included in this version and everything works as expected.

PS: we use PHP 7.0.26 ;-)

@Nyholm
Copy link
Member

Nyholm commented Mar 8, 2018

Can you see where you are trying to use that class? Ie, what line generates the error? The error message is nothing that is printed here.

@NickvdMeij
Copy link
Author

@Nyholm well that's the problem right there, we do not use it anywhere. Furthermore, the only error shown is the one posted above, and the only place where it could have been thrown is in an underlying package (and my assumption is php-http/discovery, since its the only package using your code).

@Nyholm
Copy link
Member

Nyholm commented Mar 8, 2018

Furthermore, the only error shown is the one posted above,

Yes. But who is printing the error message or throwing an exception

and my assumption is php-http/discovery, since its the only package using your code

It is only used if it is installed. That is why I cant figure out your errors with the details you describe.

well that's the problem right there, we do not use it anywhere.

I understand that you do not use it in production. But try research this problem locally.

@dbu
Copy link
Contributor

dbu commented Mar 8, 2018

auto discovery does class_exists checks to see what implementation is available. could it be that your autoloader logs these errors? class_exists on non-existing classes is something that can happen and should not be an error.

@NickvdMeij
Copy link
Author

Took me a while, but here is the trace:

1 0.0012 410344 {main}( ) .../index.php:0
2 0.5278 13885744 Magento\Framework\App\Bootstrap->run( ) .../index.php:40
3 0.5318 13937664 Magento\Framework\App\Http->launch( ) .../Bootstrap.php:258
4 0.8095 25306408 Magento\Framework\App\FrontController\Interceptor->dispatch( ) .../Http.php:135
5 0.8162 25318160 Magento\Framework\App\FrontController\Interceptor->___callPlugins( ) .../Interceptor.php:26
6 0.8470 26202768 Magento\Framework\Module\Plugin\DbStatusValidator->aroundDispatch( ) .../Interceptor.php:142
7 0.8472 26202752 Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception{closure}( ) .../DbStatusValidator.php:69
8 0.8472 26203128 Magento\Framework\Interception\Chain\Chain->invokeNext( ) .../Interceptor.php:138
9 0.8473 26204112 Magento\Framework\App\FrontController\Interceptor->___callParent( ) .../Chain.php:70
10 0.8473 26204112 Magento\Framework\App\FrontController\Interceptor->dispatch( ) .../Interceptor.php:74
11 1.5913 36184320 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->dispatch( ) .../FrontController.php:55
12 1.5978 36274432 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->___callPlugins( ) .../Interceptor.php:26
13 1.8571 39995416 Magento\Backend\App\Action\Plugin\MassactionKey->aroundDispatch( ) .../Interceptor.php:142
14 1.8571 39995416 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->Magento\Framework\Interception{closure}( ) .../MassactionKey.php:33
15 1.8571 39995792 Magento\Framework\Interception\Chain\Chain->invokeNext( ) .../Interceptor.php:138
16 1.8692 40247008 Magento\Backend\App\Action\Plugin\Authentication->aroundDispatch( ) .../Chain.php:67
17 1.8898 40419576 Magento\Framework\Interception\Chain\Chain->Magento\Framework\Interception\Chain{closure}( ) .../Authentication.php:143
18 1.8898 40419952 Magento\Framework\Interception\Chain\Chain->invokeNext( ) .../Chain.php:63
19 1.8898 40419992 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->___callParent( ) .../Chain.php:70
20 1.8898 40419992 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->dispatch( ) .../Interceptor.php:74
21 1.9104 41331904 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->dispatch( ) .../AbstractAction.php:226
22 1.9354 41525888 Magento\Sales\Controller\Adminhtml\Order\Email\Interceptor->execute( ) .../Action.php:102
23 2.4657 46701376 Magento\Sales\Model\Service\OrderService\Interceptor->notify( ) .../Email.php:27
24 2.4680 46703856 Magento\Sales\Model\OrderNotifier->notify( ) .../OrderService.php:140
25 2.4680 46703856 Fooman\EmailAttachments\Model\Email\Sender\OrderSender->send( ) .../AbstractNotifier.php:59
26 2.5936 54148424 Fooman\EmailAttachments\Model\Email\Sender\OrderSender->send( ) .../OrderSender.php:56
27 2.5936 54148504 Fooman\EmailAttachments\Model\Email\Sender\OrderSender->checkAndSend( ) .../OrderSender.php:102
28 2.8620 57155872 Fooman\EmailAttachments\Model\SenderBuilder->send( ) .../Sender.php:77
29 2.8620 57155872 Fooman\EmailAttachments\Model\SenderBuilder->send( ) .../SenderBuilder.php:56
30 7.1825 64710352 Bogardo\Mailgun\Mail\Transport\Interceptor->sendMessage( ) .../SenderBuilder.php:67
31 7.1885 64726104 Bogardo\Mailgun\Mail\Transport\Interceptor->___callPlugins( ) .../Interceptor.php:26
32 7.1910 64732808 Magento\Email\Model\Plugin\TransportInterfacePlugin->aroundSendMessage( ) .../Interceptor.php:142
33 7.1910 64732808 Bogardo\Mailgun\Mail\Transport\Interceptor->Magento\Framework\Interception{closure}( ) .../TransportInterfacePlugin.php:54
34 7.1910 64732864 Magento\Framework\Interception\Chain\Chain->invokeNext( ) .../Interceptor.php:138
35 7.1910 64732904 Bogardo\Mailgun\Mail\Transport\Interceptor->___callParent( ) .../Chain.php:70
36 7.1910 64732904 Bogardo\Mailgun\Mail\Transport\Interceptor->sendMessage( ) .../Interceptor.php:74
37 7.2593 65405584 Mailgun\Mailgun->sendMessage( ) .../Transport.php:65
38 7.2593 65405640 Mailgun\Mailgun->post( ) .../Mailgun.php:135
39 7.2593 65405640 Mailgun\Connection\RestClient->post( ) .../Mailgun.php:204
40 7.2594 65408760 Mailgun\Connection\RestClient->send( ) .../RestClient.php:179
41 7.2633 65435688 Http\Message\MultipartStream\MultipartStreamBuilder->__construct( ) .../RestClient.php:96
42 7.2693 65459280 Http\Discovery\StreamFactoryDiscovery::find( ) .../MultipartStreamBuilder.php:43
43 7.2693 65459280 Http\Discovery\ClassDiscovery::findOneByType( ) .../StreamFactoryDiscovery.php:25
44 7.2853 65562272 Http\Discovery\ClassDiscovery::evaluateCondition( ) .../ClassDiscovery.php:63
45 7.2882 65562648 Http\Discovery\ClassDiscovery::evaluateCondition( ) .../ClassDiscovery.php:174
46 7.2882 65562648 class_exists ( ) .../ClassDiscovery.php:164
47 7.2882 65562712 spl_autoload_call ( ) .../ClassDiscovery.php:164
48 7.2883 65562856 Magento\Framework\Code\Generator\Autoloader->load( ) .../ClassDiscovery.php:164
49 7.2883 65562856 Magento\Framework\Code\Generator->generateClass( ) .../Autoloader.php:35
50 7.3155 65920760 Magento\Framework\Code\Generator->tryToLoadSourceClass( ) .../Generator.php:112
51 7.3158 66017728 __construct ( ) .../Generator.php:181

@Nyholm
Copy link
Member

Nyholm commented Mar 8, 2018

Looking at row 45 and forward. It seams like @dbu is correct.

We run class_exsits() here. Then the Magento autoloader tries to load and generate(?) this class. And this is where you fail.
I suggest opening an issue on https://github.com/magento/magento2/issues with this stack trace.

@NickvdMeij
Copy link
Author

@Nyholm & @dbu see magento/magento2#14074
Apparently, Magento thinks this is normal behavior cause you apparently can't have 3rd party classes with the name Factory in it. Due to the ignorance of the Magento developers, the issue there has been closed and marked as a non-issue, so I'd suggest to close this issue since this is appearently not getting fixed (LMAO...).

Thanks for looking anyways and I wish you guys the best of luck in the future ;-)

@Nyholm
Copy link
Member

Nyholm commented Mar 13, 2018

I just cloned magento2 and ran:

composer req php-http/discovery guzzlehttp/psr7  php-http/message

Then I added the PHP file:

<?php

include_once __DIR__.'/vendor/autoload.php';

echo "Starting\n\n";
$messageFactory = \Http\Discovery\MessageFactoryDiscovery::find();
if (null !== $messageFactory) {
    echo "Class exists!";
} else {
    echo "Fail";
}

Running that PHP file will output "Class exists!".

I forgot to register Magento's autoloaders

@Nyholm
Copy link
Member

Nyholm commented Mar 13, 2018

Im closing this since an issue has been opened on Magento's repo. I also created a PR: magento/magento2#14085

@orlangur
Copy link

This was a really strange attempt to circumvent a bug affecting end user in Magento :)

Of course there is no need to add optional dependency as mandatory and there is nothing to fix in Magento core. From the stack trace the problem comes from Bogardo_Mailgun module (or maybe Fooman_EmailAttachments? cannot say exactly without looking into the code) and it's the right place for the bugfix.

@Nyholm
Copy link
Member

Nyholm commented Mar 13, 2018

It is clear to me that you do not understand the purpose of php-http/discovery. Have a look in the documentation and you will see why requiring nyholm/psr7 does not make any sense.

@orlangur
Copy link

It is clear to me that you do not understand the purpose of php-http/discovery

Lolwhat? :) How is that different from

Of course there is no need to add optional dependency as mandatory

? Of course I DO understand when particular library needs to be specified in composer.json.

@NickvdMeij
Copy link
Author

@orlangur "? Of course I DO understand when particular library needs to be specified in composer.json."

Well clearly you dont, since nyholm/psr7 may or may not be declared, it doesn't matter. It CHECKS if the class exists, and by doing this (using official PHP methods), it breaks Magento in the process because of the bad implementation of MAGENTO'S Autoloader

@dbu
Copy link
Contributor

dbu commented Mar 14, 2018

lets not shout at each other - the analysis is done and @Nyholm provided a pull request to magento proving the bug is in the magento autoloader. there is nothing wrong with the discovery, it uses PHP as intended when it does a class_exists call with a possibly not existing class.

@php-http php-http locked as too heated and limited conversation to collaborators Mar 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants