Fix exception handling inconsistency between developer and production modes #37712

iGerchak · 2023-07-04T11:17:03Z

Description (*)

During developing our custom module was found an issue in the developer mode, for example, after implementing \Magento\Customer\Controller\AccountInterface class into the custom controller we got redirected to the login page (as was expected), but on the production on the same flow we got an internal server error 500.
It happened because the redirect URL was set to the response before the trigger exception and as a result Location header was sent to the browser and the browser redirect to the login page instead of the page with an exception message.
It's not expected flow between developer and production modes and can cause some issues for production.

Preconditions and environment

Magento 2.4.5
Controller, which sets a redirect and throws an exception

Steps to reproduce

Create a module:

app/code/Vendor/Module/registration.php:

<?php

use Magento\Framework\Component\ComponentRegistrar;

ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Vendor_Module', __DIR__);

app/code/Vendor/Module/etc/module.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
    <module name="Vendor_Module"/>
</config>

app/code/Vendor/Module/etc/frontend/routes.xml:

<?xml version="1.0" encoding="UTF-8"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:App/etc/routes.xsd">
    <router id="standard">
        <route id="vendor" frontName="vendor">
            <module name="Vendor_Module"/>
        </route>
    </router>
</config>

magento2/app/code/Vendor/Module/Controller/Example/Index.php:

<?php

namespace Vendor\Module\Controller\Example;

use LogicException;
use Magento\Customer\Model\Session;
use Magento\Framework\App\Action\HttpGetActionInterface;
use Magento\Framework\App\ResponseInterface;
use Magento\Framework\Controller\ResultInterface;
use Magento\Framework\Exception\SessionException;

class Index implements HttpGetActionInterface
{
    public function __construct(private readonly Session $customerSession,)
    {
    }

    /**
     * Execute action based on request and return result
     *
     * @return ResultInterface|ResponseInterface
     * @throws LogicException
     * @throws SessionException
     */
    public function execute()
    {
        // Here, you will be redirected to the login page when not logged in.
        $this->customerSession->authenticate();

        throw new LogicException(__('Some exception text'));
    }
}

Go to www.domain.com/vendor/example

Expected result

Page with exception text like this one:

Actual result

Redirect to the login page

Unfortunately, I don't have enough experience with test coverage, will be great if someone covers this case by test.

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Fix exception handling inconsistency between developer and production modes #38639: Fix exception handling inconsistency between developer and production modes

m2-assistant · 2023-07-04T11:17:06Z

Hi @iGerchak. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

ihor-sviziev · 2023-07-04T11:29:32Z

@magento run all tests

magento-automated-testing · 2023-07-04T11:29:39Z