Implement a Drupal8ForkingLoop to handle AssertionErrors that get thrown

[damiankloip]

When using Drupal 8 and assertions are enabled using PHP7 (PHP5 we support our own \AssertionError class which does extend Exception), which is very likely in development, our shell cannot handle AssertionErrors. This is most evident when the FokingLoop tries to serialize the container to keep state in the REPL. This is not just the container directly, but E.g. The entity manager has a reference which tries to get serialized too.

Alas, we need to override the whole ForkingLoop implementation that ships with Psysh to catch the exception in the correct place. We then avoid errors like this:

>>> exit
Exit:  Goodbye.
 [error]  AssertionError: The container was serialized. in assert() (line 29 of /Users/damianlee/Sites/D8/core/lib/Drupal/Core/DependencyInjection/Container.php) #0 /Users/damianlee/Sites/D8/core/lib/Drupal/Core/DependencyInjection/Container.php(29): assert(false, 'The container w...')
#1 [internal function]: Drupal\Core\DependencyInjection\Container->__sleep()
#2 ../drush/vendor/psy/psysh/src/Psy/ExecutionLoop/ForkingLoop.php(165): serialize(Object(Drupal\Core\Entity\EntityManager))
#3 ../drush/vendor/psy/psysh/src/Psy/ExecutionLoop/ForkingLoop.php(82): Psy\ExecutionLoop\ForkingLoop->serializeReturn(Array)
#4 ../drush/vendor/psy/psysh/src/Psy/Shell.php(308): Psy\ExecutionLoop\ForkingLoop->run(Object(Drush\Psysh\Shell))
#5 ../drush/vendor/symfony/console/Application.php(124): Psy\Shell->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Psy\Output\ShellOutput))
#6 ../drush/vendor/psy/psysh/src/Psy/Shell.php(273): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Psy\Output\ShellOutput))
#7 ../drush/lib/Drush/Commands/core/CliCommands.php(101): Psy\Shell->run()
#8 [internal function]: Drush\Commands\core\CliCommands->cli(Array)
#9 ../drush/vendor/consolidation/annotated-command/src/CommandProcessor.php(180): call_user_func_array(Array, Array)
#10 ../drush/vendor/consolidation/annotated-command/src/CommandProcessor.php(129): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#11 ../drush/vendor/consolidation/annotated-command/src/CommandProcessor.php(104): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#12 ../drush/includes/annotationcommand_adapter.inc(240): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#13 ../drush/includes/command.inc(199): annotationcommand_adapter_process_command()
#14 ../drush/lib/Drush/Boot/BaseBoot.php(73): drush_dispatch(Array)
#15 ../drush/includes/preflight.inc(93): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#16 ../drush/drush.php(11): drush_main()
#17 {main}.

[damiankloip]

This is really the part we need, the rest is copied from the Psy\ElavuationLoop\ForkingLoop class.

[dawehner]

I'm wondering whether it would be easier to fix that maybe upstream somehow?

[damiankloip]

Yeah, I thought about that first, but Psysh supports PHP5.6 and above. So we would need to add some \AssertionError exception like Drupal does. Which I think might be a no go... at least until it was only support PHP7.

[dawehner]

Are you sure this would actually break in 5.6? PHP is quite flexible in those cases. Damian Lee <notifications@github.com> schrieb am Mi., 22. Feb. 2017 um 10:54 Uhr:

…

***@***.**** commented on this pull request. ------------------------------ In src/Psysh/Drupal8ForkingLoop.php <#2635 (comment)>: > + if ($key === '_' || $key === '_e') { + continue; + } + + // Resources don't error, but they don't serialize well either. + if (is_resource($value) || $value instanceof \Closure) { + continue; + } + + try { + @serialize($value); + $serializable[$key] = $value; + } catch (\Exception $e) { + // we'll just ignore this one... + } + catch (\AssertionError $e) { Yeah, I thought about that first, but Psysh supports PHP5.6 and above. So we would need to add some \AssertionError exception like Drupal does. Which I think might be a no go... at least until it was only support PHP7. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2635 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABz7k3n7pWrDW1o-HkRhV7LLgmR_v9Bks5rfBPmgaJpZM4MIcUv> .

[damiankloip]

Yeah, seems it doesn't actually break PHP. It's still kind of dubious though IMO. I have made an upstream PR anyway bobthecow/psysh#358. Let's see. Otherwise, it's Drupal's fault for implementing this in the container really.

[damiankloip]

Looks like upstream PR is fine! 🎉 Let's leave this one until that is merged.

[dawehner]

Note: maybe drush wants to ensure that the up to date version is actually used by adapting its composer.json file? Damian Lee <notifications@github.com> schrieb am Mi., 22. Feb. 2017 um 16:13 Uhr:

…

Looks like upstream PR is fine! 🎉 Let's leave this one until that is merged. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2635 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABz7nq8upR0gnC8vDJbvZnlafLy7gN6ks5rfF6cgaJpZM4MIcUv> .

[damiankloip]

You'd think that, but composer will still get the latest version (currently 0.8.1)

[dawehner]

Oh nice Damian Lee <notifications@github.com> schrieb am Mi., 22. Feb. 2017 um 18:40 Uhr:

…

You'd think that, but composer will still get the latest version (currently 0.8.1) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2635 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABz7i80UQILXrFgW7zbR9HcWfLiCea3ks5rfIEHgaJpZM4MIcUv> .