Skip to content

Commit 38d4b9a

Browse files
author
Oleksii Korshenko
authored
Merge pull request #158 from magento-tango/MAGETWO-54934
[Tango] Static content errors improvements
2 parents f553937 + bf81171 commit 38d4b9a

File tree

6 files changed

+71
-17
lines changed

6 files changed

+71
-17
lines changed

app/code/Magento/Deploy/Model/Deployer.php

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Magento\Framework\ObjectManagerInterface;
1818
use Magento\Framework\Translate\Js\Config as JsTranslationConfig;
1919
use Symfony\Component\Console\Output\OutputInterface;
20+
use Psr\Log\LoggerInterface;
2021
use Magento\Framework\View\Asset\Minification;
2122
use Magento\Framework\App\ObjectManager;
2223

@@ -81,6 +82,11 @@ class Deployer
8182
*/
8283
private $minification;
8384

85+
/**
86+
* @var LoggerInterface
87+
*/
88+
private $logger;
89+
8490
/**
8591
* Constructor
8692
*
@@ -166,17 +172,17 @@ public function deploy(ObjectManagerFactory $omFactory, array $locales)
166172
);
167173
$fileManager->createRequireJsConfigAsset();
168174
foreach ($appFiles as $info) {
169-
list($fileArea, $fileTheme, , $module, $filePath) = $info;
175+
list($fileArea, $fileTheme, , $module, $filePath, $fullPath) = $info;
170176
if (($fileArea == $area || $fileArea == 'base') &&
171177
($fileTheme == '' || $fileTheme == $themePath ||
172178
in_array(
173179
$fileArea . Theme::THEME_PATH_SEPARATOR . $fileTheme,
174180
$this->findAncestors($area . Theme::THEME_PATH_SEPARATOR . $themePath)
175181
))
176182
) {
177-
$compiledFile = $this->deployFile($filePath, $area, $themePath, $locale, $module);
183+
$compiledFile = $this->deployFile($filePath, $area, $themePath, $locale, $module, $fullPath);
178184
if ($compiledFile !== '') {
179-
$this->deployFile($compiledFile, $area, $themePath, $locale, $module);
185+
$this->deployFile($compiledFile, $area, $themePath, $locale, $module, $fullPath);
180186
}
181187
}
182188
}
@@ -321,14 +327,15 @@ protected function emulateApplicationLocale($locale, $area)
321327
* @param string $themePath
322328
* @param string $locale
323329
* @param string $module
330+
* @param string|null $fullPath
324331
* @return string
325332
* @throws \InvalidArgumentException
326333
* @throws LocalizedException
327334
*
328335
* @SuppressWarnings(PHPMD.NPathComplexity)
329336
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
330337
*/
331-
private function deployFile($filePath, $area, $themePath, $locale, $module)
338+
private function deployFile($filePath, $area, $themePath, $locale, $module, $fullPath = null)
332339
{
333340
$compiledFile = '';
334341
$extension = pathinfo($filePath, PATHINFO_EXTENSION);
@@ -368,7 +375,13 @@ private function deployFile($filePath, $area, $themePath, $locale, $module)
368375
}
369376
$this->count++;
370377
} catch (ContentProcessorException $exception) {
371-
throw $exception;
378+
$pathInfo = $fullPath ?: $filePath;
379+
$errorMessage = __('Compilation from source: ') . $pathInfo
380+
. PHP_EOL . $exception->getMessage();
381+
$this->errorCount++;
382+
$this->output->write(PHP_EOL . PHP_EOL . $errorMessage . PHP_EOL, true);
383+
384+
$this->getLogger()->critical($errorMessage);
372385
} catch (\Exception $exception) {
373386
$this->output->write('.');
374387
$this->verboseLog($exception->getTraceAsString());
@@ -409,4 +422,19 @@ private function verboseLog($message)
409422
$this->output->writeln($message);
410423
}
411424
}
425+
426+
/**
427+
* Retrieves LoggerInterface instance
428+
*
429+
* @return LoggerInterface
430+
* @deprecated
431+
*/
432+
private function getLogger()
433+
{
434+
if (!$this->logger) {
435+
$this->logger = $this->objectManager->get(LoggerInterface::class);
436+
}
437+
438+
return $this->logger;
439+
}
412440
}

dev/tests/integration/testsuite/Magento/Email/Model/Template/FilterTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public function cssDirectiveDataProvider()
270270
'File with compilation error results in error message' => [
271271
TemplateTypesInterface::TYPE_HTML,
272272
'file="css/file-with-error.css"',
273-
\Magento\Framework\View\Asset\ContentProcessorInterface::ERROR_MESSAGE_PREFIX,
273+
'variable @non-existent-variable is undefined',
274274
],
275275
];
276276
}
@@ -356,7 +356,7 @@ public function inlinecssDirectiveDataProvider()
356356
],
357357
'Developer mode - File with compilation error results in error message' => [
358358
'<html><p></p> {{inlinecss file="css/file-with-error.css"}}</html>',
359-
\Magento\Framework\View\Asset\ContentProcessorInterface::ERROR_MESSAGE_PREFIX,
359+
'CSS inlining error:',
360360
false,
361361
],
362362
];

lib/internal/Magento/Framework/App/StaticResource.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Magento\Framework\App\Filesystem\DirectoryList;
99
use Magento\Framework\ObjectManager\ConfigLoaderInterface;
1010
use Magento\Framework\Filesystem;
11+
use Psr\Log\LoggerInterface;
1112

1213
/**
1314
* Entry point for retrieving static resources like JS, CSS, images by requested public path
@@ -43,6 +44,9 @@ class StaticResource implements \Magento\Framework\AppInterface
4344
/** @var Filesystem */
4445
private $filesystem;
4546

47+
/** @var LoggerInterface */
48+
private $logger;
49+
4650
/**
4751
* @param State $state
4852
* @param Response\FileInterface $response
@@ -105,6 +109,7 @@ public function launch()
105109
*/
106110
public function catchException(Bootstrap $bootstrap, \Exception $exception)
107111
{
112+
$this->getLogger()->critical($exception->getMessage());
108113
if ($bootstrap->isDeveloperMode()) {
109114
$this->response->setHttpResponseCode(404);
110115
$this->response->setHeader('Content-Type', 'text/plain');
@@ -161,4 +166,19 @@ private function getFilesystem()
161166
}
162167
return $this->filesystem;
163168
}
169+
170+
/**
171+
* Retrieves LoggerInterface instance
172+
*
173+
* @return LoggerInterface
174+
* @deprecated
175+
*/
176+
private function getLogger()
177+
{
178+
if (!$this->logger) {
179+
$this->logger = $this->objectManager->get(LoggerInterface::class);
180+
}
181+
182+
return $this->logger;
183+
}
164184
}

lib/internal/Magento/Framework/App/Test/Unit/StaticResourceTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ class StaticResourceTest extends \PHPUnit_Framework_TestCase
5858
*/
5959
private $object;
6060

61+
/**
62+
* @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
63+
*/
64+
private $logger;
65+
6166
protected function setUp()
6267
{
6368
$this->state = $this->getMock('Magento\Framework\App\State', [], [], '', false);
@@ -67,6 +72,7 @@ protected function setUp()
6772
$this->assetRepo = $this->getMock('Magento\Framework\View\Asset\Repository', [], [], '', false);
6873
$this->moduleList = $this->getMock('Magento\Framework\Module\ModuleList', [], [], '', false);
6974
$this->objectManager = $this->getMockForAbstractClass('Magento\Framework\ObjectManagerInterface');
75+
$this->logger = $this->getMockForAbstractClass('Psr\Log\LoggerInterface');
7076
$this->configLoader = $this->getMock(
7177
'Magento\Framework\App\ObjectManager\ConfigLoader', [], [], '', false
7278
);
@@ -193,6 +199,12 @@ public function testLaunchWrongPath()
193199

194200
public function testCatchExceptionDeveloperMode()
195201
{
202+
$this->objectManager->expects($this->once())
203+
->method('get')
204+
->with('Psr\Log\LoggerInterface')
205+
->willReturn($this->logger);
206+
$this->logger->expects($this->once())
207+
->method('critical');
196208
$bootstrap = $this->getMockBuilder(Bootstrap::class)->disableOriginalConstructor()->getMock();
197209
$bootstrap->expects($this->once())->method('isDeveloperMode')->willReturn(true);
198210
$exception = new \Exception('Error: nothing works');

lib/internal/Magento/Framework/Css/PreProcessor/Adapter/Less/Processor.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,7 @@ public function processContent(File $asset)
9494

9595
return $content;
9696
} catch (\Exception $e) {
97-
$errorMessage = PHP_EOL . self::ERROR_MESSAGE_PREFIX . PHP_EOL . $path . PHP_EOL . $e->getMessage();
98-
$this->logger->critical($errorMessage);
99-
100-
throw new ContentProcessorException(new Phrase($errorMessage));
97+
throw new ContentProcessorException(new Phrase($e->getMessage()));
10198
}
10299
}
103100
}

lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Adapter/Less/ProcessorTest.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ protected function setUp()
8181
* Test for processContent method (exception)
8282
*
8383
* @expectedException \Magento\Framework\View\Asset\ContentProcessorException
84-
* @expectedExceptionMessageRegExp (Compilation from source:.*Test exception)
84+
* @expectedExceptionMessageRegExp (Test exception)
8585
*/
8686
public function testProcessContentException()
8787
{
@@ -96,11 +96,8 @@ public function testProcessContentException()
9696
->with($assetMock)
9797
->willThrowException(new \Exception(self::ERROR_MESSAGE));
9898

99-
$this->loggerMock->expects(self::once())
100-
->method('critical')
101-
->with(
102-
PHP_EOL . Processor::ERROR_MESSAGE_PREFIX . PHP_EOL . self::ASSET_PATH . PHP_EOL . self::ERROR_MESSAGE
103-
);
99+
$this->loggerMock->expects(self::never())
100+
->method('critical');
104101

105102
$this->temporaryFileMock->expects(self::never())
106103
->method('createFile');

0 commit comments

Comments
 (0)