Use per-store or per-website skin if available in the error application #35095

fredden · 2022-02-13T16:42:57Z

Description (*)

When Magento is unable or unwilling to process a request (eg, because maintenance mode has been enabled) the request is forwarded to the application within pub/errors/. This uses a different set of theme files to the main website (intentionally), but is not flexible when dealing with multi-store or multi-website configurations.

This is not necessarily a problem when all the stores/website share a common theme and language. (For example, British & American & Australian stores with the same brand but different currencies / localisation will all be fine using a shared skin/theme in English for pub/errors/.) However, when there are multiple distinct brands being served from a single Magento instance or when different languages are being used, a single skin for pub/errors/ is not suitable.

We have implemented a similar approach to what is being proposed here on websites for our merchants with good results. Here we are using the MAGE_RUN_TYPE and MAGE_RUN_CODE environment variables to determine which skin / theme to use in error pages.

Calling _setSkin() multiple times is safe. If there is no suitable skin matching the name of the store/website, then these lines have no impact, falling back to other skin-setting mechanisms like $_GET['skin'] or pub/errors/local.xml or pub/errors/design.xml.

Related Pull Requests

Update documentation to match new feature - localised error pages devdocs#9363

Fixed Issues

None. Please feel free to create an issue if you feel this is appropriate.

Manual testing scenarios (*)

Set up Magento with multi-store or multi-website, ensure these have distinct URLs, eg https://de.magento.test/ https://es.magento.test/ https://fr.magento.test/. - see guide here for details: https://devdocs.magento.com/guides/v2.4/config-guide/multi-site/ms_over.html
Enable maintenance mode with bin/magento maintenance:enable
Navigate to each store front. Notice that all serve the same skin for pub/errors/.
Create a new skin for error pages by copying pub/errors/local.xml.sample to pub/errors/local.xml, and pub/errors/default/ to pub/errors/custom/, and setting <skin>custom</skin> in pub/errors/local.xml.
Navigate to each store front. Notice that all serve the same skin for pub/errors/.
Notice no ability to customise the errors per-store nor per-website before the changes here.
Apply the changes in this pull request.
Create new skins for each store/website as desired. For example, copy pub/errors/custom/ to pub/errors/es/ and translate/theme this appropriately.
Navigate to each store front. Notice that a different skin is applied to each where this exists, and that where no matching theme/skin exists, the default is used.

Questions or comments

None

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)

m2-assistant · 2022-02-13T16:42:59Z

Hi @fredden. Thank you for your contribution
Here are some useful tips 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 Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

magento-automated-testing · 2022-02-13T16:45:39Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

engcom-Hotel · 2022-02-14T05:52:37Z

@magento run static tests

magento-automated-testing · 2022-02-14T05:52:41Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

magento-automated-testing · 2022-10-21T11:30:55Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

magento-automated-testing · 2022-11-04T12:17:33Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

leonhelmus

I do suggest another approach the approach taken now should do as well.

leonhelmus · 2023-03-20T08:24:46Z

pub/errors/processor.php

@@ -186,6 +186,15 @@ public function __construct(
        $this->_root  = is_dir($this->_indexDir . 'app');

        $this->_prepareConfig();
+
+        if (isset($_ENV['MAGE_RUN_CODE'])) {


Suggested change

if (isset($_ENV['MAGE_RUN_CODE'])) {

if (isset($_ENV['MAGE_RUN_CODE'])) {

$skin = isset($_ENV['MAGE_RUN_TYPE']) ? $ENV_['MAGE_RUN_TYPE'] . '-' . $_ENV['MAGE_RUN_CODE'] : $_ENV['MAGE_RUN_CODE'];

$this->_setSkin($skin);

}

This way you prevent a second request of the same method _setSkin()

Calling _setSkin() multiple times (and in this order) is intentional. The _setSkin() method includes some name validation and (importantly) an is_dir() check. With the current implementation, a developer can create a per-website theme, even if there is a store code in the web server configuration. With the proposed implementation, a per-website theme is ignored / unusable when a store code is specified in web server configuration.

Edit: I've clearly forgotten how the code actually works; it's been a long time since I opened this pull request. The double-call allows developers to differentiate between a store-hats and website-hats (or store-hats and hats) while keeping the simple case simple with just hats when the same theme is used in both scopes. Like with other places in Magento, the most specific option will be used.

@leonhelmus would it help if I add a code comment to explain why we're calling _setSkin() multiple times?

…bsite-errors

magento-automated-testing · 2023-03-20T09:41:07Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

Use per-store or per-website skin if available

0091e15

magento-engcom-team added the Release Line: 2.4 label Feb 13, 2022

m2-github-services added Partner: Fisheye partners-contribution Pull Request is created by Magento Partner labels Feb 13, 2022

fredden mentioned this pull request Feb 13, 2022

Update documentation to match new feature - localised error pages magento/devdocs#9363

Merged

m2-community-project bot added the Progress: pending review label Feb 13, 2022

sdzhepa added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Feb 17, 2022

Merge branch '2.4-develop' into per-store-website-errors

f45a2ca

Merge branch '2.4-develop' into per-store-website-errors

af837d1

leonhelmus approved these changes Mar 20, 2023

View reviewed changes

m2-community-project bot assigned leonhelmus Mar 20, 2023

m2-community-project bot added Progress: ready for testing and removed Progress: pending review labels Mar 20, 2023

fredden added 2 commits March 20, 2023 09:19

Merge remote-tracking branch 'upstream/2.4-develop' into per-store-we…

6706279

…bsite-errors

Permit dash in skin name

051acf8

m2-community-project bot added Progress: review and removed Progress: ready for testing labels Mar 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use per-store or per-website skin if available in the error application #35095

Use per-store or per-website skin if available in the error application #35095

Uh oh!

fredden commented Feb 13, 2022 •

edited

Loading

Uh oh!

m2-assistant bot commented Feb 13, 2022

Uh oh!

magento-automated-testing bot commented Feb 13, 2022

Uh oh!

engcom-Hotel commented Feb 14, 2022

Uh oh!

magento-automated-testing bot commented Feb 14, 2022

Uh oh!

magento-automated-testing bot commented Oct 21, 2022

Uh oh!

magento-automated-testing bot commented Nov 4, 2022

Uh oh!

leonhelmus left a comment

Uh oh!

leonhelmus Mar 20, 2023

Uh oh!

leonhelmus Mar 20, 2023

Uh oh!

fredden Mar 20, 2023 •

edited

Loading

Uh oh!

fredden Mar 20, 2023

Uh oh!

magento-automated-testing bot commented Mar 20, 2023

Uh oh!

Uh oh!

Use per-store or per-website skin if available in the error application #35095

Are you sure you want to change the base?

Use per-store or per-website skin if available in the error application #35095

Uh oh!

Conversation

fredden commented Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Related Pull Requests

Fixed Issues

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Feb 13, 2022

Uh oh!

magento-automated-testing bot commented Feb 13, 2022

Uh oh!

engcom-Hotel commented Feb 14, 2022

Uh oh!

magento-automated-testing bot commented Feb 14, 2022

Uh oh!

magento-automated-testing bot commented Oct 21, 2022

Uh oh!

magento-automated-testing bot commented Nov 4, 2022

Uh oh!

leonhelmus left a comment

Choose a reason for hiding this comment

Uh oh!

leonhelmus Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

leonhelmus Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

fredden Mar 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredden Mar 20, 2023

Choose a reason for hiding this comment

Uh oh!

magento-automated-testing bot commented Mar 20, 2023

Uh oh!

Uh oh!

fredden commented Feb 13, 2022 •

edited

Loading

fredden Mar 20, 2023 •

edited

Loading