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.

Sign up for GitHub

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

Jump to bottom

FPC: Upsells & Related product list bug - getIdentities gets called while the block doesn't render and then crashes #5897

Closed
hostep opened this issue Jul 29, 2016 · 5 comments
Closed

FPC: Upsells & Related product list bug - getIdentities gets called while the block doesn't render and then crashes #5897

hostep opened this issue Jul 29, 2016 · 5 comments
Assignees
@kassner
Labels
bug report Component: Cms Component: Framework/Cache

Comments

@hostep
Copy link
Contributor

hostep commented Jul 29, 2016 •
edited
Loading

Preconditions

  1. Magento CE 2.1.0
  2. PHP 7.0.9
  3. MariaDB 10.0.25
  4. A lot of time to setup everything properly ;)

Steps to reproduce

  1. Install Magento CE 2.1.0 using composer

  2. Go to the backend

  3. Create 2 simple products: 'Product 1' & 'Product 2', set a name, a price, make sure they have a possitive qty and are in stock and make sure you can see them on the frontend

  4. Create 2 configurable products: 'Product 3' & 'Product 4', set a name, a price, add some configurations (with the color attribute for example)

  5. Add 'Product 1' & 'Product 2' as upsells to the products 'Product 3' & 'Product 4'

  6. Change the 'Stock Status' of 'Product 3' to 'Out of Stock'

  7. Run bin/magento indexer:reindex

  8. Run bin/magento cache:flush

  9. Verify you can see 'Product 3' (http://example.com/product-3.html) and 'Product 4' (http://example.com/product-4.html) with their upsells

  10. Create a custom frontend theme which inherits from 'Magento/blank': 'My/Theme'

  11. Create a file: app/design/frontend/My/Theme/Magento_Catalog/layout/catalog_product_view.xml with this contents:

    <?xml version="1.0"?>
<page layout="1column" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
   <body>
       <move element="product.info.upsell" destination="product.info.form.options" before="-" />
   </body>
</page>

  12. Go in the backend to Content => Design => Configuration

  13. Change the Global theme to 'My Theme'

  14. Run bin/magento cache:flush

  15. Refresh the 'Product 3' page (the page crashes)

  16. Run bin/magento cache:disable full_page

  17. Refresh the 'Product 3' page (it works)

  18. Run bin/magento cache:enable full_page

  19. Refresh the 'Product 3' page (it crashes again)

  20. Go to 'Product 4' page (this one does work all the time)

Expected result

The page shouldn't crash with the full page cache enabled

Actual result

In step 15 and 19 we get this error:

Call to a member function getItems() on null in vendor/magento/module-catalog/Block/Product/ProductList/Upsell.php on line 146

Call Stack
#   Time    Memory      Function                                                                                        Location
1   0.0003  372832      {main}( )                                                                                       .../index.php:0
2   0.1488  12366368    Magento\Framework\App\Bootstrap->run( )                                                         .../index.php:37
3   0.1496  12405744    Magento\Framework\App\Http->launch( )                                                           .../Bootstrap.php:258
4   1.0836  55857816    Magento\Framework\View\Result\Page\Interceptor->renderResult( )                                 .../Http.php:139
5   1.0836  55858192    Magento\Framework\View\Result\Page\Interceptor->___callPlugins( )                               .../Interceptor.php:130
6   1.0838  55861536    Magento\PageCache\Model\Controller\Result\BuiltinPlugin->aroundRenderResult( )                  .../Interceptor.php:142
7   1.0838  55861536    Magento\Framework\View\Result\Page\Interceptor->Magento\Framework\Interception\{closure}( )     .../BuiltinPlugin.php:67
8   1.0838  55861912    Magento\Framework\Interception\Chain\Chain->invokeNext( )                                       .../Interceptor.php:138
9   1.0840  55865304    Magento\PageCache\Model\Controller\Result\VarnishPlugin->aroundRenderResult( )                  .../Chain.php:67
10  1.0840  55865304    Magento\Framework\Interception\Chain\Chain->Magento\Framework\Interception\Chain\{closure}( )   .../VarnishPlugin.php:74
11  1.0840  55865680    Magento\Framework\Interception\Chain\Chain->invokeNext( )                                       .../Chain.php:63
12  1.0840  55865720    Magento\Framework\View\Result\Page\Interceptor->___callParent( )                                .../Chain.php:70
13  1.0840  55865720    Magento\Framework\View\Result\Layout->renderResult( )                                           .../Interceptor.php:74
14  1.0840  55865720    Magento\Framework\View\Result\Page->render( )                                                   .../Layout.php:164
15  1.1605  56954088    Magento\Framework\View\Layout\Interceptor->getOutput( )                                         .../Page.php:243
16  1.1605  56954144    Magento\Framework\View\Layout\Interceptor->___callPlugins( )                                    .../Interceptor.php:494
17  1.5502  61762680    Magento\PageCache\Model\Layout\LayoutPlugin->afterGetOutput( )                                  .../Interceptor.php:152
18  1.5972  62349280    Magento\Catalog\Block\Product\ProductList\Upsell\Interceptor->getIdentities( )                  .../LayoutPlugin.php:71
19  1.5972  62349280    Magento\Catalog\Block\Product\ProductList\Upsell->getIdentities( )                              .../Interceptor.php:141
20  1.5972  62349336    Magento\Catalog\Block\Product\ProductList\Upsell\Interceptor->getItems( )                       .../Upsell.php:238
21  1.5972  62349336    Magento\Catalog\Block\Product\ProductList\Upsell->getItems( )                                   .../Interceptor.php:37

Discussion

Although my use case is not very clever (because the related products don't show up with the full_page cache disabled on 'Product 3'), I still believe this is an actual bug.

What I think happens (not debugged properly): 'Product 3' is out of stock, so the block product.info.form.options doesn't get rendered because it makes no sense to show the options (right?).
This probably causes the Magento\Catalog\Block\Product\ProductList\Upsell::_beforeToHtml to not being called, while the Magento\Catalog\Block\Product\ProductList\Upsell::getIdentities does get called, and that method assumes the _beforeToHtml method gets called first

The same thing happens with Related products btw, so not only Upsells.

So no idea what the actual bug is, either:

  • the block product.info.form.options should get rendered
  • the method getIdentities shouldn't get called when a block isn't rendered
  • the method getIdentities should return an empty array when the block isn't rendered
  • something else...

My biggest issue with this is that we have a different behavior between fpc enabled or not, the result should be the same, and in this case, it isn't.

Thanks!

(thanks to my colleague @koenner01 for helping me debug this thing!)

Related ticket: #3362
@hostep hostep mentioned this issue Jul 29, 2016
Closed
@hostep hostep changed the title Upsells & Related product list bug: getIdentities gets called while the block doesn't render and then crashes FPC: Upsells & Related product list bug - getIdentities gets called while the block doesn't render and then crashes Jul 29, 2016
@hostep hostep mentioned this issue Jan 5, 2017
Closed
@surfer190
Copy link

@hostep Where are we on this issue? What do we need to do for an interim fix?

@choukalos choukalos assigned choukalos and unassigned choukalos Feb 21, 2017
@vherasymenko
Copy link

vherasymenko commented May 11, 2017 •
edited
Loading

Bug is still exist on latest develop version.
Steps for reproduce correct.

@kassner
Copy link
Contributor

kassner commented May 29, 2017

I'll have a look on it @okorshenko

kassner added a commit to kassner/magento2 that referenced this issue May 29, 2017
@kassner
Fix for magento#5897: getIdentities relies on uninitialized collection
b0c7380
This was referenced May 29, 2017
Merged
Closed
magento-team pushed a commit that referenced this issue Jun 1, 2017
MAGETWO-69540: Fix for #5897: getIdentities relies on uninitialized c…
56441ae 
…ollection #9777

 - Merge Pull Request #9777 from kassner/magento2:patch-11
magento-team pushed a commit that referenced this issue Jun 1, 2017
@ishakhsuvarov
MAGETWO-69540: Fix for #5897: getIdentities relies on uninitialized c…
0b48e93 
…ollection #9777
magento-team pushed a commit that referenced this issue Jun 1, 2017
@ishakhsuvarov
Merge pull request #1158 from magento-engcom/develop-prs
6e15e7d 
[EngCom] Public Pull Requests
 - MAGETWO-69573: Adding logo in media folder #9797
 - MAGETWO-69555: Allow for referenceBlock to include template argument #9772
 - MAGETWO-69540: Fix for #5897: getIdentities relies on uninitialized collection #9777
 - MAGETWO-69533: [BUGFIX][6244] Fix Issue with code label display in cart checkout. #9721
 - MAGETWO-69499: Update select.js #9475
 - MAGETWO-69451: Replace Zend_Json in the configurable product block test #9753
 - MAGETWO-69373: Customer with unique attribute can't be saved #7844 #9712
 - MAGETWO-69369: Replace the direct usage of Zend_Json with a call to the Json Help class #9344
 - MAGETWO-69085: Do not hardcode product link types #9600
 - MAGETWO-69554: Patch to allow multiple filter_url_params to function #9723
@okorshenko
Copy link
Contributor

Fixed in #9777 and merged to develop branch

@hostep
Copy link
Contributor Author

hostep commented Jun 8, 2017

Thanks @kassner & @okorshenko!

@vovayatsyuk vovayatsyuk mentioned this issue Aug 16, 2017
Closed
magento-engcom-team pushed a commit that referenced this issue Jul 17, 2020
@slavvka
Merge pull request #5897 from magento-performance/MC-35827-2
e0bf5fe 
Fixed issues:
- MC-35884 Consumers improvement
- MC-35827 Test and merge PR with extended message queue consumer configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kassner kassner

Labels
bug report Component: Cms Component: Framework/Cache
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@hostep @choukalos @kassner @andimov @surfer190 @ishakhsuvarov @okorshenko @vherasymenko @KrystynaKabannyk