Skip to content

[VarDumper] Add VAR_DUMPER_FORMAT=server format #35967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 13, 2020
Merged

[VarDumper] Add VAR_DUMPER_FORMAT=server format #35967

merged 1 commit into from
Aug 13, 2020

Conversation

ogizanagi
Copy link
Contributor

@ogizanagi ogizanagi commented Mar 5, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #35801
License MIT
Doc PR symfony/symfony-docs#14067

This PR follows discussion in #35801 and adds support for a server value for the existing VAR_DUMPER_FORMAT env var.

It comes as well with two more things:

  • the handler is registered as soon as the VAR_DUMPER_FORMAT env var is detected we prevent registering another handler as soon as the VAR_DUMPER_FORMAT env var is set, instead of checking if there was a previous handler (which could make this env var useless in some conditions where the handler was already set by another "process")
  • the handler registered this way cannot be replaced. This prevents another "process" to takeover dump handling while undesired. E.g: a phpunit functional test booting the kernel to call an endpoint => the handler is replaced. It's (in a sense) a satisfying alternative to [VarDumper][PhpUnitBridge] VarDumperServerListener #26696

This PR means anyone can use dump with a server in any context, without changing a single line of code in the project by:

  • starting the server using ./vendor/bin/var-dump-server --format=html > dumps.html
  • using the env var: VAR_DUMPER_FORMAT=server [your-cli-command]

Previous related PRs:

@ogizanagi ogizanagi added this to the next milestone Mar 6, 2020
@ogizanagi
Copy link
Contributor Author

@nicolas-grekas : I've updated the PR, so it should work now using .env files or phpunit.xml

@@ -5,6 +5,8 @@ CHANGELOG
-----

* added `RdKafka` support
* added `VAR_DUMPER_FORMAT=server` support
* prevent replacing the handler when the `VAR_DUMPER_FORMAT` env var is set
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw what about considering this behavior as a bug fix? Getting something else than the expected format when using VAR_DUMPER_FORMAT looks unexpected to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this? Bug fix or not bug fix?

@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@xabbuh xabbuh modified the milestones: 5.1, next May 5, 2020
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me this way. Here are some ideas to go a bit further. WDYT?

@fabpot
Copy link
Member

fabpot commented Aug 13, 2020

Thank you @ogizanagi.

@fabpot fabpot merged commit ab3b0c9 into symfony:master Aug 13, 2020
@ogizanagi ogizanagi deleted the var-dumper/server-format-env-var branch August 13, 2020 08:59
fabpot added a commit that referenced this pull request Aug 16, 2020
…RMAT (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

[VarDumper] Backport handler lock when using VAR_DUMPER_FORMAT

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | N/A <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | N/A

Backport of the "lock" behavior from #35967, preventing unexpected handler change when using the `VAR_DUMPER_FORMAT` env var.

---

As a concrete example of this not working as expected:
Start a PHPUnit test suite with this env var to the desired format. If a web test case is making a request (with debug enabled), the `DebugBundle` replaces the handler set initially by the `VAR_DUMPER_FORMAT`, will collect dumps into the profiler, but won't output these.
As well, for dumps made in between the the kernel is boot (`DebugBundle::build()` is called) and a request, the dumps are properly sent to the output with expected format, but looses colors.

IMHO, the use-cases of `VAR_DUMPER_FORMAT` justifies locking the handler for the whole process.

Commits
-------

19b341e [VarDumper] Backport handler lock when using VAR_DUMPER_FORMAT
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
@fabpot fabpot mentioned this pull request Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[VarDumper] Add ddf() and dumpf() functions to dump to a file
6 participants