BUGFIX: Ensure onAfterCatchUp is always executed _after_ the projec…

…tion is persisted. Even in error case.

Neos.ContentRepository.BehavioralTests/Tests/Functional/Subscription/CatchUpHookErrorTest.php

@@ -156,7 +156,7 @@ public function error_onBeforeCatchUp_abortsCatchup()
     }
 
     /** @test */
-    public function error_onAfterCatchUp_abortsCatchupAndRollBack()
+    public function error_onAfterCatchUp_crashesAfterProjectionsArePersisted()
     {
         $this->eventStore->setup();
         $this->fakeProjection->expects(self::once())->method('setUp');
@@ -190,10 +190,64 @@ public function error_onAfterCatchUp_abortsCatchupAndRollBack()
 
         self::assertSame($expectedFailure->getMessage(), 'Subscriber "Vendor.Package:SecondFakeProjection" failed onAfterCatchUp: This catchup hook is kaputt.');
 
-        // still the initial status
+        // one event is applied!
+        $this->expectOkayStatus('Vendor.Package:SecondFakeProjection', SubscriptionStatus::ACTIVE, SequenceNumber::fromInteger(1));
+        self::assertEquals(
+            [SequenceNumber::fromInteger(1)],
+            $this->secondFakeProjection->getState()->findAppliedSequenceNumbers()
+        );
+    }
+
+    /** @test */
+    public function error_onAfterCatchUp_crashesAfterProjectionsArePersisted_withProjectionError()
+    {
+        $this->eventStore->setup();
+        $this->fakeProjection->expects(self::once())->method('setUp');
+        $this->fakeProjection->expects(self::once())->method('apply');
+        $this->subscriptionEngine->setup();
+        $this->subscriptionEngine->boot();
+
         $this->expectOkayStatus('Vendor.Package:SecondFakeProjection', SubscriptionStatus::ACTIVE, SequenceNumber::none());
 
-        // must be empty because full rollback
+        // commit an event
+        $this->commitExampleContentStreamEvent();
+
+        $this->catchupHookForFakeProjection->expects(self::once())->method('onBeforeCatchUp');
+        $this->catchupHookForFakeProjection->expects(self::once())->method('onBeforeEvent');
+        $this->catchupHookForFakeProjection->expects(self::once())->method('onAfterEvent')->willThrowException(
+            $innerException = new \RuntimeException('Inner event handling is kaputt.')
+        );;
+        $this->catchupHookForFakeProjection->expects(self::once())->method('onAfterCatchUp')->willThrowException(
+            new \RuntimeException('This catchup hook is kaputt.')
+        );
+
+        self::assertEmpty(
+            $this->secondFakeProjection->getState()->findAppliedSequenceNumbers()
+        );
+
+        $expectedFailure = null;
+        try {
+            $this->subscriptionEngine->catchUpActive();
+        } catch (\Throwable $e) {
+            $expectedFailure = $e;
+        }
+        self::assertInstanceOf(CatchUpFailed::class, $expectedFailure);
+
+        self::assertSame($expectedFailure->getMessage(), 'Subscriber "Vendor.Package:SecondFakeProjection" had an error and also failed onAfterCatchUp: This catchup hook is kaputt.');
+
+        $expectedFailure = ProjectionSubscriptionStatus::create(
+            subscriptionId: SubscriptionId::fromString('Vendor.Package:SecondFakeProjection'),
+            subscriptionStatus: SubscriptionStatus::ERROR,
+            subscriptionPosition: SequenceNumber::none(),
+            subscriptionError: SubscriptionError::fromPreviousStatusAndException(SubscriptionStatus::ACTIVE, $innerException),
+            setupStatus: ProjectionStatus::ok(),
+        );
+
+        // projection is still marked as error
+        self::assertEquals(
+            $expectedFailure,
+            $this->subscriptionStatus('Vendor.Package:SecondFakeProjection')
+        );
         self::assertEmpty(
             $this->secondFakeProjection->getState()->findAppliedSequenceNumbers()
         );

Neos.ContentRepository.Core/Classes/Projection/CatchUpHook/CatchUpHookInterface.php

@@ -5,7 +5,8 @@
 namespace Neos\ContentRepository\Core\Projection\CatchUpHook;
 
 use Neos\ContentRepository\Core\EventStore\EventInterface;
-use Neos\ContentRepository\Core\Subscription\Engine\SubscriptionEngine;
+use Neos\ContentRepository\Core\Projection\ProjectionInterface;
+use Neos\ContentRepository\Core\Subscription\Exception\CatchUpFailed;
 use Neos\ContentRepository\Core\Subscription\SubscriptionStatus;
 use Neos\EventStore\Model\EventEnvelope;
 
@@ -21,25 +22,34 @@ interface CatchUpHookInterface
 {
     /**
      * This hook is called at the beginning of a catch-up run;
-     * AFTER the Database Lock is acquired ({@see SubscriptionEngine::catchUpActive()}).
+     * AFTER the Database Lock is acquired, BEFORE any projection was opened.
+     *
+     * Its important that no errors are thrown, as they will cause the catchup to directly halt with a {@see CatchUpFailed} exception.
      */
     public function onBeforeCatchUp(SubscriptionStatus $subscriptionStatus): void;
 
     /**
      * This hook is called for every event during the catchup process, **before** the projection
-     * is updated. Thus, this hook runs AFTER the database lock is acquired.
+     * is updated but in the same transaction: {@see ProjectionInterface::transactional()}.
+     *
+     * Any errors will cause the transaction being rolled back, and the projection goes into {@see SubscriptionStatus::ERROR} state.
      */
     public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $eventEnvelope): void;
 
     /**
      * This hook is called for every event during the catchup process, **after** the projection
-     * is updated. Thus, this hook runs AFTER the database lock is acquired.
+     * is updated but in the same transaction: {@see ProjectionInterface::transactional()}.
+     *
+     * Any errors will cause the transaction being rolled back, and the projection goes into {@see SubscriptionStatus::ERROR} state.
      */
     public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $eventEnvelope): void;
 
     /**
      * This hook is called at the END of a catch-up run
-     * BEFORE the Database Lock is released ({@see SubscriptionEngine::catchUpActive()}).
+     * BEFORE the Database Lock is released, but AFTER the transaction is commited.
+     *
+     * Its important that no errors are thrown, as they will cause the catchup to directly halt with a {@see CatchUpFailed} exception.
+     * The projections and their new position will already be persisted and there is no rollback.
      */
     public function onAfterCatchUp(): void;
 }