FIX Don't swallow relevant BadMethodCallExceptions from ViewLayerData (…

…#11482)

src/Model/ModelDataCustomised.php

@@ -24,10 +24,10 @@ public function __construct(ModelData $originalObject, ModelData $customisedObje
     public function __call($method, $arguments)
     {
         if ($this->customised->hasMethod($method)) {
-            return call_user_func_array([$this->customised, $method], $arguments ?? []);
+            return $this->customised->$method(...$arguments);
         }
 
-        return call_user_func_array([$this->original, $method], $arguments ?? []);
+        return $this->original->$method(...$arguments);
     }
 
     public function __get(string $property): mixed

src/View/ViewLayerData.php

@@ -244,12 +244,41 @@ private function callDataMethod(object $data, string $name, array $arguments, bo
             } catch (BadMethodCallException $e) {
                 // Only throw the exception if we weren't relying on __call
                 // It's common for __call to throw BadMethodCallException for methods that aren't "implemented"
-                // so we just want to return null in those cases.
-                if (!$hasDynamicMethods) {
+                // so we just want to return null in those cases - but only if it's a direct result of our method call.
+                if (!$hasDynamicMethods || $this->mustThrow($e->getTrace())) {
                     throw $e;
                 }
             }
         }
         return null;
     }
+
+    private function mustThrow(array $trace): bool
+    {
+        $dataClass = get_class($this->data);
+        foreach ($trace as $data) {
+            $class = $data['class'] ?? '';
+            $method = $data['function'] ?? '';
+            if ($class === ViewLayerData::class) {
+                // If we hit ViewLayerData::callDataMethod() we've finished checking the relevant parts of the stack
+                if ($method === 'callDataMethod') {
+                    break;
+                }
+                // If we're trying to call some other method on ViewLayerData and it causes problems, throw the exception.
+                return true;
+            }
+            // If we find a non __call method return true
+            // This means our method exists, but it tried to call something else which doesn't
+            if ($method !== '__call') {
+                return true;
+            }
+            // If we break class inheritance return true
+            if (!is_a($class, $dataClass, true) && !is_a($dataClass, $class, true)) {
+                return true;
+            }
+        }
+        // Hitting this means `callDataMethod()` only hit `__call()` methods inside the class hierarchy of our data, which
+        // means the method we were actually trying to call is missing and we can safely ignore the exception.
+        return false;
+    }
 }

tests/php/View/ViewLayerDataTest.php

@@ -5,7 +5,6 @@
 use ArrayIterator;
 use BadMethodCallException;
 use Error;
-use InvalidArgumentException;
 use PHPUnit\Framework\Attributes\DataProvider;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Model\ArrayData;
@@ -20,6 +19,7 @@
 use SilverStripe\View\Tests\ViewLayerDataTest\GetCountObject;
 use SilverStripe\View\Tests\ViewLayerDataTest\NonIterableObject;
 use SilverStripe\View\Tests\ViewLayerDataTest\StringableObject;
+use SilverStripe\View\Tests\ViewLayerDataTest\TestBadMethodCallObjectSubclass;
 use SilverStripe\View\Tests\ViewLayerDataTest\TestFixture;
 use SilverStripe\View\Tests\ViewLayerDataTest\TestFixtureComplex;
 use SilverStripe\View\ViewLayerData;
@@ -707,4 +707,39 @@ public function testSpecialNames(): void
         $this->assertSame('some other class', $viewLayerData->getRawDataValue('ClassName'));
         $this->assertSame('something else', $viewLayerData->getRawDataValue('Me'));
     }
+
+    public static function provideCallDataMethodExceptionCatching(): array
+    {
+        return [
+            'exception thrown directly in __call()' => [
+                'method' => 'directMethod',
+                'exceptionCaught' => true,
+            ],
+            'exception thrown in __call() on superclass' => [
+                'method' => 'inheritedMethod',
+                'exceptionCaught' => true,
+            ],
+            'exception thrown outside __call()' => [
+                'method' => 'realMethod',
+                'exceptionCaught' => false,
+            ],
+            'exception thrown in __call() in a different class hierarchy' => [
+                'method' => 'anotherClass',
+                'exceptionCaught' => false,
+            ],
+        ];
+    }
+
+    #[DataProvider('provideCallDataMethodExceptionCatching')]
+    public function testCallDataMethodExceptionCatching(string $method, bool $exceptionCaught): void
+    {
+        $data = new TestBadMethodCallObjectSubclass();
+        $viewLayerData = new ViewLayerData($data);
+        if (!$exceptionCaught) {
+            $this->expectException(BadMethodCallException::class);
+        }
+
+        $result = $viewLayerData->$method();
+        $this->assertNull($result);
+    }
 }

tests/php/View/ViewLayerDataTest/TestBadMethodCallObject.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\View\Tests\ViewLayerDataTest;
+
+use BadMethodCallException;
+use SilverStripe\Dev\TestOnly;
+
+class TestBadMethodCallObject implements TestOnly
+{
+    public function __call(string $name, array $arguments): void
+    {
+        throw new BadMethodCallException('This exception should be caught by ViewLayerData');
+    }
+}

tests/php/View/ViewLayerDataTest/TestBadMethodCallObjectSubclass.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace SilverStripe\View\Tests\ViewLayerDataTest;
+
+use BadMethodCallException;
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\Model\ModelData;
+
+class TestBadMethodCallObjectSubclass extends TestBadMethodCallObject implements TestOnly
+{
+    public function __call(string $name, array $arguments): void
+    {
+        if ($name === 'directMethod') {
+            throw new BadMethodCallException('This exception should be caught by ViewLayerData');
+        }
+        if ($name === 'realMethod') {
+            $this->throwException();
+        }
+        if ($name === 'anotherClass') {
+            $data = new ModelData();
+            $data->thisMethodDoesntExist();
+        }
+        parent::__call($name, $arguments);
+    }
+
+    public function throwException(): void
+    {
+        throw new BadMethodCallException('This exception should NOT be caught by ViewLayerData');
+    }
+}