fix(bundle): the production container compiles, and the storage barrier joins the core

UPGRADE.md

@@ -145,6 +145,27 @@ changement existe. `hasSideEffectForSlot()` étant désormais au port, ce cas re
 Une exécution qui a déjà écrit un marqueur de version garde le sien : `versionForChangeId()` est
 consulté en premier, et rien de ce commit ne le touche.
 
+### Le profileur ne s'enregistre plus hors debug
+
+**Qui est concerné** : une application qui tirait `durable.execution_trace` du conteneur en
+production, ou qui injectait `WorkflowExecutionObserverInterface` en s'attendant à la trace.
+
+Le collecteur, sa trace, son écouteur de remise à zéro et son middleware Messenger n'étaient posés
+sous aucune condition. L'observateur qu'ils installent est injecté dans `ExecutionRuntime`,
+`ExecutionEngine` et `ActivityMessageProcessor` : il passait donc sur le chemin chaud de chaque
+exécution en production, pour alimenter une page que personne n'y sert. Et sa trace n'était vidée
+que par un écouteur `kernel.request`, que `messenger:consume` ne déclenche jamais — un worker
+l'accumulait tant qu'il vivait.
+
+Hors `kernel.debug`, `WorkflowExecutionObserverInterface` pointe désormais
+`Gplanchat\Durable\Debug\NullWorkflowExecutionObserver`. Le contrat d'observation est intact ;
+c'est son implémentation qui ne fait plus rien. En debug, rien ne change, sinon que la trace porte
+un tag `kernel.reset` et se vide donc aussi entre deux messages d'un worker.
+
+Une application qui veut observer les exécutions en production n'a pas à ressusciter le profileur :
+elle implémente `WorkflowExecutionObserverInterface` et aliase l'interface sur son propre service —
+ce que le profileur faisait, en moins cher et sans accumuler une timeline pour l'écran de personne.
+
 ## 0.1.0-alpha8
 
 ### The divergence guard compares the payload too

phpstan.neon

@@ -63,6 +63,13 @@ parameters:
         # de StubMethodsExtensionTest qui le diront.
         - identifier: phpstanApi.interface
           path: src/DurablePhpstan/Reflection/SchedulingMethodReflection.php
+        # `json_encode` appelle le `jsonSerialize()` de la valeur qu'on lui passe : du code
+        # applicatif, qui peut lever. PHPStan modélise la fonction comme incapable de lever, et
+        # déclare donc le `catch` mort. Il ne l'est pas — `RecordedDetailsStorableTest` le
+        # contredit en exécution, et sans ce `catch` l'exception remonte jusqu'à `collect()`,
+        # c'est-à-dire `kernel.response`, et emporte la requête.
+        - identifier: catch.neverThrown
+          path: src/Durable/Observation/RecordedDetails.php
         # DurableBundleTestTrait is consumed by application tests, not by library code
         - identifier: trait.unused
           path: src/DurableBundle/Testing/DurableBundleTestTrait.php

src/Durable/Debug/NullWorkflowExecutionObserver.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Gplanchat\Durable\Debug;
+
+/**
+ * The observer for when nobody observes.
+ *
+ * Observation is optional by contract, but the three services on the hot path,
+ * {@see \Gplanchat\Durable\ExecutionRuntime}, {@see \Gplanchat\Durable\ExecutionEngine} and
+ * {@see \Gplanchat\Durable\Worker\ActivityMessageProcessor}, take one by injection. So they need
+ * somebody, including in production where there is no profiler to feed.
+ *
+ * Doing nothing is a real behaviour here rather than a hole plugged to satisfy a signature: an
+ * execution nobody watches runs the same. It is the same reason that makes `Psr\Log\NullLogger` a
+ * legitimate null object.
+ */
+final class NullWorkflowExecutionObserver implements WorkflowExecutionObserverInterface
+{
+    #[\Override]
+    public function onWorkflowRun(string $executionId, string $workflowType, bool $isResume): void {}
+
+    #[\Override]
+    public function onActivityExecuted(
+        string $executionId,
+        string $activityId,
+        string $activityName,
+        float $durationSeconds,
+        bool $success,
+        ?string $errorClass,
+    ): void {}
+}

src/Durable/Observation/RecordedDetails.php

@@ -52,4 +52,45 @@ public static function of(array $details): ?string
         // over on the very event you came to look at.
         return false === $rendered ? null : $rendered;
     }
+
+    /**
+     * The same degradation, rendered as **structure** rather than as text.
+     *
+     * {@see self::of()} serves the surfaces that show a fold-out: they want text, once. The Symfony
+     * profiler has to *store* what it observed before the Profiler serialises the whole profile, and
+     * a payload that refuses `serialize()` does not break the Durable panel there, it breaks the
+     * request's profile, other bundles' panels included. The need is the same but for a type, and the
+     * degradation decision must stay here: it is what this class is for.
+     *
+     * Three departures from `of()`, each one measured:
+     *
+     * - **`json_encode` can throw.** It calls the payload's `jsonSerialize()`, so business code. No
+     *   flag covers that case, and an exception surfacing from here kills the request from
+     *   `kernel.response`, earlier and more visibly than the defect being fixed. Hence the `catch`.
+     * - **`JSON_PRESERVE_ZERO_FRACTION`.** Without it a `float` holding a whole value comes back as
+     *   an `int`, and the timeline bounds (`tMin`, `tMax`, `spanSec`), which declare `float`, lie
+     *   about their type.
+     * - **Depth.** Past 512 levels `json_decode` returns `null` where the encoding had produced
+     *   text. The caller therefore applies this method **key by key**: the pathological payload
+     *   disappears on its own, and the rest of the panel holds.
+     *
+     * @return mixed the value brought back to the types JSON holds; `null` when nothing survived
+     */
+    public static function storable(mixed $value): mixed
+    {
+        try {
+            $rendered = json_encode(
+                $value,
+                \JSON_INVALID_UTF8_SUBSTITUTE | \JSON_PARTIAL_OUTPUT_ON_ERROR | \JSON_PRESERVE_ZERO_FRACTION,
+            );
+        } catch (\Throwable) {
+            return null;
+        }
+
+        if (false === $rendered) {
+            return null;
+        }
+
+        return json_decode($rendered, true);
+    }
 }

src/DurableBundle/Command/DiagnoseExecutionCommand.php

@@ -4,6 +4,7 @@
 
 namespace Gplanchat\Durable\Bundle\Command;
 
+use Gplanchat\Durable\Observation\RecordedDetails;
 use Gplanchat\Durable\Store\ChildWorkflowParentLinkStoreInterface;
 use Gplanchat\Durable\Store\EventStoreInterface;
 use Gplanchat\Durable\Store\WorkflowMetadataStore;
@@ -66,7 +67,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $sample[] = [
                     'type' => $short,
                     'recordedAt' => $recordedAt?->format(\DateTimeInterface::ATOM),
-                    'payload' => $event->payload(),
+                    // The same barrier as the profiler: the command reads a production journal,
+                    // and a payload that refuses encoding would bring down the very diagnosis
+                    // one came for.
+                    'payload' => RecordedDetails::storable($event->payload()),
                 ];
             }
         }

src/DurableBundle/DataCollector/DurableDataCollector.php

@@ -27,6 +27,7 @@
 use Gplanchat\Durable\Event\WorkflowExecutionFailed;
 use Gplanchat\Durable\Event\WorkflowSignalReceived;
 use Gplanchat\Durable\Event\WorkflowUpdateHandled;
+use Gplanchat\Durable\Observation\RecordedDetails;
 use Gplanchat\Durable\Store\EventStoreInterface;
 use Gplanchat\Durable\Store\WorkflowMetadataStore;
 use Symfony\Component\HttpFoundation\Request;
@@ -112,6 +113,14 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
                 $grouped,
             ),
         ];
+
+        // The barrier, at the one place `$this->data` is built. It applies **key by key**: a
+        // pathological payload makes its own panel disappear, not the whole collector, which is
+        // what the blanket barrier did not guarantee, `$this->data` being typed
+        // `array|Data` chez le parent.
+        foreach ($this->data as $cle => $valeur) {
+            $this->data[$cle] = RecordedDetails::storable($valeur);
+        }
     }
 
     /**

src/DurableBundle/DependencyInjection/DurableExtension.php

@@ -44,6 +44,7 @@
 use Gplanchat\Durable\Bundle\SchemaListener\DurableSchemaListener;
 use Gplanchat\Durable\Bundle\Transport\MessengerActivityTransport;
 use Gplanchat\Durable\Bundle\Transport\MessengerWorkflowTimerDispatcher;
+use Gplanchat\Durable\Debug\NullWorkflowExecutionObserver;
 use Gplanchat\Durable\Debug\WorkflowExecutionObserverInterface;
 use Gplanchat\Durable\Handler\FireWorkflowTimersHandler;
 use Gplanchat\Durable\Handler\ResumeWorkflowHandler;
@@ -76,6 +77,7 @@
 use Gplanchat\Durable\Worker\ActivityMessageProcessor;
 use Gplanchat\Durable\Workflow\WorkflowDefinitionLoader;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Extension\Extension;
 use Symfony\Component\DependencyInjection\Reference;
 use Temporal\Api\Workflowservice\V1\WorkflowServiceClient;
@@ -95,7 +97,15 @@ public function load(array $configs, ContainerBuilder $container): void
         $asyncChildMessenger = (bool) ($config['child_workflow']['async_messenger'] ?? false);
         $container->setParameter('durable.child_workflow_async_messenger', $asyncChildMessenger);
 
-        $this->registerProfiler($container);
+        // A synthetic container, an extension test for instance, does not have this parameter;
+        // that does not make it production, hence the default to debug.
+        $debug = !$container->hasParameter('kernel.debug') || (bool) $container->getParameter('kernel.debug');
+
+        if ($debug) {
+            $this->registerProfiler($container);
+        } else {
+            $this->registerNullObserver($container);
+        }
         $this->registerChildWorkflowParentLinkStore($container);
         $this->registerWorkflowDefinitionLoader($container);
         $this->registerEventStore($container, $config);
@@ -677,7 +687,11 @@ private function registerWorkflowMessengerServices(ContainerBuilder $container,
                     new Reference(WorkflowClientInterface::class),
                     new Reference(WorkflowMetadataStore::class),
                     new Reference(WorkflowDefinitionLoader::class),
-                    new Reference('durable.execution_trace'),
+                    // Le profileur n'existe qu'en debug depuis ce correctif, et le constructeur
+                    // target declares the dependency `?DurableExecutionTrace $executionTrace = null`.
+                    // A bare reference would fail the production container's compilation as soon as
+                    // a `temporal.dsn` is configured.
+                    new Reference('durable.execution_trace', ContainerInterface::NULL_ON_INVALID_REFERENCE),
                 ])
                 ->setPublic(true)
             ;
@@ -760,16 +774,60 @@ private function registerCommands(ContainerBuilder $container, array $config): v
         ;
     }
 
-    private function registerProfiler(ContainerBuilder $container): void
+    /**
+     * The profiler is not neutral plumbing: its observer is injected into
+     * `ExecutionRuntime`, `ExecutionEngine` et `ActivityMessageProcessor`, donc il passe sur le
+     * hot path of every execution, and its trace is emptied only by a `kernel.request` listener,
+     * which `messenger:consume` never fires.
+     *
+     * Hors debug, on n'en enregistre donc rien du tout et l'observation retombe sur un objet nul.
+     * FrameworkBundle does the same for its own collectors, loaded from separate files under a
+     * condition.
+     */
+    private function registerNullObserver(ContainerBuilder $container): void
     {
-        $container->register('durable.execution_trace', DurableExecutionTrace::class)
+        $container->register('durable.execution_observer.null', NullWorkflowExecutionObserver::class)
+            ->setPublic(false)
+        ;
+
+        self::aliaserObservateur($container, 'durable.execution_observer.null');
+    }
+
+    /**
+     * Aliases the observation interface, **without overwriting what the application already
+     * declared**.
+     *
+     * `UPGRADE.md` invites an application that wants to observe its executions in production to
+     * implement the contract and alias the interface onto its own service. The definitions in the
+     * application's `services.yaml` already exist when the extension loads, since
+     * `MergeExtensionConfigurationPass` runs at compilation, after the configuration is loaded, so
+     * an unconditional `setAlias()` erased that alias and the escape hatch did not work.
+     */
+    private static function aliaserObservateur(ContainerBuilder $container, string $service): void
+    {
+        if ($container->hasAlias(WorkflowExecutionObserverInterface::class)
+            || $container->hasDefinition(WorkflowExecutionObserverInterface::class)
+        ) {
+            return;
+        }
+
+        $container->setAlias(WorkflowExecutionObserverInterface::class, $service)
             ->setPublic(true)
         ;
+    }
 
-        $container->setAlias(WorkflowExecutionObserverInterface::class, 'durable.execution_trace')
+    private function registerProfiler(ContainerBuilder $container): void
+    {
+        $container->register('durable.execution_trace', DurableExecutionTrace::class)
+            // `ResetDurableProfilerListener` ne borne que le cas HTTP. Dans un worker il n'y a pas
+            // request, and it is `services_resetter`, so this tag, that empties the trace between
+            // deux messages. Sans lui, un `messenger:consume` accumule la timeline tant qu'il vit.
+            ->addTag('kernel.reset', ['method' => 'reset'])
             ->setPublic(true)
         ;
 
+        self::aliaserObservateur($container, 'durable.execution_trace');
+
         $container->register(ResetDurableProfilerListener::class)
             ->setArguments([new Reference('durable.execution_trace')])
             ->addTag('kernel.event_subscriber')

tests/unit/Durable/Observation/RecordedDetailsStorableTest.php

@@ -0,0 +1,101 @@
+<?php
+
+declare(strict_types=1);
+
+namespace unit\Gplanchat\Durable\Observation;
+
+use Gplanchat\Durable\Observation\RecordedDetails;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Les quatre façons dont un aller-retour JSON naïf trahit — chacune constatée en exécution avant
+ * d'être écrite ici.
+ *
+ * Une barrière de stockage se juge sur ce qu'elle fait des entrées hostiles, pas sur ce qu'elle
+ * fait des entrées ordinaires. Trois de ces cas passaient à travers.
+ */
+final class RecordedDetailsStorableTest extends TestCase
+{
+    /**
+     * `json_encode` appelle le `jsonSerialize()` de la charge utile : du code métier, qui peut
+     * lever. Aucun drapeau ne couvre ce cas, et l'exception remonterait jusqu'à `collect()`,
+     * c'est-à-dire `kernel.response` — la requête tombe, alors que le défaut d'origine ne
+     * cassait que l'écriture du profil sur `kernel.terminate`.
+     */
+    public function testUneChargeUtileDontLaSerialisationLeveNeFaitPasTomberLAppelant(): void
+    {
+        $piege = new class implements \JsonSerializable {
+            public function jsonSerialize(): mixed
+            {
+                throw new \RuntimeException('du code métier, dans le profileur');
+            }
+        };
+
+        self::assertNull(RecordedDetails::storable(['payload' => $piege]));
+    }
+
+    /**
+     * Au-delà de 512 niveaux, `json_decode` rend `null` là où l'encodage avait produit du texte.
+     * La valeur disparaît — c'est assumé — mais l'appelant doit pouvoir ranger le résultat dans
+     * une propriété typée sans lever, d'où l'application clé par clé côté collecteur.
+     */
+    public function testUneImbricationPlusProfondeQueJsonNeLeTientRendNull(): void
+    {
+        $profond = 'fond';
+        for ($i = 0; $i < 600; ++$i) {
+            $profond = [$profond];
+        }
+
+        self::assertNull(RecordedDetails::storable($profond));
+    }
+
+    /**
+     * Les bornes de la frise se déclarent `float`. Sans `JSON_PRESERVE_ZERO_FRACTION`, une durée
+     * de trois secondes tout rondes revient en `int` et le type déclaré ment.
+     */
+    public function testUnFlottantDeValeurEntiereResteUnFlottant(): void
+    {
+        $storable = RecordedDetails::storable(['spanSec' => 3.0, 'tMin' => 0.0]);
+
+        self::assertIsArray($storable);
+        self::assertIsFloat($storable['spanSec']);
+        self::assertIsFloat($storable['tMin']);
+    }
+
+    #[DataProvider('chargesUtilesOrdinaires')]
+    public function testCeQuiEtaitLisibleLeResteALIdentique(mixed $valeur): void
+    {
+        self::assertSame($valeur, RecordedDetails::storable($valeur));
+    }
+
+    /**
+     * @return iterable<string, array{mixed}>
+     */
+    public static function chargesUtilesOrdinaires(): iterable
+    {
+        yield 'chaîne' => ['bonjour'];
+        yield 'entier' => [42];
+        yield 'flottant' => [1.5];
+        yield 'booléen' => [true];
+        yield 'null' => [null];
+        yield 'liste' => [[1, 2, 3]];
+        yield 'tableau associatif' => [['a' => 1, 'b' => ['c' => 'd']]];
+    }
+
+    /**
+     * Une référence récursive, elle, survit : `JSON_PARTIAL_OUTPUT_ON_ERROR` la coupe et rend le
+     * reste. Le cas est ici pour qu'on cesse de le croire cassé.
+     */
+    public function testUneReferenceRecursiveEstTronqueeEtNonPerdue(): void
+    {
+        $objet = new \stdClass();
+        $objet->nom = 'boucle';
+        $objet->soi = $objet;
+
+        $storable = RecordedDetails::storable(['payload' => $objet]);
+
+        self::assertIsArray($storable);
+        self::assertSame('boucle', $storable['payload']['nom']);
+    }
+}