Skip to content

Commit 1c37bf3

Browse files
author
Yuri Kovsher
committed
MAGETWO-34920: Catch all exceptions inside of Phrase::__toString() method and log messages
1 parent 8959beb commit 1c37bf3

File tree

8 files changed

+128
-31
lines changed

8 files changed

+128
-31
lines changed

‎lib/internal/Magento/Framework/Phrase.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ public function getArguments()
9393
*/
9494
public function render()
9595
{
96-
return self::$renderer ? self::$renderer->render([$this->text], $this->arguments) : $this->text;
96+
try {
97+
return self::$renderer ? self::$renderer->render([$this->text], $this->getArguments()) : $this->getText();
98+
} catch (\Exception $e) {
99+
return $this->getText();
100+
}
97101
}
98102

99103
/**
@@ -103,11 +107,7 @@ public function render()
103107
*/
104108
public function __toString()
105109
{
106-
try {
107-
return $this->render();
108-
} catch (\Exception $e) {
109-
return $this->getText();
110-
}
110+
return $this->render();
111111
}
112112

113113
/**

‎lib/internal/Magento/Framework/Phrase/Renderer/Composite.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class Composite implements RendererInterface
1717
protected $_renderers;
1818

1919
/**
20-
* @param RendererInterface[] $renderers
20+
* @param \Magento\Framework\Phrase\RendererInterface[] $renderers
2121
* @throws \InvalidArgumentException
2222
*/
2323
public function __construct(array $renderers)

‎lib/internal/Magento/Framework/Phrase/Renderer/Inline.php

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77
*/
88
namespace Magento\Framework\Phrase\Renderer;
99

10-
class Inline implements \Magento\Framework\Phrase\RendererInterface
10+
use Magento\Framework\Phrase\RendererInterface;
11+
use Magento\Framework\TranslateInterface;
12+
use Magento\Framework\Translate\Inline\ProviderInterface;
13+
use Psr\Log\LoggerInterface;
14+
15+
class Inline implements RendererInterface
1116
{
1217
/**
1318
* @var \Magento\Framework\TranslateInterface
@@ -19,16 +24,24 @@ class Inline implements \Magento\Framework\Phrase\RendererInterface
1924
*/
2025
protected $inlineProvider;
2126

27+
/**
28+
* @var \Psr\Log\LoggerInterface
29+
*/
30+
protected $logger;
31+
2232
/**
2333
* @param \Magento\Framework\TranslateInterface $translator
2434
* @param \Magento\Framework\Translate\Inline\ProviderInterface $inlineProvider
35+
* @param \Psr\Log\LoggerInterface $logger
2536
*/
2637
public function __construct(
27-
\Magento\Framework\TranslateInterface $translator,
28-
\Magento\Framework\Translate\Inline\ProviderInterface $inlineProvider
38+
TranslateInterface $translator,
39+
ProviderInterface $inlineProvider,
40+
LoggerInterface $logger
2941
) {
3042
$this->translator = $translator;
3143
$this->inlineProvider = $inlineProvider;
44+
$this->logger = $logger;
3245
}
3346

3447
/**
@@ -37,22 +50,28 @@ public function __construct(
3750
* @param [] $source
3851
* @param [] $arguments
3952
* @return string
53+
* @throws \Exception
4054
*/
4155
public function render(array $source, array $arguments)
4256
{
4357
$text = end($source);
4458

45-
if (!$this->inlineProvider->get()->isAllowed()) {
46-
return $text;
47-
}
59+
try {
60+
if (!$this->inlineProvider->get()->isAllowed()) {
61+
return $text;
62+
}
4863

49-
if (strpos($text, '{{{') === false
50-
|| strpos($text, '}}}') === false
51-
|| strpos($text, '}}{{') === false
52-
) {
53-
$text = '{{{'
54-
. implode('}}{{', array_reverse($source))
55-
. '}}{{' . $this->translator->getTheme() . '}}}';
64+
if (strpos($text, '{{{') === false
65+
|| strpos($text, '}}}') === false
66+
|| strpos($text, '}}{{') === false
67+
) {
68+
$text = '{{{'
69+
. implode('}}{{', array_reverse($source))
70+
. '}}{{' . $this->translator->getTheme() . '}}}';
71+
}
72+
} catch (\Exception $e) {
73+
$this->logger->critical($e->getMessage());
74+
throw $e;
5675
}
5776

5877
return $text;

‎lib/internal/Magento/Framework/Phrase/Renderer/Placeholder.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
*/
88
namespace Magento\Framework\Phrase\Renderer;
99

10-
class Placeholder implements \Magento\Framework\Phrase\RendererInterface
10+
use Magento\Framework\Phrase\RendererInterface;
11+
12+
class Placeholder implements RendererInterface
1113
{
1214
/**
1315
* Render source text
@@ -23,7 +25,7 @@ public function render(array $source, array $arguments)
2325
if ($arguments) {
2426
$placeholders = [];
2527
foreach (array_keys($arguments) as $key) {
26-
$placeholders[] = "%" . (is_int($key) ? strval($key + 1) : $key);
28+
$placeholders[] = '%' . (is_int($key) ? strval($key + 1) : $key);
2729
}
2830
$text = str_replace($placeholders, $arguments, $text);
2931
}

‎lib/internal/Magento/Framework/Phrase/Renderer/Translate.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,34 @@
77
*/
88
namespace Magento\Framework\Phrase\Renderer;
99

10-
class Translate implements \Magento\Framework\Phrase\RendererInterface
10+
use Magento\Framework\Phrase\RendererInterface;
11+
use Magento\Framework\TranslateInterface;
12+
use Psr\Log\LoggerInterface;
13+
14+
class Translate implements RendererInterface
1115
{
1216
/**
1317
* @var \Magento\Framework\TranslateInterface
1418
*/
1519
protected $translator;
1620

21+
/**
22+
* @var \Psr\Log\LoggerInterface
23+
*/
24+
protected $logger;
25+
1726
/**
1827
* Renderer construct
1928
*
2029
* @param \Magento\Framework\TranslateInterface $translator
30+
* @param \Psr\Log\LoggerInterface $logger
2131
*/
22-
public function __construct(\Magento\Framework\TranslateInterface $translator)
23-
{
32+
public function __construct(
33+
TranslateInterface $translator,
34+
LoggerInterface $logger
35+
) {
2436
$this->translator = $translator;
37+
$this->logger = $logger;
2538
}
2639

2740
/**
@@ -30,12 +43,18 @@ public function __construct(\Magento\Framework\TranslateInterface $translator)
3043
* @param [] $source
3144
* @param [] $arguments
3245
* @return string
46+
* @throws \Exception
3347
*/
3448
public function render(array $source, array $arguments)
3549
{
3650
$text = end($source);
3751

38-
$data = $this->translator->getData();
52+
try {
53+
$data = $this->translator->getData();
54+
} catch (\Exception $e) {
55+
$this->logger->critical($e->getMessage());
56+
throw $e;
57+
}
3958

4059
return array_key_exists($text, $data) ? $data[$text] : $text;
4160
}

‎lib/internal/Magento/Framework/Phrase/Test/Unit/Renderer/CompositeTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,17 @@ public function testRender()
7777

7878
$this->assertEquals($resultAfterSecond, $this->object->render([$text], $arguments));
7979
}
80+
81+
public function testRenderException()
82+
{
83+
$message = 'something went wrong';
84+
$exception = new \Exception($message);
85+
86+
$this->rendererOne->expects($this->once())
87+
->method('render')
88+
->willThrowException($exception);
89+
90+
$this->setExpectedException('Exception', $message);
91+
$this->object->render(['text'], []);
92+
}
8093
}

‎lib/internal/Magento/Framework/Phrase/Test/Unit/Renderer/InlineTest.php

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,36 @@
88
class InlineTest extends \PHPUnit_Framework_TestCase
99
{
1010
/**
11-
* @var \Magento\Framework\TranslateInterface
11+
* @var \Magento\Framework\TranslateInterface|\PHPUnit_Framework_MockObject_MockObject
1212
*/
1313
protected $translator;
1414

1515
/**
1616
* @var \Magento\Framework\Phrase\Renderer\Translate
1717
*/
18-
protected $_renderer;
18+
protected $renderer;
1919

2020
/**
21-
* @var \Magento\Framework\Translate\Inline\ProviderInterface
21+
* @var \Magento\Framework\Translate\Inline\ProviderInterface|\PHPUnit_Framework_MockObject_MockObject
2222
*/
2323
protected $provider;
2424

25+
/**
26+
* @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
protected $loggerMock;
29+
2530
protected function setUp()
2631
{
2732
$this->translator = $this->getMock('Magento\Framework\TranslateInterface', [], [], '', false);
2833
$this->provider = $this->getMock('Magento\Framework\Translate\Inline\ProviderInterface', [], [], '', false);
34+
$this->loggerMock = $this->getMockBuilder('Psr\Log\LoggerInterface')
35+
->getMock();
2936

3037
$this->renderer = new \Magento\Framework\Phrase\Renderer\Inline(
3138
$this->translator,
32-
$this->provider
39+
$this->provider,
40+
$this->loggerMock
3341
);
3442
}
3543

@@ -70,4 +78,17 @@ public function testRenderIfInlineTranslationIsNotAllowed()
7078

7179
$this->assertEquals($text, $this->renderer->render([$text], []));
7280
}
81+
82+
public function testRenderException()
83+
{
84+
$message = 'something went wrong';
85+
$exception = new \Exception($message);
86+
87+
$this->provider->expects($this->once())
88+
->method('get')
89+
->willThrowException($exception);
90+
91+
$this->setExpectedException('Exception', $message);
92+
$this->renderer->render(['text'], []);
93+
}
7394
}

‎lib/internal/Magento/Framework/Phrase/Test/Unit/Renderer/TranslateTest.php

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,24 @@ class TranslateTest extends \PHPUnit_Framework_TestCase
1717
*/
1818
protected $_renderer;
1919

20+
/**
21+
* @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
22+
*/
23+
protected $loggerMock;
24+
2025
protected function setUp()
2126
{
2227
$this->_translator = $this->getMock('Magento\Framework\TranslateInterface', [], [], '', false);
28+
$this->loggerMock = $this->getMockBuilder('Psr\Log\LoggerInterface')
29+
->getMock();
2330

2431
$objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
2532
$this->_renderer = $objectManagerHelper->getObject(
2633
'Magento\Framework\Phrase\Renderer\Translate',
27-
['translator' => $this->_translator]
34+
[
35+
'translator' => $this->_translator,
36+
'logger' => $this->loggerMock
37+
]
2838
);
2939
}
3040

@@ -41,4 +51,17 @@ public function testRender()
4151
$this->assertEquals($translate, $this->_renderer->render([$translatedText], []));
4252
$this->assertEquals($text, $this->_renderer->render([$text], []));
4353
}
54+
55+
public function testRenderException()
56+
{
57+
$message = 'something went wrong';
58+
$exception = new \Exception($message);
59+
60+
$this->_translator->expects($this->once())
61+
->method('getData')
62+
->willThrowException($exception);
63+
64+
$this->setExpectedException('Exception', $message);
65+
$this->_renderer->render(['text'], []);
66+
}
4467
}

0 commit comments

Comments
 (0)