From 08e9e2f0f91d6965ffae2e3d1b7830aad21b147e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Planchat?= Date: Fri, 4 Sep 2026 00:07:49 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(bundle):=20le=20profileur=20cesse=20d'i?= =?UTF-8?q?nstrumenter=20la=20production,=20et=20son=20profil=20se=20s?= =?UTF-8?q?=C3=A9rialise?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deux défauts de la même surface, l'un cachant l'autre. **Le profileur tournait en production.** `registerProfiler()` était appelé sans condition, et l'observateur qu'il aliase est injecté dans ExecutionRuntime, ExecutionEngine et ActivityMessageProcessor : il passait sur le chemin chaud de chaque exécution, pour alimenter une page que personne ne sert en production. Hors kernel.debug, rien de tout cela n'est plus enregistré et l'observation retombe sur NullWorkflowExecutionObserver — ne rien faire est ici un comportement réel, pas un bouche-trou de signature : une exécution que personne ne regarde s'exécute pareil. **Sa trace n'était jamais vidée dans un worker.** ResetDurableProfilerListener écoute kernel.request, que `messenger:consume` ne déclenche jamais ; la timeline grossissait tant que le processus vivait. Le tag `kernel.reset` la confie à services_resetter, qui est ce qui vide entre deux messages. La méthode reset() existait déjà — il ne manquait que la déclaration. **Une charge utile non sérialisable cassait le profil entier.** Le collecteur rangeait les charges utiles brutes et __serialize() les rendait telles quelles : une closure dans une charge utile ne fait pas tomber le panneau Durable, elle fait tomber le profil de la requête, panneaux des autres bundles compris. Une barrière au seul endroit où $this->data est constitué ramène tout à des scalaires et des tableaux — la conversion que le gabarit fait de toute façon pour afficher. JSON_INVALID_UTF8_SUBSTITUTE règle au passage un second défaut du même endroit : un journal porte des octets, pas forcément du texte valide, et le gabarit affichait un vide là où il y avait une charge utile. Un cas garde la forme : une charge utile ordinaire — entiers, flottants, booléens, null, liste, tableau imbriqué — traverse la barrière à l'identique, sinon un aller-retour JSON casserait en silence les clés que le gabarit lit. Le placement de l'objet nul dans le cœur est validé par CoreDependsOnNoHostTest, garde à jeu de données sur chaque fichier de src/Durable qui vérifie qu'aucun n'importe un hôte ni un pont. Suite unit : 1084 tests contre 1073 sur main, mêmes 4 erreurs d'environnement. PHPStan : 2 erreurs, exactement la base de main, aucun diagnostic nouveau. Refs: B2 et B3 de documentation/audit/ Co-Authored-By: Claude Opus 5 (1M context) --- UPGRADE.md | 23 ++++ .../Debug/NullWorkflowExecutionObserver.php | 33 +++++ .../DataCollector/DurableDataCollector.php | 29 +++++ .../DependencyInjection/DurableExtension.php | 36 +++++- .../DurableProfilerWiringTest.php | 94 ++++++++++++++ .../DurableDataCollectorSerialisationTest.php | 121 ++++++++++++++++++ 6 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 src/Durable/Debug/NullWorkflowExecutionObserver.php create mode 100644 tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php create mode 100644 tests/unit/DurableBundle/DurableDataCollectorSerialisationTest.php diff --git a/UPGRADE.md b/UPGRADE.md index 074f7922..81b56f1e 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -22,6 +22,29 @@ vendor/bin/rector process src Le set est **cumulatif** : le passer une fois rattrape toutes les versions franchies d'un coup. Il ne contient que ce que Rector sait faire sans deviner ; tout le reste est écrit à la main ci-dessous. +## Non publié + +### 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 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat diff --git a/src/Durable/Debug/NullWorkflowExecutionObserver.php b/src/Durable/Debug/NullWorkflowExecutionObserver.php new file mode 100644 index 00000000..11c11911 --- /dev/null +++ b/src/Durable/Debug/NullWorkflowExecutionObserver.php @@ -0,0 +1,33 @@ +data` est constitué : ce qui sort d'ici + // est stockable, quelle que soit la charge utile observée. + $this->data = self::storable($this->data); } /** @@ -833,6 +837,31 @@ public function reset(): void $this->data = []; } + /** + * Ramène à des scalaires et des tableaux ce que le collecteur vient de ranger. + * + * Une charge utile de workflow est de la donnée métier : n'importe quoi peut s'y trouver, y + * compris ce qui refuse `serialize()`. Or le Profiler sérialise le profil **entier** pour + * l'écrire — une closure dans une charge utile ne casse donc pas le panneau Durable, elle + * casse le profil de la requête, panneaux des autres bundles compris. + * + * Le passage par JSON est la conversion que le gabarit fait de toute façon pour afficher + * (`|json_encode`) : rien n'est perdu de ce qui était montré, et ce qui ne pouvait pas l'être + * ne fait plus tomber ce qui l'entoure. + * + * - `JSON_INVALID_UTF8_SUBSTITUTE` — un journal porte des octets, pas forcément du texte + * valide ; sans lui `json_encode` rend `false`, et le gabarit affiche un vide là où il y + * avait une charge utile. + * - `JSON_PARTIAL_OUTPUT_ON_ERROR` — une valeur inencodable devient `null` plutôt que + * d'emporter tout le tableau qui la contient. + */ + private static function storable(mixed $value): mixed + { + $json = json_encode($value, \JSON_INVALID_UTF8_SUBSTITUTE | \JSON_PARTIAL_OUTPUT_ON_ERROR); + + return false === $json ? null : json_decode($json, true); + } + /** * Pas de `#[\Override]` : le DataCollector de Symfony ne déclare `__serialize()` qu'à partir * de 7.0, et l'attribut ferait échouer le chargement de cette classe sur 6.4 (PHP ≥ 8.3). diff --git a/src/DurableBundle/DependencyInjection/DurableExtension.php b/src/DurableBundle/DependencyInjection/DurableExtension.php index f7aba215..bb191565 100644 --- a/src/DurableBundle/DependencyInjection/DurableExtension.php +++ b/src/DurableBundle/DependencyInjection/DurableExtension.php @@ -42,6 +42,7 @@ use Gplanchat\Durable\Bundle\Profiler\DurableExecutionTrace; 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; @@ -93,7 +94,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); + // Un conteneur synthétique — un test d'extension, par exemple — n'a pas ce paramètre ; + // il n'est pas en production pour autant, d'où le défaut à « 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); @@ -739,9 +748,34 @@ private function registerCommands(ContainerBuilder $container, array $config): v ; } + /** + * Le profileur n'est pas de la plomberie neutre : son observateur est injecté dans + * `ExecutionRuntime`, `ExecutionEngine` et `ActivityMessageProcessor`, donc il passe sur le + * chemin chaud de chaque exécution, et sa trace n'est vidée que par un écouteur + * `kernel.request` — que `messenger:consume` ne déclenche jamais. + * + * Hors debug, on n'en enregistre donc rien du tout et l'observation retombe sur un objet nul. + * FrameworkBundle procède ainsi pour ses propres collecteurs, chargés depuis des fichiers + * séparés sous condition. + */ + private function registerNullObserver(ContainerBuilder $container): void + { + $container->register('durable.execution_observer.null', NullWorkflowExecutionObserver::class) + ->setPublic(false) + ; + + $container->setAlias(WorkflowExecutionObserverInterface::class, 'durable.execution_observer.null') + ->setPublic(true) + ; + } + 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 + // de requête, et c'est `services_resetter` — donc ce tag — qui vide la trace entre + // deux messages. Sans lui, un `messenger:consume` accumule la timeline tant qu'il vit. + ->addTag('kernel.reset', ['method' => 'reset']) ->setPublic(true) ; diff --git a/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php b/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php new file mode 100644 index 00000000..08870bc2 --- /dev/null +++ b/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php @@ -0,0 +1,94 @@ +load(debug: true)->getDefinition('durable.execution_trace'); + + self::assertArrayHasKey( + 'kernel.reset', + $definition->getTags(), + "sans ce tag, services_resetter ignore la trace et un worker l'accumule sans borne", + ); + self::assertSame( + 'reset', + $definition->getTag('kernel.reset')[0]['method'] ?? null, + 'et le resetter a besoin du nom de la méthode', + ); + } + + public function testHorsDebugAucunCollecteurNEstEnregistre(): void + { + $container = $this->load(debug: false); + + self::assertFalse( + $container->has('durable.execution_trace'), + 'la trace de profil n\'a rien à faire en production', + ); + + foreach ($container->getDefinitions() as $id => $definition) { + self::assertArrayNotHasKey( + 'data_collector', + $definition->getTags(), + \sprintf('%s ne doit pas collecter hors debug', $id), + ); + } + } + + /** + * Le contrat d'observation reste satisfait : les trois services du chemin chaud le reçoivent + * en injection, et un conteneur qui ne le fournirait pas ne compilerait plus. + */ + public function testHorsDebugLObservateurEstUnObjetNul(): void + { + $container = $this->load(debug: false); + + self::assertTrue($container->hasAlias(WorkflowExecutionObserverInterface::class)); + + $target = (string) $container->getAlias(WorkflowExecutionObserverInterface::class); + self::assertSame( + \Gplanchat\Durable\Debug\NullWorkflowExecutionObserver::class, + $container->getDefinition($target)->getClass(), + ); + } + + public function testEnDebugLObservateurEstBienLaTrace(): void + { + $container = $this->load(debug: true); + + self::assertSame( + 'durable.execution_trace', + (string) $container->getAlias(WorkflowExecutionObserverInterface::class), + ); + } + + private function load(bool $debug): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', $debug); + (new DurableExtension())->load([[]], $container); + + return $container; + } +} diff --git a/tests/unit/DurableBundle/DurableDataCollectorSerialisationTest.php b/tests/unit/DurableBundle/DurableDataCollectorSerialisationTest.php new file mode 100644 index 00000000..d1ed2055 --- /dev/null +++ b/tests/unit/DurableBundle/DurableDataCollectorSerialisationTest.php @@ -0,0 +1,121 @@ + + */ + public static function valeursQuiNeSeSerialisentPas(): iterable + { + yield 'closure' => [static fn(): int => 1]; + yield 'ressource' => [fopen('php://memory', 'rb')]; + yield 'objet anonyme portant une closure' => [new class { + public \Closure $callback; + + public function __construct() + { + $this->callback = static fn(): int => 1; + } + }]; + } + + #[DataProvider('valeursQuiNeSeSerialisentPas')] + public function testLeProfilResteStockableQuoiQueLaChargeUtilePorte(mixed $valeur): void + { + $collector = self::collectorAyantObserve(['commande' => 'X-1', 'hostile' => $valeur]); + + $serialise = serialize($collector); + + self::assertIsString($serialise); + self::assertInstanceOf(DurableDataCollector::class, unserialize($serialise)); + } + + /** + * Le reste de la charge utile est ce que l'exploitant est venu lire ; une valeur qui ne se + * rend pas ne doit pas l'emporter avec elle. + */ + public function testCeQuiEstLisibleDansLaChargeUtileEstConserve(): void + { + $collector = self::collectorAyantObserve([ + 'commande' => 'X-1', + 'montant' => 1250, + 'hostile' => static fn(): int => 1, + ]); + + $rendu = json_encode(unserialize(serialize($collector))->getTimeline()); + + self::assertStringContainsString('X-1', (string) $rendu); + self::assertStringContainsString('1250', (string) $rendu); + } + + /** + * Le journal peut porter des octets qui ne sont pas du texte valide. `json_encode` rend alors + * `false`, et le gabarit affiche un vide là où il y avait une charge utile. + */ + public function testUneChargeUtileBinaireResteAffichable(): void + { + $collector = self::collectorAyantObserve(['blob' => "\xB1\x31\xFE"]); + + $rendu = json_encode(unserialize(serialize($collector))->getTimeline()); + + self::assertIsString($rendu, 'une charge utile binaire ne doit pas rendre le panneau vide'); + } + + /** + * La barrière ne doit rien déformer de ce qui passait déjà : le gabarit lit des clés précises, + * et un aller-retour JSON qui transformerait une liste en objet les casserait en silence. + */ + public function testUneChargeUtileOrdinaireTraverseSansEtreDeformee(): void + { + $payload = [ + 'commande' => 'X-1', + 'montant' => 1250, + 'remise' => 0.15, + 'urgent' => false, + 'lignes' => ['a', 'b'], + 'client' => ['id' => 7, 'nom' => 'Dupont'], + 'note' => null, + ]; + + $timeline = self::collectorAyantObserve($payload)->getTimeline(); + + self::assertSame($payload, $timeline[0]['payload'] ?? null); + } + + /** + * @param array $payload + */ + private static function collectorAyantObserve(array $payload): DurableDataCollector + { + $trace = new DurableExecutionTrace(); + $trace->onWorkflowDispatchRequested('exec-1', 'Commande', $payload, false, 'async'); + + $collector = new DurableDataCollector($trace, new InMemoryWorkflowMetadataStore(), new InMemoryEventStore()); + $collector->collect(new Request(), new Response()); + + return $collector; + } +} From f765ce6d8a50332c980187aeaeb1e9de915be834 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Planchat?= Date: Fri, 4 Sep 2026 01:50:50 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(bundle):=20le=20conteneur=20de=20produc?= =?UTF-8?q?tion=20compile,=20et=20la=20barri=C3=A8re=20de=20stockage=20rej?= =?UTF-8?q?oint=20le=20c=C5=93ur?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trois défauts que la relecture croisée a trouvés dans le correctif lui-même. **Le conteneur de production ne compilait plus.** `registerProfiler()` devenu conditionnel, `durable.execution_trace` n'existe plus hors debug — mais `DurableExtension:668` le référençait encore par une référence nue dans la branche Temporal native. Une application de production avec un `temporal.dsn` levait `ServiceNotFoundException` à la compilation ; une boutique Sylius ne démarrait plus. Le constructeur cible déclare pourtant `?DurableExecutionTrace = null` : la référence passe en `NULL_ON_INVALID_REFERENCE`. Huit relectures indépendantes ont atteint ce défaut, trois en compilant réellement. Aucun test ne pouvait le voir : tous chargeaient `load([[]])`, sans DSN, donc sans jamais construire la branche fautive, et aucun n'appelait de passe de compilation. `testHorsDebugUneApplicationTemporaleCompileEncore` charge la configuration Temporale et fait tourner `CheckExceptionOnInvalidReferenceBehaviorPass` — en déclarant en synthétique, à chaque tour, le service manquant que la passe signale, jusqu'à convergence. Ce qui reste est la liste de ce que le bundle attend de FrameworkBundle ; qu'un service **à nous** s'y trouve est le bug. **La barrière de sérialisation était un doublon, et elle mordait plus fort que le défaut.** `DurableDataCollector::storable()` refaisait, en privé et en statique, ce que `Durable\Observation\RecordedDetails` fait pour toutes les surfaces d'observation depuis qu'un `json_encode` sans tolérance a rendu un dépliant vide dans le back-office Sylius. Son docbloc dit déjà que « c'est à cet endroit-ci que la dégradation se décide, pour toutes les surfaces à la fois » : la variante qui rend une structure plutôt qu'un texte y rejoint `of()`. Trois modes de panne, chacun constaté en exécution avant d'être corrigé : - `json_encode` appelle le `jsonSerialize()` de la charge utile, donc du code métier, qui peut lever. Aucun drapeau ne couvre ce cas. L'exception remontait à `collect()` — `kernel.response` — là où le défaut d'origine ne cassait que `saveProfile()`, sur `kernel.terminate`, réponse déjà partie. Le correctif rendait la panne plus précoce et visible de l'utilisateur. - Au-delà de 512 niveaux, `json_decode` rend `null`, affecté à `$this->data` typée `array|Data` chez le parent : `TypeError`. La barrière s'applique donc clé par clé — la charge utile pathologique disparaît seule, le panneau tient. - Sans `JSON_PRESERVE_ZERO_FRACTION`, un `float` de valeur entière revient `int`, et les bornes de frise qui se déclarent `float` mentent sur leur type. La récursion, elle, survit : `JSON_PARTIAL_OUTPUT_ON_ERROR` la coupe et rend le reste. Le cas est testé pour qu'on cesse de le croire cassé. `DiagnoseExecutionCommand` — le frère non traité — déversait `payload()` sans barrière : il lit un journal de production, et une charge utile inencodable y faisait tomber le diagnostic qu'on était venu chercher. Même appel. **L'échappatoire prescrite par UPGRADE.md ne fonctionnait pas.** Aliaser `WorkflowExecutionObserverInterface` sur son propre service était écrasé par le `setAlias()` de l'extension : les définitions du `services.yaml` de l'application existent déjà quand l'extension se charge. L'alias respecte désormais ce que l'application a déclaré, en debug comme en production. Restent ouverts, hors périmètre : `kernel.reset` ne se déclenche pas sur les transports Temporal, qui ne rendent aucune enveloppe — B2 n'est donc fermé que pour la moitié pilotée par le bus ; le double emploi entre ce tag et `ResetDurableProfilerListener`, dont la relecture n'a pas tranché ; et le couplage inversé qui rend tout ceci possible — `Bridge/Temporal` type-hinte une classe du bundle que son `composer.json` ne requiert pas. Co-Authored-By: Claude Opus 5 (1M context) --- phpstan.neon | 7 ++ src/Durable/Observation/RecordedDetails.php | 43 +++++++ .../Command/DiagnoseExecutionCommand.php | 6 +- .../DataCollector/DurableDataCollector.php | 36 ++---- .../DependencyInjection/DurableExtension.php | 34 ++++- .../RecordedDetailsStorableTest.php | 101 +++++++++++++++ .../DurableProfilerWiringTest.php | 118 +++++++++++++++++- 7 files changed, 310 insertions(+), 35 deletions(-) create mode 100644 tests/unit/Durable/Observation/RecordedDetailsStorableTest.php diff --git a/phpstan.neon b/phpstan.neon index 548ef88d..71f732e5 100644 --- a/phpstan.neon +++ b/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 diff --git a/src/Durable/Observation/RecordedDetails.php b/src/Durable/Observation/RecordedDetails.php index 4fa99788..fa2660f8 100644 --- a/src/Durable/Observation/RecordedDetails.php +++ b/src/Durable/Observation/RecordedDetails.php @@ -53,4 +53,47 @@ public static function of(array $details): ?string // tombe sur l'événement qu'on était venu regarder. return false === $rendered ? null : $rendered; } + + /** + * La même dégradation, rendue en **structure** plutôt qu'en texte. + * + * {@see self::of()} sert les surfaces qui affichent un dépliant : elles veulent du texte, une + * fois. Le profileur Symfony, lui, doit *ranger* ce qu'il a observé avant que le Profiler + * sérialise le profil entier — une charge utile qui refuse `serialize()` n'y casse pas le + * panneau Durable, elle casse le profil de la requête, panneaux des autres bundles compris. + * Le besoin est le même à un type près, et la décision de dégradation doit rester ici : c'est + * tout l'objet de cette classe. + * + * Trois écarts avec `of()`, chacun mesuré : + * + * - **`json_encode` peut lever.** Il appelle le `jsonSerialize()` de la charge utile, donc du + * code métier. Aucun drapeau ne couvre ce cas, et une exception qui remonte d'ici tue la + * requête depuis `kernel.response` — plus tôt et plus visiblement que le défaut qu'on + * corrigeait. D'où le `catch`. + * - **`JSON_PRESERVE_ZERO_FRACTION`** — sans lui, un `float` de valeur entière revient en + * `int` et les bornes de la frise (`tMin`, `tMax`, `spanSec`), qui se déclarent `float`, + * mentent sur leur type. + * - **La profondeur.** Au-delà de 512 niveaux, `json_decode` rend `null` là où l'encodage + * avait produit du texte. L'appelant applique donc cette méthode **clé par clé** : la + * charge utile pathologique disparaît seule, le reste du panneau tient. + * + * @return mixed la valeur ramenée aux types que JSON tient ; `null` si rien n'a survécu + */ + 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); + } } diff --git a/src/DurableBundle/Command/DiagnoseExecutionCommand.php b/src/DurableBundle/Command/DiagnoseExecutionCommand.php index 6a95fecf..64e26ba2 100644 --- a/src/DurableBundle/Command/DiagnoseExecutionCommand.php +++ b/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(), + // Même barrière que le profileur : la commande lit un journal de production, + // et une charge utile qui refuse l'encodage y ferait tomber le diagnostic + // qu'on était précisément venu chercher. + 'payload' => RecordedDetails::storable($event->payload()), ]; } } diff --git a/src/DurableBundle/DataCollector/DurableDataCollector.php b/src/DurableBundle/DataCollector/DurableDataCollector.php index 8384c549..cf43f871 100644 --- a/src/DurableBundle/DataCollector/DurableDataCollector.php +++ b/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; @@ -113,9 +114,13 @@ public function collect(Request $request, Response $response, ?\Throwable $excep ), ]; - // Une seule barrière, au seul endroit où `$this->data` est constitué : ce qui sort d'ici - // est stockable, quelle que soit la charge utile observée. - $this->data = self::storable($this->data); + // La barrière, au seul endroit où `$this->data` est constitué. Elle s'applique **clé par + // clé** : une charge utile pathologique fait disparaître son panneau, pas le collecteur + // entier — ce que la barrière d'ensemble ne garantissait pas, `$this->data` étant typée + // `array|Data` chez le parent. + foreach ($this->data as $cle => $valeur) { + $this->data[$cle] = RecordedDetails::storable($valeur); + } } /** @@ -837,31 +842,6 @@ public function reset(): void $this->data = []; } - /** - * Ramène à des scalaires et des tableaux ce que le collecteur vient de ranger. - * - * Une charge utile de workflow est de la donnée métier : n'importe quoi peut s'y trouver, y - * compris ce qui refuse `serialize()`. Or le Profiler sérialise le profil **entier** pour - * l'écrire — une closure dans une charge utile ne casse donc pas le panneau Durable, elle - * casse le profil de la requête, panneaux des autres bundles compris. - * - * Le passage par JSON est la conversion que le gabarit fait de toute façon pour afficher - * (`|json_encode`) : rien n'est perdu de ce qui était montré, et ce qui ne pouvait pas l'être - * ne fait plus tomber ce qui l'entoure. - * - * - `JSON_INVALID_UTF8_SUBSTITUTE` — un journal porte des octets, pas forcément du texte - * valide ; sans lui `json_encode` rend `false`, et le gabarit affiche un vide là où il y - * avait une charge utile. - * - `JSON_PARTIAL_OUTPUT_ON_ERROR` — une valeur inencodable devient `null` plutôt que - * d'emporter tout le tableau qui la contient. - */ - private static function storable(mixed $value): mixed - { - $json = json_encode($value, \JSON_INVALID_UTF8_SUBSTITUTE | \JSON_PARTIAL_OUTPUT_ON_ERROR); - - return false === $json ? null : json_decode($json, true); - } - /** * Pas de `#[\Override]` : le DataCollector de Symfony ne déclare `__serialize()` qu'à partir * de 7.0, et l'attribut ferait échouer le chargement de cette classe sur 6.4 (PHP ≥ 8.3). diff --git a/src/DurableBundle/DependencyInjection/DurableExtension.php b/src/DurableBundle/DependencyInjection/DurableExtension.php index bb191565..7554cd5d 100644 --- a/src/DurableBundle/DependencyInjection/DurableExtension.php +++ b/src/DurableBundle/DependencyInjection/DurableExtension.php @@ -75,6 +75,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; @@ -665,7 +666,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 + // cible déclare la dépendance `?DurableExecutionTrace $executionTrace = null`. + // Une référence nue ferait échouer la compilation du conteneur de production + // dès qu'un `temporal.dsn` est configuré. + new Reference('durable.execution_trace', ContainerInterface::NULL_ON_INVALID_REFERENCE), ]) ->setPublic(true) ; @@ -764,7 +769,28 @@ private function registerNullObserver(ContainerBuilder $container): void ->setPublic(false) ; - $container->setAlias(WorkflowExecutionObserverInterface::class, 'durable.execution_observer.null') + self::aliaserObservateur($container, 'durable.execution_observer.null'); + } + + /** + * Aliase l'interface d'observation, **sans écraser ce que l'application a déjà déclaré**. + * + * `UPGRADE.md` invite une application qui veut observer ses exécutions en production à + * implémenter le contrat et à aliaser l'interface sur son propre service. Les définitions du + * `services.yaml` de l'application existent déjà quand l'extension se charge — le + * `MergeExtensionConfigurationPass` tourne à la compilation, après le chargement de la + * configuration — si bien qu'un `setAlias()` inconditionnel effaçait cet alias-là, et + * l'échappatoire ne fonctionnait pas. + */ + 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) ; } @@ -779,9 +805,7 @@ private function registerProfiler(ContainerBuilder $container): void ->setPublic(true) ; - $container->setAlias(WorkflowExecutionObserverInterface::class, 'durable.execution_trace') - ->setPublic(true) - ; + self::aliaserObservateur($container, 'durable.execution_trace'); $container->register(ResetDurableProfilerListener::class) ->setArguments([new Reference('durable.execution_trace')]) diff --git a/tests/unit/Durable/Observation/RecordedDetailsStorableTest.php b/tests/unit/Durable/Observation/RecordedDetailsStorableTest.php new file mode 100644 index 00000000..fdb89f14 --- /dev/null +++ b/tests/unit/Durable/Observation/RecordedDetailsStorableTest.php @@ -0,0 +1,101 @@ + $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 + */ + 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']); + } +} diff --git a/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php b/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php index 08870bc2..d6a0163e 100644 --- a/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php +++ b/tests/unit/DurableBundle/DependencyInjection/DurableProfilerWiringTest.php @@ -6,8 +6,14 @@ use Gplanchat\Durable\Bundle\DependencyInjection\DurableExtension; use Gplanchat\Durable\Debug\WorkflowExecutionObserverInterface; +use Gplanchat\Durable\Port\WorkflowResumeDispatcher; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; +use Symfony\Component\DependencyInjection\Reference; /** * Ce que le profileur coûte quand personne ne le regarde. @@ -22,6 +28,8 @@ */ final class DurableProfilerWiringTest extends TestCase { + private const DSN = 'temporal://127.0.0.1:7233?namespace=demo-boutique&tls=0'; + public function testLaTraceEstReinitialisableEntreDeuxMessagesDUnWorker(): void { $definition = $this->load(debug: true)->getDefinition('durable.execution_trace'); @@ -83,12 +91,120 @@ public function testEnDebugLObservateurEstBienLaTrace(): void ); } - private function load(bool $debug): ContainerBuilder + /** + * Retirer le profileur de la production ne suffit pas : il faut que plus rien ne le réclame. + * + * `TemporalWorkflowResumeDispatcher` recevait `durable.execution_trace` par une référence nue. + * Le service n'étant plus enregistré hors debug, le conteneur d'une application de production + * configurée en Temporal natif ne compilait plus — et aucun test ne le voyait, tous chargeant + * une configuration vide, donc sans jamais construire cette branche. + */ + public function testHorsDebugUneApplicationTemporaleCompileEncore(): void + { + $container = $this->load(debug: false, config: ['temporal' => ['dsn' => self::DSN]]); + + $arguments = $container->getDefinition(WorkflowResumeDispatcher::class)->getArguments(); + $trace = $arguments[3] ?? null; + + self::assertInstanceOf(Reference::class, $trace); + self::assertSame('durable.execution_trace', (string) $trace); + self::assertSame( + ContainerInterface::NULL_ON_INVALID_REFERENCE, + $trace->getInvalidBehavior(), + 'une référence nue vers un service absent hors debug fait échouer la compilation', + ); + + self::assertNotContains( + 'durable.execution_trace', + self::servicesManquants($container), + 'le conteneur de production ne doit plus réclamer un service que le debug seul enregistre', + ); + } + + public function testEnDebugLeMemeConteneurRecoitLaVraieTrace(): void + { + $container = $this->load(debug: true, config: ['temporal' => ['dsn' => self::DSN]]); + + self::assertTrue($container->has('durable.execution_trace')); + self::assertSame( + 'durable.execution_trace', + (string) $container->getDefinition(WorkflowResumeDispatcher::class)->getArgument(3), + ); + } + + /** + * `UPGRADE.md` prescrit cette échappatoire aux applications qui veulent observer en + * production : implémenter le contrat, aliaser l'interface. Elle ne fonctionnait pas — le + * `setAlias()` de l'extension écrasait celui de l'application, dont les définitions sont + * pourtant déjà là quand l'extension se charge. + */ + #[DataProvider('environnements')] + public function testUnAliasDeLApplicationNEstPasEcrase(bool $debug): void { $container = new ContainerBuilder(); $container->setParameter('kernel.debug', $debug); + $container->register('app.observateur', \stdClass::class); + $container->setAlias(WorkflowExecutionObserverInterface::class, 'app.observateur'); + (new DurableExtension())->load([[]], $container); + self::assertSame( + 'app.observateur', + (string) $container->getAlias(WorkflowExecutionObserverInterface::class), + ); + } + + /** + * @return iterable + */ + public static function environnements(): iterable + { + yield 'debug' => [true]; + yield 'production' => [false]; + } + + /** + * Les identifiants qu'un conteneur réclame sans les avoir. + * + * Le bundle seul ne compile pas : il référence légitimement des services que FrameworkBundle + * fournit (`messenger.default_bus`, …). On déclare donc chaque manquant en synthétique et on + * recommence, jusqu'à ce que la passe amont passe — ce qui reste est la liste exacte de ce + * que le bundle attend de l'extérieur. Un service **à nous** dans cette liste est un bug. + * + * @return list + */ + private static function servicesManquants(ContainerBuilder $container): array + { + $manquants = []; + $passe = new CheckExceptionOnInvalidReferenceBehaviorPass(); + + for ($i = 0; $i < 100; ++$i) { + try { + $passe->process($container); + + return $manquants; + } catch (ServiceNotFoundException $e) { + $id = $e->getId(); + if (null === $id || \in_array($id, $manquants, true)) { + throw $e; + } + $manquants[] = $id; + $container->register($id, \stdClass::class)->setSynthetic(true); + } + } + + self::fail('la passe de vérification ne converge pas'); + } + + /** + * @param array $config + */ + private function load(bool $debug, array $config = []): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.debug', $debug); + (new DurableExtension())->load([$config], $container); + return $container; } }