Problem/Motivation
The trigger_error call in the Drupal\Core\DependencyInjection\Container::__sleep method causes problems with the drush shell (php) command. We now use PsySH (!) instead. It attempts to generically save state and serialize the scoped variables. It works on the assumption that it will catch all exceptions and eat them. Discarding any serialization that fails. I think that's a fair assumption.
Before any asks, I have looked at fixing this on the PsySH and drush side, we would need to copy the loop handlers from PsySH and maintain own own versions, or try to get an upstream PR in to turn private methods into protected methods, or a blacklist. Both of which I think as unlikely.
Proposed resolution
If we convert this to use a runtime assertion, we can control this and use the handler that throws an exception from #2536560: Runtime Assertion unit and functional testing. We then get the similar behaviour - this can be supressed on production environments for example. But using an exception (added by the assert_options ASSERT_CALLBACK handler over there) we can just rely on exceptions instead and everyone is happy.
Remaining tasks
Wait on #2536560: Runtime Assertion unit and functional testing
User interface changes
N/A
API changes
I guess kind of, in that this will depend on assertions being enabled or disabled instead of trigger_error.
Data model changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff-2550641-10.txt | 1.24 KB | damiankloip |
#10 | 2550641-10.patch | 2.41 KB | damiankloip |
#7 | interdiff-2550641-7.txt | 596 bytes | damiankloip |
#7 | 2550641-7.patch | 1.18 KB | damiankloip |
d8.container-sleep-assert.patch | 609 bytes | damiankloip | |
Comments
Comment #2
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #4
stefan.r CreditAttribution: stefan.r commented#2536560: Runtime Assertion unit and functional testing is now in
Comment #5
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedAwesome!
Comment #6
dawehnerI can haz test coverage?
Comment #7
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedWell, this kind of already has it, PHPUnit exception is caught, which I guess is kind of a hack?
Comment #8
stefan.r CreditAttribution: stefan.r commentedShould we do this in ContainerBuilder as well?
Comment #9
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedYes, sounds like a good idea!
Comment #10
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commentedComment #11
dawehnerNice!
Comment #12
webchickCommitted and pushed to 8.0.x. Thanks!
Comment #14
damiankloip CreditAttribution: damiankloip at Tag1 Consulting commented