TASK: Improve errors for edge-cases in configuration

Flowpack · Oct 18, 2023 · 470c0cf · 470c0cf
1 parent fa2e283
commit 470c0cf
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 17 deletions.
diff --git a/Classes/Domain/TemplateConfiguration/TemplateConfigurationProcessor.php b/Classes/Domain/TemplateConfiguration/TemplateConfigurationProcessor.php
@@ -2,7 +2,6 @@
 
 namespace Flowpack\NodeTemplates\Domain\TemplateConfiguration;
 
-use Flowpack\NodeTemplates\Domain\ErrorHandling\ProcessingError;
 use Flowpack\NodeTemplates\Domain\ErrorHandling\ProcessingErrors;
 use Flowpack\NodeTemplates\Domain\Template\RootTemplate;
 use Flowpack\NodeTemplates\Domain\Template\Template;
@@ -49,6 +48,13 @@ private function createTemplatesFromTemplatePart(TemplatePart $templatePart): Te
         try {
             $withContext = [];
             foreach ($templatePart->getRawConfiguration('withContext') ?? [] as $key => $_) {
+                if (!is_string($key)) {
+                    $templatePart->addProcessingErrorForPath(
+                        new \RuntimeException(sprintf('Key must be string. Got %s.', gettype($key)), 1697663846),
+                        ['withContext', $key]
+                    );
+                    continue;
+                }
                 $withContext[$key] = $templatePart->processConfiguration(['withContext', $key]);
             }
             $templatePart = $templatePart->withMergedEvaluationContext($withContext);
@@ -63,10 +69,9 @@ private function createTemplatesFromTemplatePart(TemplatePart $templatePart): Te
             $items = $templatePart->processConfiguration('withItems');
 
             if (!is_iterable($items)) {
-                $templatePart->getProcessingErrors()->add(
-                    ProcessingError::fromException(
-                        new \RuntimeException(sprintf('Type %s is not iterable.', gettype($items)), 1685802354186)
-                    )->withOrigin(sprintf('Configuration "%s" in "%s"', json_encode($templatePart->getRawConfiguration('withItems')), join('.', array_merge($templatePart->getFullPathToConfiguration(), ['withItems']))))
+                $templatePart->addProcessingErrorForPath(
+                    new \RuntimeException(sprintf('Type %s is not iterable.', gettype($items)), 1685802354186),
+                    'withItems'
                 );
                 return Templates::empty();
             }
@@ -95,9 +100,17 @@ private function createTemplateFromTemplatePart(TemplatePart $templatePart): Tem
         $processedProperties = [];
         foreach ($templatePart->getRawConfiguration('properties') ?? [] as $propertyName => $value) {
             if (!is_scalar($value) && !is_null($value)) {
-                $templatePart->getProcessingErrors()->add(ProcessingError::fromException(
-                    new \RuntimeException(sprintf('Template configuration properties can only hold int|float|string|bool|null. Property "%s" has type "%s"', $propertyName, gettype($value)), 1685725310730)
-                ));
+                $templatePart->addProcessingErrorForPath(
+                    new \RuntimeException(sprintf('Template configuration properties can only hold int|float|string|bool|null. Property "%s" has type "%s"', $propertyName, gettype($value)), 1685725310730),
+                    ['properties', $propertyName]
+                );
+                continue;
+            }
+            if (!is_string($propertyName)) {
+                $templatePart->addProcessingErrorForPath(
+                    new \RuntimeException(sprintf('Template configuration property names must be of type string. Got: %s', gettype($propertyName)), 1697663340),
+                    ['properties', $propertyName]
+                );
                 continue;
             }
             try {

diff --git a/Classes/Domain/TemplateConfiguration/TemplatePart.php b/Classes/Domain/TemplateConfiguration/TemplatePart.php
@@ -81,9 +81,19 @@ public static function createRoot(
         );
     }
 
-    public function getProcessingErrors(): ProcessingErrors
+    /**
+     * @param string|list<string|int> $configurationPath
+     */
+    public function addProcessingErrorForPath(\Throwable $throwable, $configurationPath): void
     {
-        return $this->processingErrors;
+        $this->processingErrors->add(
+            ProcessingError::fromException(
+                $throwable
+            )->withOrigin(sprintf(
+                'Configuration "%s"',
+                join('.', array_merge($this->getFullPathToConfiguration(), is_array($configurationPath) ? $configurationPath : [$configurationPath]))
+            ))
+        );
     }
 
     public function getFullPathToConfiguration(): array
@@ -201,15 +211,17 @@ private function validateTemplateConfigurationKeys(): void
         $isRootTemplate = $this->fullPathToConfiguration === [];
         foreach (array_keys($this->configuration) as $key) {
             if (!in_array($key, ['type', 'name', 'properties', 'childNodes', 'when', 'withItems', 'withContext'], true)) {
-                $this->processingErrors->add(
-                    ProcessingError::fromException(new \InvalidArgumentException(sprintf('Template configuration has illegal key "%s"', $key), 1686150349274))
+                $this->addProcessingErrorForPath(
+                    new \InvalidArgumentException(sprintf('Template configuration has illegal key "%s"', $key), 1686150349274),
+                    $key
                 );
                 throw new StopBuildingTemplatePartException();
             }
             if ($isRootTemplate) {
                 if (!in_array($key, ['properties', 'childNodes', 'when', 'withContext'], true)) {
-                    $this->processingErrors->add(
-                        ProcessingError::fromException(new \InvalidArgumentException(sprintf('Root template configuration doesnt allow option "%s', $key), 1686150340657))
+                    $this->addProcessingErrorForPath(
+                        new \InvalidArgumentException(sprintf('Root template configuration doesnt allow option "%s', $key), 1686150340657),
+                        $key
                     );
                     throw new StopBuildingTemplatePartException();
                 }

diff --git a/Tests/Functional/Features/Exceptions/Snapshots/SomeExceptions.messages.json b/Tests/Functional/Features/Exceptions/Snapshots/SomeExceptions.messages.json
@@ -12,7 +12,7 @@
         "severity": "ERROR"
     },
     {
-        "message": "RuntimeException(Template configuration properties can only hold int|float|string|bool|null. Property \"nonEelArrayNotAllowed\" has type \"array\", 1685725310730)",
+        "message": "Configuration \"properties.nonEelArrayNotAllowed\" | RuntimeException(Template configuration properties can only hold int|float|string|bool|null. Property \"nonEelArrayNotAllowed\" has type \"array\", 1685725310730)",
         "severity": "ERROR"
     },
     {
@@ -44,11 +44,11 @@
         "severity": "ERROR"
     },
     {
-        "message": "Configuration \"null\" in \"childNodes.withItemsAbortBecauseNotIterable.withItems\" | RuntimeException(Type NULL is not iterable., 1685802354186)",
+        "message": "Configuration \"childNodes.withItemsAbortBecauseNotIterable.withItems\" | RuntimeException(Type NULL is not iterable., 1685802354186)",
         "severity": "ERROR"
     },
     {
-        "message": "InvalidArgumentException(Template configuration has illegal key \"crazy\", 1686150349274)",
+        "message": "Configuration \"childNodes.invalidOption.crazy\" | InvalidArgumentException(Template configuration has illegal key \"crazy\", 1686150349274)",
         "severity": "ERROR"
     },
     {