Exception catching in catalog/product/view controller may be too broad

[max_daim2k5]

Hi, 

a user had a "question" in github and maybe you can answer use and bring some light into the darkness ;-)

I was working on adding plugin for block on PDP and made a mistake in plugin class name. I couldn't figure out what is wrong for a while as PDP was returning 404 error. Then I noticed thatMagento\Catalog\Controller\Product\View::execute() displays 404 error if any Exception is thrown.

Isn't it too broad? I think it would be better to throw 404 only when product doesn't exist and let other exceptions go through.

For reference, I used Training\Test\Block\Product\View\DescriptionPlugin class name and put this in di.xml file:

<type name="Magento\Catalog\Block\Product\View\Description">
    <plugin name="product-view-desription-plugin"
            type="Training\Test\Block\Product\View\Description" sortOrder="10" />
</type>

On github https://github.com/magento/magento2/issues/1918#issuecomment-158523677 

[akent99]

I tend to know "a little bit about a lot of things", so when it gets this detailed a question I might not be able to help as much.

Generally we don't want to return exceptions to customers on a site - it reflects poorly on the site quality.

Some questions

• Are you using developer mode?  That is, set the MAGE_MODE environment variable to "developer" - it might display additional information if you are not doing this already.
• Have you looked in the var/log directory for error messages? Or the web server log files?  Was the exception information shown anywhere?  If not, that may be considered a bug if a simple error does not generate a simple error message.

[Wojtek N]

Alan, I'm one who reported this issue in github.

My intention wasn't to change it to return exception to customer. The way Magento does that now is good, as it displays nice generic error page and logs full detailed error in log.

The point is that PDP controller works differently than other Magento 2 parts - it catches all possible exceptions and doesn't trigger nice generic error page but displays 404 page every time.  

I think it would be better to:

* show 404 page only if "not found" exception is caught there

* let other exceptions go through to let bootstrap catch them and display generic error page

[akent99]

I am keeping this open for now. If it is already resolved please let me know. I will try to have a browse sometime (an excuse to learn more about the code base myself!), but no promises.