diff --git a/core/components/pdotools/docs/changelog.txt b/core/components/pdotools/docs/changelog.txt index b62d9f3..0f4bff4 100644 --- a/core/components/pdotools/docs/changelog.txt +++ b/core/components/pdotools/docs/changelog.txt @@ -15,6 +15,7 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) - [#358] [pdoTools] cachePath can be set via the "pdotools_cache_path" option. - [#392] [pdoPage] Snippet properties are available as placeholders in pagination templates. - Added GitHub Actions release workflow for 3.x transport packages. +- [pdoTools3#21] [Fenom] Compile and runtime errors log the element name, resource, source excerpt, and cache path instead of a bare content hash. ### Changed diff --git a/core/components/pdotools/src/CoreTools.php b/core/components/pdotools/src/CoreTools.php index 7bf5236..52a7b74 100644 --- a/core/components/pdotools/src/CoreTools.php +++ b/core/components/pdotools/src/CoreTools.php @@ -723,6 +723,7 @@ protected function _loadElement($name, $type, $row = []) } $properties = []; + $sourceFile = ''; /** @var modElement $element */ switch ($binding) { case 'CODE': @@ -765,6 +766,7 @@ protected function _loadElement($name, $type, $row = []) return false; } + $sourceFile = $path; if ($content = file_get_contents($path)) { $element = $this->modx->newObject($type, ['name' => $cache_name]); $element->setContent($content); @@ -820,6 +822,16 @@ protected function _loadElement($name, $type, $row = []) return false; } + if ($sourceFile === '' && $element instanceof modElement && method_exists($element, 'isStatic') && $element->isStatic()) { + $file = $element->getSourceFile(); + if (!empty($file)) { + $sourceFile = $file; + } + } + $elementName = $type === 'modTemplate' + ? (string)$element->get('templatename') + : (string)$element->get('name'); + $data = [ 'object' => $element, 'content' => $content, @@ -827,7 +839,10 @@ protected function _loadElement($name, $type, $row = []) 'name' => $cache_name, 'id' => (int)$element->get('id'), 'binding' => strtolower($type), + 'origin' => $binding, 'cacheable' => $cacheable, + 'elementName' => $elementName, + 'sourceFile' => $sourceFile, ]; $this->setStore($cache_key, $data, $type); diff --git a/core/components/pdotools/src/Parsing/Fenom/ErrorLog.php b/core/components/pdotools/src/Parsing/Fenom/ErrorLog.php new file mode 100644 index 0000000..197fcfd --- /dev/null +++ b/core/components/pdotools/src/Parsing/Fenom/ErrorLog.php @@ -0,0 +1,290 @@ + 0) { + $label .= ' (#' . $id . ')'; + } + if ($file !== '') { + $label .= ' file:' . $file; + } + + return $label; + } + if ($id > 0) { + return ($kind !== '' ? $kind : 'element') . ':#' . $id; + } + if ($file !== '') { + return 'file:' . $file; + } + + $resource = self::formatResource($source); + if ($resource !== '') { + return $resource; + } + + return $cacheName !== '' ? $cacheName : 'unknown'; + } + + /** + * Full MODX / &showLog error block. + * + * @param Throwable $e + * @param string $cacheName + * @param string $content + * @param string $label + * @param string $phase compile|runtime + * @param array $extra compiled, sourceDump (paths), resource (facts for a second line) + * @return string + */ + public static function format(Throwable $e, $cacheName, $content, $label, $phase, array $extra = []) + { + if ($label === '') { + $label = $cacheName !== '' ? $cacheName : 'unknown'; + } + $raw = self::replaceTemplateName($e->getMessage(), $cacheName, $label); + $line = self::extractLine($e); + $near = self::extractNear($e->getMessage()); + $excerpt = self::excerpt($content, $line); + $hint = self::modxHint($near . "\n" . $excerpt); + + $lines = [ + '[pdoTools][Fenom] ' . $phase . ' error in ' . $label, + ]; + if (!empty($extra['resource']) && is_array($extra['resource']) && strpos($label, 'resource:') !== 0) { + $resourceLine = self::formatResource($extra['resource']); + if ($resourceLine !== '') { + $lines[] = $resourceLine; + } + } + if ($cacheName !== '' && $cacheName !== $label) { + $lines[] = 'cache name: ' . $cacheName; + } + $lines[] = $raw; + if ($excerpt !== '') { + $lines[] = $excerpt; + } + if ($hint !== '') { + $lines[] = $hint; + } + if (!empty($extra['compiled'])) { + $compiled = self::relativePath((string)$extra['compiled']); + if ($compiled !== '') { + $lines[] = 'compiled: ' . $compiled; + } + } + if (!empty($extra['sourceDump'])) { + $dump = self::relativePath((string)$extra['sourceDump']); + if ($dump !== '') { + $lines[] = 'source dump: ' . $dump; + } + } + + return implode("\n", $lines); + } + + /** + * @param string $binding + * @return string + */ + private static function elementKind($binding) + { + switch (strtolower((string)$binding)) { + case 'modchunk': + return 'chunk'; + case 'modtemplate': + return 'template'; + case 'modsnippet': + return 'snippet'; + default: + return ''; + } + } + + /** + * @param array $source + * @return string + */ + private static function formatResource(array $source) + { + if (!array_key_exists('resourceId', $source)) { + return ''; + } + $id = (int)$source['resourceId']; + $ctx = isset($source['resourceContext']) ? (string)$source['resourceContext'] : ''; + $uri = isset($source['resourceUri']) ? (string)$source['resourceUri'] : ''; + $label = 'resource:#' . $id; + if ($ctx !== '' || $uri !== '') { + $label .= ' (' . $ctx . ':' . $uri . ')'; + } + if (!empty($source['templateId'])) { + $label .= ', template:#' . (int)$source['templateId']; + } + + return $label; + } + + /** + * @param string $value + * @return bool + */ + private static function looksLikeHash($value) + { + return is_string($value) && (bool)preg_match('/^[a-f0-9]{32}$/i', $value); + } + + /** + * @param string $path + * @return string + */ + private static function relativePath($path) + { + if (!is_string($path) || $path === '') { + return ''; + } + $path = str_replace('\\', '/', $path); + if (defined('MODX_CORE_PATH') && MODX_CORE_PATH !== '' && strpos($path, str_replace('\\', '/', MODX_CORE_PATH)) === 0) { + return 'core/' . ltrim(substr($path, strlen(str_replace('\\', '/', MODX_CORE_PATH))), '/'); + } + if (defined('MODX_BASE_PATH') && MODX_BASE_PATH !== '' && strpos($path, str_replace('\\', '/', MODX_BASE_PATH)) === 0) { + return ltrim(substr($path, strlen(str_replace('\\', '/', MODX_BASE_PATH))), '/'); + } + + return $path; + } + + /** + * @param Throwable $e + * @return int + */ + private static function extractLine(Throwable $e) + { + if (preg_match('/\bline\s+(\d+)/i', $e->getMessage(), $m)) { + return (int)$m[1]; + } + + return 0; + } + + /** + * @param string $message + * @return string + */ + private static function extractNear($message) + { + if (!is_string($message) || !preg_match("/near '([^']*)'/s", $message, $m)) { + return ''; + } + + return $m[1]; + } + + /** + * @param string $text + * @return bool + */ + private static function hasUnprocessedModx($text) + { + return is_string($text) && (bool)preg_match('/\[\[(?:\+|\*|\$|%|~|#|&)?/', $text); + } + + /** + * @param string $text + * @return string + */ + private static function modxHint($text) + { + if (!self::hasUnprocessedModx($text)) { + return ''; + } + if (preg_match('/\[\[\+([a-zA-Z0-9._-]+)/', $text, $m)) { + return 'Unprocessed MODX tag inside Fenom. Use {$' . $m[1] . '} or parse MODX before Fenom.'; + } + + return 'Unprocessed MODX tag inside Fenom. Use {$placeholder} or parse MODX before Fenom.'; + } + + /** + * @param string $content + * @param int $line + * @param int $radius + * @return string + */ + private static function excerpt($content, $line, $radius = 2) + { + if (!is_string($content) || $content === '' || $line < 1) { + return ''; + } + $lines = preg_split("/\r\n|\n|\r/", $content); + $index = $line - 1; + if (!isset($lines[$index])) { + return ''; + } + $start = max(0, $index - $radius); + $end = min(count($lines) - 1, $index + $radius); + $out = []; + for ($i = $start; $i <= $end; $i++) { + $mark = ($i === $index) ? '>' : ' '; + $out[] = sprintf('%s %d: %s', $mark, $i + 1, $lines[$i]); + } + + return implode("\n", $out); + } + + /** + * @param string $message + * @param string $name + * @param string $label + * @return string + */ + private static function replaceTemplateName($message, $name, $label) + { + if (!is_string($message) || $name === '' || $label === '' || $name === $label) { + return $message; + } + + return str_replace(' in ' . $name . ' ', ' in ' . $label . ' ', $message); + } +} diff --git a/core/components/pdotools/src/Parsing/Fenom/Fenom.php b/core/components/pdotools/src/Parsing/Fenom/Fenom.php index adb5885..b9bc895 100644 --- a/core/components/pdotools/src/Parsing/Fenom/Fenom.php +++ b/core/components/pdotools/src/Parsing/Fenom/Fenom.php @@ -103,11 +103,12 @@ public function process($chunk, array $properties = []) $name = md5($content); } /** @var \Fenom\Template $tpl */ + $source = is_array($chunk) ? $chunk : []; if (!$tpl = $this->pdoTools->getStore($name, 'fenom')) { if (!empty($this->pdoTools->config('useFenomCache'))) { $compileKey = 'pdotools/' . $name; if (!$cache = $this->pdoTools->getExactCache($compileKey)) { - if ($tpl = $this->_compileChunk($content, $name)) { + if ($tpl = $this->_compileChunk($content, $name, $source)) { $this->pdoTools->setExactCache($compileKey, $tpl->getTemplateCode()); } } else { @@ -115,7 +116,7 @@ public function process($chunk, array $properties = []) $tpl = eval($cache); } } else { - $tpl = $this->_compileChunk($content, $name); + $tpl = $this->_compileChunk($content, $name, $source); } if ($tpl) { $this->pdoTools->setStore($name, $tpl, 'fenom'); @@ -134,8 +135,7 @@ public function process($chunk, array $properties = []) try { $content = $tpl->fetch($properties); } catch (Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, $e->getMessage()); - $this->modx->log(modX::LOG_LEVEL_INFO, $tpl->getTemplateCode()); + $this->logFenomError($e, $name, $content, $source, 'runtime'); } } @@ -173,10 +173,11 @@ public function setCompileDir($dir) * * @param $content * @param string $name + * @param array $source Element/resource facts for error labels * * @return \Fenom\Template */ - protected function _compileChunk($content, $name = '') + protected function _compileChunk($content, $name = '', array $source = []) { if (empty($name)) { $name = md5($content); @@ -185,18 +186,90 @@ protected function _compileChunk($content, $name = '') $tpl = $this->getRawTemplate()->source($name, $content, true); $this->pdoTools->addTime('Compiled Fenom chunk with name "' . $name . '"'); } catch (Exception $e) { - $this->modx->log(modX::LOG_LEVEL_ERROR, $e->getMessage()); - $this->modx->log(modX::LOG_LEVEL_INFO, $content); if ($this->modx->getOption('pdotools_fenom_save_on_errors')) { $this->pdoTools->setExactCache('error/' . $name, $content); } + $this->logFenomError($e, $name, $content, $source, 'compile'); $tpl = $this->getRawTemplate()->source($name, '', false); - $this->pdoTools->addTime('Can`t compile Fenom chunk with name "' . $name . '": ' . $e->getMessage()); } return $tpl; } + /** + * @param Exception $e + * @param string $name + * @param string $content + * @param array $source + * @param string $phase + */ + protected function logFenomError(Exception $e, $name, $content, array $source, $phase) + { + $label = ErrorLog::label($source, $name); + $extra = [ + 'resource' => $this->currentResourceFacts(), + ]; + $compiled = $this->compiledPathIfExists($name); + if ($compiled !== '') { + $extra['compiled'] = $compiled; + } + if ($phase === 'compile' && $this->modx->getOption('pdotools_fenom_save_on_errors')) { + $extra['sourceDump'] = rtrim((string)$this->pdoTools->config('cachePath'), '/') . '/error/' . $name; + } + $message = ErrorLog::format($e, $name, $content, $label, $phase, $extra); + $this->modx->log(modX::LOG_LEVEL_ERROR, $message); + $this->pdoTools->addTime($message); + } + + /** + * Current page facts for a secondary log line (no template id). + * + * @return array + */ + protected function currentResourceFacts() + { + $resource = $this->modx->resource; + if (!is_object($resource) || !method_exists($resource, 'get')) { + return []; + } + $id = (int)$resource->get('id'); + if ($id <= 0) { + return []; + } + $uri = (string)$resource->get('uri'); + if ($uri === '') { + $uri = (string)$resource->get('alias'); + } + $facts = [ + 'resourceId' => $id, + 'resourceUri' => $uri, + ]; + if (is_object($this->modx->context) && method_exists($this->modx->context, 'get')) { + $facts['resourceContext'] = (string)$this->modx->context->get('key'); + } + + return $facts; + } + + /** + * Absolute path to a compiled template file, if it exists. + * + * @param string $name + * @return string + */ + protected function compiledPathIfExists($name) + { + if ($name === '' || empty($this->_compile_dir)) { + return ''; + } + $file = rtrim((string)$this->_compile_dir, '/\\') . '/' . $this->getCompileName($name); + if (!is_file($file)) { + return ''; + } + + return $file; + } + /** * Add default modifiers diff --git a/core/components/pdotools/src/Parsing/Parser.php b/core/components/pdotools/src/Parsing/Parser.php index e4422af..d2346a3 100644 --- a/core/components/pdotools/src/Parsing/Parser.php +++ b/core/components/pdotools/src/Parsing/Parser.php @@ -66,7 +66,10 @@ public function processElementTags( } $_processingUncacheable = $this->_processingUncacheable; $this->_processingUncacheable = true; - $content = $this->pdoTools->getFenom()->process($content, $this->modx->placeholders); + $content = $this->pdoTools->getFenom()->process( + $this->fenomSourcePayload($content), + $this->modx->placeholders + ); $this->_processingUncacheable = $_processingUncacheable; } @@ -279,4 +282,31 @@ public function processTag($tag, $processUncacheable = true) return $output; } + /** + * Facts for Fenom page-parser error labels. Not a cache key. + * + * @param string $content + * @return array + */ + protected function fenomSourcePayload($content) + { + $payload = ['content' => $content]; + $resource = $this->modx->resource; + if (!is_object($resource) || !method_exists($resource, 'get')) { + return $payload; + } + $payload['resourceId'] = (int)$resource->get('id'); + $uri = (string)$resource->get('uri'); + if ($uri === '') { + $uri = (string)$resource->get('alias'); + } + $payload['resourceUri'] = $uri; + $payload['templateId'] = (int)$resource->get('template'); + if (is_object($this->modx->context) && method_exists($this->modx->context, 'get')) { + $payload['resourceContext'] = (string)$this->modx->context->get('key'); + } + + return $payload; + } + } diff --git a/core/components/pdotools/tests/Stubs/ModxStub.php b/core/components/pdotools/tests/Stubs/ModxStub.php index 10ceba1..1f59a81 100644 --- a/core/components/pdotools/tests/Stubs/ModxStub.php +++ b/core/components/pdotools/tests/Stubs/ModxStub.php @@ -51,6 +51,11 @@ public function offsetUnset($offset): void if (!class_exists(modX::class, false)) { class modX { + public const LOG_LEVEL_ERROR = 0; + public const LOG_LEVEL_WARN = 1; + public const LOG_LEVEL_INFO = 2; + public const LOG_LEVEL_DEBUG = 3; + public $user; public $context; public $resource; @@ -92,6 +97,12 @@ public function getOption($key, $options = null, $default = null, $skipEmpty = f return $value; } } + if (array_key_exists($key, $this->config)) { + $value = $this->config[$key]; + if (!$skipEmpty || ($value !== '' && $value !== null)) { + return $value; + } + } return $default; } diff --git a/core/components/pdotools/tests/Unit/Parsing/ErrorLogTest.php b/core/components/pdotools/tests/Unit/Parsing/ErrorLogTest.php index 3d438a9..75495c3 100644 --- a/core/components/pdotools/tests/Unit/Parsing/ErrorLogTest.php +++ b/core/components/pdotools/tests/Unit/Parsing/ErrorLogTest.php @@ -4,43 +4,138 @@ namespace ModxPro\PdoTools\Tests\Unit\Parsing; +use Exception; use ModxPro\PdoTools\Parsing\Fenom\ErrorLog; use PHPUnit\Framework\TestCase; class ErrorLogTest extends TestCase { - protected function setUp(): void + public function testLabelForNamedChunk(): void { - parent::setUp(); - if (!class_exists(ErrorLog::class)) { - $this->markTestSkipped('ErrorLog is not on this branch yet (pdoTools3#21).'); - } + $this->assertSame( + 'chunk:tpl.product.row (#12)', + ErrorLog::label([ + 'binding' => 'modchunk', + 'id' => 12, + 'elementName' => 'tpl.product.row', + ], 'modchunk/12') + ); } - public function testLooksLikeHash(): void + public function testLabelForFileAndInlineOrigins(): void { - $this->assertTrue(ErrorLog::looksLikeHash(md5('chunk'))); - $this->assertFalse(ErrorLog::looksLikeHash('my-chunk')); + $file = rtrim(str_replace('\\', '/', MODX_CORE_PATH), '/') . '/elements/chunks/item.tpl'; + $this->assertSame( + 'file:core/elements/chunks/item.tpl', + ErrorLog::label([ + 'binding' => 'modchunk', + 'origin' => 'FILE', + 'sourceFile' => $file, + ], 'modchunk/' . md5('file')) + ); + $this->assertSame( + 'inline', + ErrorLog::label(['binding' => 'modchunk', 'origin' => 'INLINE'], 'inline') + ); } - public function testExcerptMarksTheLine(): void + public function testLabelFallsBackToFileWithoutOrigin(): void { - $content = "one\ntwo\n{var \$x = [[+limit]]}\nfour"; - $excerpt = ErrorLog::excerpt($content, 3, 1); + $file = rtrim(str_replace('\\', '/', MODX_CORE_PATH), '/') . '/elements/chunks/item.tpl'; + $this->assertSame( + 'file:core/elements/chunks/item.tpl', + ErrorLog::label([ + 'binding' => 'modchunk', + 'sourceFile' => $file, + ], 'modchunk/' . md5('file')) + ); + } + + public function testLabelForResourceWithTemplate(): void + { + $this->assertSame( + 'resource:#42 (web:catalog/item), template:#5', + ErrorLog::label([ + 'resourceId' => 42, + 'resourceContext' => 'web', + 'resourceUri' => 'catalog/item', + 'templateId' => 5, + ], md5('page')) + ); + } - $this->assertStringContainsString('>', $excerpt); - $this->assertStringContainsString('[[+limit]]', $excerpt); + public function testFormatReplacesHashAndAddsHint(): void + { + $hash = 'ee058690d9fd7413748b95b0960e006b'; + $content = "one\ntwo\nthree\nfour\nfive\n{var \$limit = [[+limit]]}\nseven"; + $e = new Exception( + "Unexpected token '+' in expression in {$hash} line 6, near '{var \$limit = [[+' <- there" + ); + $message = ErrorLog::format( + $e, + $hash, + $content, + 'chunk:tpl.product.row (#12)', + 'compile' + ); + + $this->assertStringContainsString('[pdoTools][Fenom] compile error in chunk:tpl.product.row (#12)', $message); + $this->assertStringContainsString('cache name: ' . $hash, $message); + $this->assertStringContainsString('chunk:tpl.product.row (#12) line 6', $message); + $this->assertStringNotContainsString(' in ' . $hash . ' ', $message); + $this->assertStringContainsString('> 6:', $message); + $this->assertStringContainsString('{$limit}', $message); } - public function testModxHintNamesThePlaceholder(): void + public function testFormatAddsResourceLineAndSourceDump(): void { - $hint = ErrorLog::modxHint('{var $limit = [[+limit]]}'); - $this->assertStringContainsString('{$limit}', $hint); + $hash = md5('broken'); + $e = new Exception("Unexpected token '+' in expression in {$hash} line 1, near '{var'"); + $dump = rtrim(str_replace('\\', '/', MODX_CORE_PATH), '/') . '/cache/pdotools/error/' . $hash; + $message = ErrorLog::format( + $e, + $hash, + '{var $limit = [[+limit]]}', + 'inline', + 'compile', + [ + 'resource' => [ + 'resourceId' => 42, + 'resourceContext' => 'web', + 'resourceUri' => 'catalog/item', + ], + 'sourceDump' => $dump, + ] + ); + + $this->assertStringContainsString('resource:#42 (web:catalog/item)', $message); + $this->assertStringContainsString('source dump: core/cache/pdotools/error/' . $hash, $message); + $this->assertStringContainsString('{$limit}', $message); } - public function testHasUnprocessedModx(): void + public function testFormatSkipsResourceLineWhenLabelIsResource(): void { - $this->assertTrue(ErrorLog::hasUnprocessedModx('[[+limit]]')); - $this->assertFalse(ErrorLog::hasUnprocessedModx('{$limit}')); + $e = new Exception('syntax error near token'); + $message = ErrorLog::format( + $e, + md5('x'), + '{var $x = 1}', + 'resource:#1 (web:home), template:#2', + 'compile', + [ + 'resource' => [ + 'resourceId' => 1, + 'resourceContext' => 'web', + 'resourceUri' => 'home', + ], + ] + ); + + $lines = explode("\n", $message); + $resourceLines = array_filter($lines, static function ($line) { + return strpos($line, 'resource:#') === 0; + }); + $this->assertCount(0, $resourceLines); + $this->assertStringContainsString('compile error in resource:#1 (web:home), template:#2', $message); } } diff --git a/core/components/pdotools/tests/Unit/Parsing/FenomErrorLogTest.php b/core/components/pdotools/tests/Unit/Parsing/FenomErrorLogTest.php new file mode 100644 index 0000000..81ae1d8 --- /dev/null +++ b/core/components/pdotools/tests/Unit/Parsing/FenomErrorLogTest.php @@ -0,0 +1,63 @@ + '{var $limit = [[+limit]]}', + 'binding' => 'modchunk', + 'origin' => '', + 'id' => 12, + 'name' => 'tpl.product.row', + 'elementName' => 'tpl.product.row', + ]; + $this->fenom()->process($chunk); + + $this->assertNotEmpty($this->modx->logs); + $logged = (string)$this->modx->logs[0]['message']; + $this->assertStringContainsString('chunk:tpl.product.row (#12)', $logged); + $this->assertStringContainsString('cache name: modchunk/12', $logged); + $this->assertStringContainsString('{$limit}', $logged); + $this->assertNotNull($this->pdoTools->getStore('modchunk/12', 'fenom')); + } + + public function testProcessFileOriginUsesCanonicalFileLabel(): void + { + $file = rtrim(str_replace('\\', '/', MODX_CORE_PATH), '/') . '/elements/chunks/item.tpl'; + $this->fenom()->process([ + 'content' => '{var $limit = [[+limit]]}', + 'binding' => 'modchunk', + 'origin' => 'FILE', + 'sourceFile' => $file, + 'name' => md5('@FILE item'), + ]); + + $logged = (string)$this->modx->logs[0]['message']; + $this->assertStringContainsString('compile error in file:core/elements/chunks/item.tpl', $logged); + $this->assertStringNotContainsString('chunk file:', $logged); + } + + public function testSaveOnErrorsListsSourceDump(): void + { + $this->modx->config['pdotools_fenom_save_on_errors'] = true; + $this->modx->cacheManager = $this->modx->getCacheManager(); + $this->fenom()->process([ + 'content' => '{var $limit = [[+limit]]}', + 'binding' => 'modchunk', + 'origin' => 'INLINE', + 'name' => 'inline-broken', + ]); + + $logged = (string)$this->modx->logs[0]['message']; + $this->assertStringContainsString('compile error in inline', $logged); + $this->assertStringContainsString('source dump:', $logged); + $this->assertStringContainsString('error/', $logged); + } +}