Reject synchronous batch callback failures

src/DataLoader.php

@@ -91,6 +91,11 @@ function () {
             'promise' => $promise,
         ];
 
+        // If caching, cache this promise before it may be dispatched.
+        if ($shouldCache) {
+            $this->promiseCache->set($cacheKey, $promise);
+        }
+
         // Determine if a dispatch of this queue should be scheduled.
         // A single dispatch should be scheduled per queue at the time when the
         // queue changes from "empty" to "full".
@@ -100,10 +105,6 @@ function () {
                 $this->dispatchQueue();
             }
         }
-        // If caching, cache this promise.
-        if ($shouldCache) {
-            $this->promiseCache->set($cacheKey, $promise);
-        }
 
         return $promise;
     }
@@ -331,7 +332,13 @@ private function dispatchQueueBatch(array $queue)
 
         // Call the provided batchLoadFn for this loader with the loader queue's keys.
         $batchLoadFn = $this->batchLoadFn;
-        $batchPromise = $batchLoadFn($keys);
+        try {
+            $batchPromise = $batchLoadFn($keys);
+        } catch (\Throwable $error) {
+            $this->failedDispatch($queue, $error);
+
+            return;
+        }
 
         // Assert the expected response from batchLoadFn
         if (!$batchPromise || !is_callable([$batchPromise, 'then'])) {

tests/DataLoadTestCase.php

@@ -541,6 +541,57 @@ public function testPropagatesErrorFromFailedBatchToAllLoads()
         $this->assertEquals([[1, 2]], $loadCalls->getArrayCopy());
     }
 
+    /**
+     * @dataProvider provideSynchronousBatchThrowable
+     */
+    public function testPropagatesSynchronousBatchThrowableToAllLoadsAndAllowsRetry(\Throwable $throwable)
+    {
+        $attempt = 0;
+        list($loader, $loadCalls) = self::idLoader(null, function ($keys) use (&$attempt, $throwable) {
+            if (0 === $attempt++) {
+                throw $throwable;
+            }
+
+            return self::$promiseAdapter->createFulfilled($keys);
+        });
+
+        $promise1 = $loader->load(1);
+        $promise2 = $loader->load(2);
+
+        $this->assertSame($throwable, DataLoader::await($promise1, false));
+        $this->assertSame($throwable, DataLoader::await($promise2, false));
+
+        $this->assertEquals(
+            [1, 2],
+            DataLoader::await(self::$promiseAdapter->createAll([$loader->load(1), $loader->load(2)]))
+        );
+        $this->assertEquals([[1, 2], [1, 2]], $loadCalls->getArrayCopy());
+    }
+
+    public function testDoesNotCacheSynchronousBatchFailureWhenBatchingIsDisabled()
+    {
+        $attempt = 0;
+        list($loader, $loadCalls) = self::idLoader(new Option(['batch' => false]), function ($keys) use (&$attempt) {
+            if (0 === $attempt++) {
+                throw new \Exception('Synchronous batch exception');
+            }
+
+            return self::$promiseAdapter->createFulfilled($keys);
+        });
+
+        $this->assertInstanceOf(\Exception::class, DataLoader::await($loader->load(1), false));
+        $this->assertEquals(1, DataLoader::await($loader->load(1)));
+        $this->assertEquals([[1], [1]], $loadCalls->getArrayCopy());
+    }
+
+    public static function provideSynchronousBatchThrowable()
+    {
+        return [
+            'exception' => [new \Exception('Synchronous batch exception')],
+            'error' => [new \Error('Synchronous batch error')],
+        ];
+    }
+
     /**
      * @group accepts-any-kind-of-key
      */