Fixed iteration tests (#303)

Author: p123-stack

src/Bolt/BoltConnection.php

@@ -336,13 +336,12 @@ public function protocol(): V4_4|V5|V5_1|V5_2|V5_3|V5_4
      * Pulls a result set.
      *
      * Any of the preconditioned states are: 'TX_READY', 'INTERRUPTED'.
-     *
-     * @return non-empty-list<list>
      */
-    public function pull(?int $qid, ?int $fetchSize): array
+    public function pull(?int $qid, ?int $fetchSize): PullResult
     {
         $extra = $this->buildResultExtra($fetchSize, $qid);
 
+        /** @var list<array<array-key, mixed>> $tbr */
         $tbr = [];
         $message = $this->messageFactory->createPullMessage($extra);
 
@@ -357,25 +356,39 @@ public function pull(?int $qid, ?int $fetchSize): array
             }
 
             foreach ($message->send()->getResponses() as $response) {
-                $this->assertNoFailure($response);
+                if ($response->signature === Signature::FAILURE) {
+                    $this->logger?->log(LogLevel::ERROR, 'FAILURE', $response->content);
+                    $this->subscribedResults = [];
+                    $failure = Neo4jException::fromBoltResponse($response);
+                    if ($tbr !== []) {
+                        $this->reset();
+
+                        return PullResult::withDeferredFailure($tbr, $failure);
+                    }
+
+                    throw $failure;
+                }
+
                 $tbr[] = $response->content;
             }
 
             $this->restoreOriginalTimeout();
 
-            /** @var non-empty-list<list> */
-            return $tbr;
+            if ($tbr === []) {
+                throw new Exception('PULL returned no responses');
+            }
+
+            return PullResult::complete($tbr);
         } catch (Throwable $e) {
             $this->restoreOriginalTimeout();
             // If we've received some records before the disconnect, return them so first next() succeeds.
             // Second next() must pull again and fail with a connection error (TestKit exit_after_record scripts).
             // Do not append []: BoltResult treats trailing empty SUCCESS as stream completion, so the iterator
             // would stop cleanly instead of surfacing the disconnect. A synthetic has_more:true means "not done".
-            if (!empty($tbr)) {
+            if ($tbr !== []) {
                 $tbr[] = ['has_more' => true];
 
-                /** @var non-empty-list<list> */
-                return $tbr;
+                return PullResult::complete($tbr);
             }
             throw $e;
         }

src/Bolt/BoltResult.php

@@ -14,32 +14,30 @@
 namespace Laudis\Neo4j\Bolt;
 
 use function array_key_exists;
-use function array_splice;
 
 use Bolt\error\BoltException;
 use Bolt\error\ConnectException as BoltConnectException;
-
-use function count;
-
 use Generator;
 
 use function in_array;
 use function is_array;
 
 use Iterator;
+use Laudis\Neo4j\Exception\Neo4jException;
 use Laudis\Neo4j\Formatter\SummarizedResultFormatter;
 use Throwable;
 
 /**
  * @psalm-import-type BoltCypherStats from SummarizedResultFormatter
  *
- * @implements Iterator<int, list<mixed>>
+ * @implements Iterator<int, array<array-key, mixed>>
  */
 final class BoltResult implements Iterator
 {
-    /** @var list<list> */
+    /** @var list<array<array-key, mixed>> */
     private array $rows = [];
     private ?array $meta = null;
+    private ?Neo4jException $deferredFailure = null;
     /** @var list<(callable(array):void)> */
     private array $finishedCallbacks = [];
 
@@ -92,7 +90,7 @@ public function addFinishedCallback(callable $finishedCallback): void
     }
 
     /**
-     * @return Generator<int, list>
+     * @return Generator<int, array<array-key, mixed>>
      */
     public function getIt(): Generator
     {
@@ -104,7 +102,7 @@ public function getIt(): Generator
     }
 
     /**
-     * @return Generator<int, list<mixed>>
+     * @return Generator<int, array<array-key, mixed>>
      */
     public function iterator(): Generator
     {
@@ -143,24 +141,32 @@ public function consume(): array
 
     private function fetchResults(): void
     {
+        if ($this->deferredFailure !== null) {
+            throw $this->deferredFailure;
+        }
+
         $this->networkPullOccurred = true;
 
         try {
-            $meta = $this->connection->pull($this->qid, $this->effectivePullSize());
+            $pullResult = $this->connection->pull($this->qid, $this->effectivePullSize());
         } catch (BoltConnectException|BoltException $e) {
             // Invalidate connection on socket/network errors so pool does not reuse it.
             // Rethrow as-is - Session retry logic inspects the actual exception via isConnectionError().
             $this->connection->invalidate();
             throw $e;
         }
         // Neo4jException and other Throwable propagate naturally - no invalidate needed for server errors
-        // $meta is non-empty: {@see BoltConnection::pull()} is contractually non-empty-list<list>.
 
-        /** @var list<list> $rows */
-        $rows = array_splice($meta, 0, count($meta) - 1);
-        $this->rows = $rows;
+        $this->rows = $pullResult->getRecordRows();
+
+        $deferredFailure = $pullResult->getDeferredFailure();
+        if ($deferredFailure !== null) {
+            $this->deferredFailure = $deferredFailure;
+
+            return;
+        }
 
-        $summarySlot = $meta[0] ?? null;
+        $summarySlot = $pullResult->getSummary();
         if (!is_array($summarySlot)) {
             // No summary received (connection closed before summary)
             $this->meta = null;
@@ -169,7 +175,7 @@ private function fetchResults(): void
         }
 
         $summaryEmpty = $summarySlot === [];
-        $hasDataRows = $rows !== [];
+        $hasDataRows = $this->rows !== [];
 
         if ($summaryEmpty && !$hasDataRows) {
             // Normal completion with no records
@@ -188,7 +194,7 @@ private function fetchResults(): void
     /**
      * @psalm-suppress InvalidNullableReturnType
      *
-     * @return list<mixed>
+     * @return array<array-key, mixed>
      */
     public function current(): array
     {

src/Bolt/PullResult.php

@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Neo4j PHP Client and Driver package.
+ *
+ * (c) Nagels <https://nagels.tech>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Laudis\Neo4j\Bolt;
+
+use Laudis\Neo4j\Exception\Neo4jException;
+
+/**
+ * Internal result of a Bolt PULL. May carry a deferred server failure when
+ * RECORD rows were received before a Bolt FAILURE in the same pull response.
+ *
+ * @internal
+ */
+final class PullResult
+{
+    /**
+     * @param list<array<array-key, mixed>> $recordRows
+     * @param array<string, mixed>|null     $summary
+     */
+    private function __construct(
+        private readonly array $recordRows,
+        private readonly ?array $summary,
+        private readonly ?Neo4jException $deferredFailure,
+    ) {
+    }
+
+    /**
+     * @param non-empty-list<array<array-key, mixed>> $content record rows followed by a summary map
+     */
+    public static function complete(array $content): self
+    {
+        /** @var array<string, mixed> $summary */
+        $summary = array_pop($content);
+
+        return new self($content, $summary, null);
+    }
+
+    /**
+     * @param non-empty-list<array<array-key, mixed>> $bufferedRows
+     */
+    public static function withDeferredFailure(array $bufferedRows, Neo4jException $failure): self
+    {
+        return new self($bufferedRows, null, $failure);
+    }
+
+    /**
+     * @return list<array<array-key, mixed>>
+     */
+    public function getRecordRows(): array
+    {
+        return $this->recordRows;
+    }
+
+    /**
+     * @return array<string, mixed>|null
+     */
+    public function getSummary(): ?array
+    {
+        return $this->summary;
+    }
+
+    public function getDeferredFailure(): ?Neo4jException
+    {
+        return $this->deferredFailure;
+    }
+}

testkit-backend/features.php

@@ -99,7 +99,7 @@
     // notified when the server reports a token expired.
     'Feature:Auth:Managed' => false,
     // The driver supports Bolt protocol version 3
-    'Feature:Bolt:3.0' => true,
+    'Feature:Bolt:3.0' => false,
     // The driver supports Bolt protocol version 4.1
     'Feature:Bolt:4.1' => true,
     // The driver supports Bolt protocol version 4.2

testkit-backend/testkit.sh

@@ -43,6 +43,7 @@ echo ""
 
 ### Failing/error tests
 #python3 -m unittest -vvv \
+#       tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_error
 #      tests.stub.datatypes.test_vector_types.TestVectorTypes.test_vector \
 #      tests.stub.datatypes.test_unsupported_type.TestUnsupportedTypes.test_unsupported_type \
 #      tests.stub.datatypes.test_temporal_types.TestTemporalTypesV4x4.test_date_time_with_patch \
@@ -71,7 +72,7 @@ echo ""
 #
 # Passing tests (commented out)
 # python3 -m unittest -vvv \
-
+#
 python3 -m unittest -vvv \
      tests.neo4j.test_authentication.TestAuthenticationBasic.test_error_on_incorrect_credentials \
      tests.neo4j.test_authentication.TestAuthenticationBasic.test_success_on_basic_token \
@@ -179,13 +180,20 @@ python3 -m unittest -vvv \
      tests.stub.iteration.test_result_peek.TestResultPeek.test_result_peek_with_0_records \
      tests.stub.iteration.test_result_peek.TestResultPeek.test_result_peek_with_1_records \
      tests.stub.iteration.test_result_peek.TestResultPeek.test_result_peek_with_2_records \
+     tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_error \
+     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_batch \
+     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_all \
+     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_all_slow_connection \
+     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_nested \
+     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_nested_using_list \
      tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_full_batch \
      tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_half_batch \
      tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_empty_batch \
+     tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_error \
      tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_all \
-     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_batch \
-     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_all \
-     tests.stub.iteration.test_iteration_tx_run.TestIterationTxRun.test_nested \
+     tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_all_slow_connection \
+     tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_nested \
+     tests.stub.iteration.test_iteration_session_run.TestIterationSessionRun.test_nested_using_list \
      tests.stub.driver_parameters.telemetry.test_telemetry.TestTelemetry.test_execute_read \
      tests.stub.driver_parameters.telemetry.test_telemetry.TestTelemetry.test_execute_write \
      tests.stub.driver_parameters.telemetry.test_telemetry.TestTelemetry.test_begin_transaction \