Upgrade to Magento 2.2.5 with patches in place. Area code is not set in Magento_Sales fillQuoteAddressIdInSalesOrderAddress

[daniel-ifrim]

Preconditions

Steps to reproduce

1. Upgrading a store via composer. Version Magento 2.2.2 to Magento 2.2.5
2. Run composer update -vvv
3. Apply 2 patches: CE-MAGETWO-93036-2018-07-02-07-06-53.patch and htdocs/CE-MAGETWO-93083-COMPOSER-2018-07-16-03-39-02.patch
4. Run php bin/magento setup:upgrade
5. When Magento_Sales upgrades, in UpgradeData.php, function fillQuoteAddressIdInSalesOrderAddress() throws an error Area code is not set.

Expected result

1. No error

Actual result

1. php bin/magento setup:upgrade fails

I guess I've been doing the upgrade slightly differently.
I run php bin/magento setup:upgrade after applying the 2 patches.
In future releases, upgrading from a previous Magento 2 version could fail because of this error.

The error is triggered in vendor/magento/framework/Session/SidResolver.php, function getSid():

Magento\Framework\Session\SidResolver\Proxy->getSid(Object(Magento\Framework\Session\Generic\Interceptor))
Magento\Framework\Session\SidResolver->getSid(Object(Magento\Framework\Session\Generic\Interceptor))
Magento\Framework\App\State->getAreaCode()

Even if fillQuoteAddressIdInSalesOrderAddress is called in an emulated area with area code set:

$this->state->emulateAreaCode(
    \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE,
    [$this, 'fillQuoteAddressIdInSalesOrderAddress'],
    [$setup]
);

it stills fails because in Magento\Framework\Session\SidResolver $this->appState is a Magento style "singleton"/shared class:
$this->appState = $appState ?: \Magento\Framework\App\ObjectManager::getInstance()->get(State::class);
where in UpgradeData.php, $this->state is a new instance of the same class.
Area code is not seen in $this->appState from SidResolver class.

I guess the fix is something like:

$this->state = \Magento\Framework\App\ObjectManager::getInstance()->get(State::class);

in __construct(), vendor/magento/module-sales/Setup/UpgradeData.php.
Or declare the state parameter as shared in di.xml, arguments of UpgradeData.php.
This works for me. I am not assuming it is a perfect solution that does not has the potential to impact the other Magento 2 modules setup upgrade.

[magento-engcom-team]

Hi @daniel-ifrim. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento-engcom-team give me {$VERSION} instance

where {$VERSION} is version tags (starting from 2.2.0+) or develop branches (2.2-develop +).
For more details, please, review the Magento Contributor Assistant documentation.

@daniel-ifrim do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?

• yes
• no

[daniel-ifrim]

@magento-engcom-team give me 2.2.2 instance

[magento-engcom-team]

Hi @daniel-ifrim. Thank you for your request. I'm working on Magento 2.2.2 instance for you

[magento-engcom-team]

Hi @daniel-ifrim, here is your Magento instance.
Admin access: https://i-17434-2-2-2.engcom.dev.magento.com/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

[daniel-ifrim]

It seems, I will have to try to reproduce the issue on my system using vanilla Magento 2.2.2 and do the steps.

[daniel-ifrim]

Btw, it looks to me like fillQuoteAddressIdInSalesOrderAddress() from UpgradeData should work with SQL queries $connection->update(), $connection->select() .. instead of accessing and saving data through entire models and classes. It modifies one column data in a table and updates old orders.
A patch can use alternate code than the usual heavy models / classes.
This is an opinion.

[engcom-backlog-nickolas]

Hello @daniel-ifrim, thank you for your report.
We've acknowledged the issue and added to our backlog.
Will be fixed in #16570