[DebugBundle] Fix dump/die doesn't work with server dumper #27397
Conversation
|
I did not read the code, but:
|
| public function dump(Data $data) | ||
| { | ||
| // only delay web requests | ||
| if (!\in_array(PHP_SAPI, array('cli', 'phpdbg'), true)) { |
There was a problem hiding this comment.
I would use if (\PHP_SAPI === 'cli' || \PHP_SAPI === 'phpdbg'), which can be resolved by OPCache at compile time
| $response = $event->getResponse(); | ||
| $request = $event->getRequest(); | ||
|
|
||
| // In all conditions that remove the web debug toolbar, dumps must be written on the output. |
There was a problem hiding this comment.
why would this service check the debug toolbar being injected or no ? This class is not related to the data collector AFAICT
There was a problem hiding this comment.
You're right it's not directly related to the data collector, but this whole patch aims to ensure the exact same behavior exists whether the server dumper is wired or not.
When not wired and the toolbar being injected, dumps are already rendered in the toolbar and considered not to be output twice by the collector.
|
After some discussion, closing in favor of #27614 with some compromises to avoid BC breaks. |
… of Dumper/ServerDumper (nicolas-grekas) This PR was merged into the 4.1 branch. Discussion ---------- [VarDumper] Fix dumping by splitting Server/Connection out of Dumper/ServerDumper | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27622 | License | MIT | Doc PR | - Right now, the `dump()` function is broken on 4.1 as soon as one sets up a `dump_destination` for the dump server (as done by default by our Flex recipe). #27397 describes the issue and proposes a tentative fix. Yet, I think the issue is deeper and exists at the design level. Writting to the server should not happen in a `DumperInterface`, that's not its semantics. Instead, I propose a `Connection` object that will allow `DumpDataCollector` to have all the info it requires to do everything on its own. My bad for not spotting this at the review stage. Commits ------- 1435d67 [VarDumper] Fix dumping by splitting Server/Connection out of Dumper/ServerDumper
This is an attempt to fully respect the promise made by the server dumper feature, i.e:
dumpshould act exactly the same when theServerDumperis wired (debug.dump_destination: "tcp://%env(VAR_DUMPER_SERVER)%") but the dump server is not up (server:dumpnot running).Currently, when using the full stack and debug bundle, the promise is broken, as someone using
ddordump+diewould not see expected dumps in the output for web requests. This is explained by the wrapped dumper (the one used byServerDumperwhen failing at contacting the server) being thevar_dumper.cli_dumper, so it outputs onphp://stdoutinstead (which is fine for cli usages).To reproduce
composer create-project "symfony/website-skeleton:^4.1@dev" no-dd-outputddinsideWhat this solution is worth
It works, but is far from ideal. It reproduces the
DumpDataCollectorbehavior (code is entirely borrowed from here) in a dedicated internal dumper used as the fallback when we failed at contacting the server. Also, it misses the file & line nb usually added by the collector in the output (or would also need duplicating this logic). => Now addedIdeally, the
ServerDumper's wrapped dumper should beDumpDataCollectorbut it's not easily achievable as those are already quite intricated/tangled with each other in the way they are wired together. I failed to find a proper solution so far.=> See #27614 (comment) last paragraph for a strategy extracting, deprecating & removing some code from DumpDataCollector, limiting it to its responsibility of collecting dumps to be shown on toolbar/profiler, in favor of the
DebugDumper(attempt in progress).Reverting #26675 makes things work, but the fix is legit and it would only rely on undesired behavior while still using the cli dumper as the wrapped dumper (thus leading to duplicates at the end of the process when one is calling a controller, e.g when executing a test suite).