Skip to content
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

[VarDumper] 2.6.7 dump() breaks all API Doc platforms and Profiler #14608

Closed
MichaelHindley opened this issue May 11, 2015 · 16 comments
Closed

Comments

@MichaelHindley
Copy link

#14478

Used to be that you could call dump() and check the profiler without having a broken api response, with this change that is completely broken now. Not only do you get html in your response when the content-type is set to json , but the request is missing from the profiler so you can't even debug it properly.

For example, you can't dump() and check the events or doctrine queries for a RESTful request anymore, is this really intended behaviour?

Also in some cases the output of dump() is way too big to be handled in a api doc interface such as Swagger or NelmioApiDoc and breaks the browser where you could just view the profiler otherwise and selectively choose among the various dump statements.

The VarDumper was a easy way to do debug things in a RESTful environment, now it's completely useless in that regard. Using rest clients (such as in phpstorm) and having it give you the corresponding profiler automatically was such a great feature, this no longer works if you want to do any debugging at the same time.

@fabpot
Copy link
Member

fabpot commented May 11, 2015

ping @nicolas-grekas

@nicolas-grekas
Copy link
Member

I'd say this is a variant of #13915 and #14373: we all want more control over the destination (and thus the format) of the dumps. Now with 4 different expectations for doing this, I have to think about a common solution...

@jaltamir
Copy link

This new behaviour also breaks AJAX requests. For example, it's no longer possible to debug an ajax request for datatables.

It would be great to allow old behaviour.

@nicolas-grekas
Copy link
Member

@NihilNovi @jaltamir could you please tell me if #14609 at least partially fixes the issue?

@jaltamir
Copy link

@nicolas-grekas It works

@MichaelHindley
Copy link
Author

@nicolas-grekas #14609 does partially work, I don't think X-Requested-With is in the headers on xdomain requests though. I can see the same behavior on those, for instance in a virtual environment setup that utilizes CORS.

@nicolas-grekas
Copy link
Member

Additionally checking for the Origin header should be the right fix then

@MichaelHindley
Copy link
Author

On further testing, neither NelmioApiDoc or Swagger works with the changes in #14609 since they don't actually send AJAX requests in their API interfaces.

@nicolas-grekas
Copy link
Member

See comments in #14609

nicolas-grekas added a commit that referenced this issue May 13, 2015
This PR was merged into the 2.6 branch.

Discussion
----------

[DebugBundle] Remove inlined dumps on XHR

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | partially #14608
| License       | MIT
| Doc PR        | -

Commits
-------

d0eb208 [DebugBundle] Remove inlined dumps on XHR
@nicolas-grekas
Copy link
Member

@NihilNovi can you please try #14640, setting debug.dump_destination to php://stderr or /dev/null

fabpot added a commit that referenced this issue May 15, 2015
…olas-grekas)

This PR was merged into the 2.6 branch.

Discussion
----------

[DebugBundle] Allow alternative destination for dumps

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #14608
| License       | MIT
| Doc PR        | -

This PR fixes #14608 by adding a new `debug.dump_destination` configuration option to give users control over a stream destination where dumps will be written to.
In HTTP mode, dumps are still/also collected in the toolbar.
This supersedes #14627, #14631 and #14373

Commits
-------

5f255e5 [DebugBundle] Allow alternative destination for dumps
5368483 [DebugBundle] Use output mechanism of dumpers instead of echoing
8cb2abb [DebugBundle] Always collect dumps
@fabpot fabpot closed this as completed May 15, 2015
@MichaelHindley
Copy link
Author

@nicolas-grekas I will do it tomorrow, unusually hectic week, sorry for my unresponsiveness :)

@teohhanhui
Copy link
Contributor

If you use Postman for testing your API, dump still does not work as expected. Is there any workaround?

@nicolas-grekas
Copy link
Member

Did you enable the DebugBundle and use the mentioned config option? Otherwise please open an issue describing your use case. Thanks

@teohhanhui
Copy link
Contributor

Doesn't the config option debug.dump_destination take effect everywhere dump is being called, including the CLI and where the content type is actually HTML? That doesn't seem like an ideal workaround. (Because who would want to turn that config option on and off before doing dump?)

This issue is about us trying to be smart and avoid doing inline dumps when we shouldn't. I'm just pointing out a use case (a completely "normal" HTTP request, but which response content type is not HTML; you don't have to use Postman for this, you might as well just be loading the URL directly from the browser) where it's still broken. Perhaps we can add a second argument to dump to avoid inline dumps (just an idea)...

@teohhanhui
Copy link
Contributor

After looking at #14478 I'd argue that it's wrong. Why do inline dumps where it'll break the response, when the dumps can still be collected by the profiler?

@nicolas-grekas
Copy link
Member

#14478 fits its own reasoning, behavior and usefulness. If it doesn't fit your use case, you can use debug.dump_destination to unconditionally dump somewhere else (being CLI, HTML, or whatever context). Or you can use your own custom configuration. Maybe we can help, but for that, you'd need to open an issue (comments in closed issues can't be tracked correctly), and you'd need also to describe your use case precisely. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants