Installing some plugins from plugin store not behaving in same way as installing them locally

[BenParizek]

Description

A user reports that installing the Lite version of Sprout Email from the Plugin Store results in an error:

A fatal error has occurred:

Status: Found
Response:

I can recreate the error when installing from the Plugin Store but not when installing locally. I can also recreate the error when installing the full Trial version of Sprout Email and a Lite version of Sprout Forms. Again, in both cases, only when installing from the Plugin Store and not when installing locally.

It appears the plugin does get installed correctly and the error is thrown only at the last moment when Craft tries to redirect things in the BaseUpdateController::actionFinish() where the Return URL value appears to be null:

cms/src/controllers/BaseUpdaterController.php

Lines 251 to 260 in 1d69bfb

	public function actionFinish(): Response
	{
	// Disable maintenance mode
	Craft::$app->disableMaintenanceMode();
	return $this->send([
	'finished' => true,
	'returnUrl' => $this->returnUrl(),
	]);
	}

I've also tried installing the Lite version of Craft Commerce and it works fine. Could this be caused by something I am overlooking in the Sprout plugins? If so, any ideas what that may be? These were working at one point in time and I haven't been successful in tracking down any reason this behavior may be different than what is happening in Craft Commerce with the same install action from the Plugin Store.

Steps to reproduce

1. Install Sprout Email Trial version from the plugin store
2. See error

Additional info

• Craft version: 3.5
• PHP version: 7.3
• Database driver & version: mysql
• Plugins & versions: various

[brandonkelly]

Is there a specific error listed in storage/logs/web.log or phperrors.log?

It appears the plugin does get installed correctly and the error is thrown only at the last moment when Craft tries to redirect things in the BaseUpdateController::actionFinish() where the Return URL value appears to be null:
https://github.com/craftcms/cms/blob/3.5.16/src/controllers/BaseUpdaterController.php#L251

Not sure how that could be possible, since returnUrl() must return a string, enforced by its return type declaration:

cms/src/controllers/BaseUpdaterController.php

Line 318 in 1d69bfb

abstract protected function returnUrl(): string;

The Plugin Store installs plugins using craft\controllers\pluginstore\InstallController, which ensures a string is returned:

cms/src/controllers/pluginstore/InstallController.php

Lines 236 to 239 in 1d69bfb

	protected function returnUrl(): string
	{
	return $this->_pluginRedirect ?? $this->data['returnUrl'] ?? 'plugin-store';
	}

[BenParizek]

I'm not seeing anything in the phperrors.log and the web.log doesn't appear to have any errors related to this.

The error I see at the completion of the installation (after Sprout Email gets installed but before it redirects correctly):
https://share.barrelstrength.co/nOuDY0wQ

And, the most relevant thing I see in the web.log would just be the POST data during some of the composer actions which appears to have returnUrl set to null, though I'm not sure where this POST data comes from and if that value would be expected at this point of the request:

$_POST = [
    'data' => 'af76ca48eb85164338a21383d453d98b066bb9b3d255542132c049be74bf7469{\"packageName\":\"barrelstrength/sprout-email\",\"handle\":\"sprout-email\",\"edition\":\"lite\",\"version\":\"4.4.8\",\"requirements\":{\"barrelstrength/sprout-email\":\"4.4.8\"},\"removed\":false,\"licenseKey\":null,\"returnUrl\":null,\"postPrecheckState\":{\"nextAction\":\"composer-install\",\"status\":\"Updating Composer dependencies (this may take a minute)…\"}}'
]

If I add log statements in the returnUrl method for each of those values, it appears they are both null. I've bolded the line that indicates what I was trying to dump and any value that appears after the semi-colon is what was logged. In the first three cases, there was nothing logged.

---

2020-11-30 22:06:57 [-][1][-][trace][yii\base\InlineAction::runWithParams] Running action: craft\controllers\pluginstore\InstallController::actionFinish()

2020-11-30 22:06:57 [-][1][-][error][craft\controllers\pluginstore\InstallController::returnUrl]
$this->_pluginRedirect:

2020-11-30 22:06:57 [-][1][-][error][craft\controllers\pluginstore\InstallController::returnUrl]
$this->data['returnUrl']:

2020-11-30 22:06:57 [-][1][-][error][craft\controllers\pluginstore\InstallController::returnUrl]
$this->_pluginRedirect ?? $this->data['returnUrl'] ?? 'plugin-store':

2020-11-30 22:06:57 [-][1][-][error][craft\controllers\BaseUpdaterController::actionFinish]
$this->returnUrl():plugin-store

---

So I guess plugin-store is being returned, however, I still see the error message in the first screenshot rather than being redirected anywhere after installation:

A fatal error has occurred:

Status: error
Response:

[brandonkelly]

I can reproduce with Sprout Email, and will look into it.

[brandonkelly]

Got this fixed for the next release. The error was a side effect of yiisoft/yii2#18083 which was merged into Yii 2.0.36, included in Craft 3.5.0. The PR added $request and $response properties to controllers, which store references to the app’s request and response objects, and Controller::redirect() was updated to call $this->response->redirect() instead of Yii::$app->getResponse()->redirect().

Before Craft runs the installer, it will swap out the app’s response object with a temporary one, in case a plugin attempts to redirect the request (which can’t be directly allowed in this context, given the way the installer works over Ajax). It will then check if the temp response contains a Location header, and if so, include that value as the returnUrl key in the JSON response, so the JavaScript can carry out the redirect instead.

I had tested that redirects still work in 3.5.0 with SEOmatic, but the way that plugin redirects the request is different than Sprout Email. It calls Craft::$app->getResponse()->redirect(), whereas Sprout Email calls Craft::$app->controller->redirect(). Those were basically identical before 3.5.0, but with the changes in yiisoft/yii2#18083, in Sprout Email’s case, it’s now talking to the real response object, instead of the temporary one. So I’ve fixed by ensuring that $this->response is also set to the temporary response, in addition to calling Craft::$app->set('response', $tempResponse).

[brandonkelly]

Craft 3.5.17 is out now with this fix.