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

Improve the HttpClient panel in the Web Profiler #33311

Closed
nicolas-grekas opened this issue Aug 23, 2019 · 16 comments
Closed

Improve the HttpClient panel in the Web Profiler #33311

nicolas-grekas opened this issue Aug 23, 2019 · 16 comments
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. HttpClient Keep open

Comments

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Aug 23, 2019

PR #33015 introduces a profiler panel for the HttpClient component.
It was decided to merge it and improve it in nex iterations.

Here are the ideas that would be great to contribute, help wante:

@nicolas-grekas nicolas-grekas added HttpClient Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. labels Aug 23, 2019
@Neirda24
Copy link
Contributor

To improve DX, what if we parse the headers in the response and detect the debug token link to display it. WDYT ?

@stof
Copy link
Member

stof commented Aug 26, 2019

@Neirda24 that's indeed a good idea.

@k0d3r1s
Copy link
Contributor

k0d3r1s commented Sep 25, 2019

i have a guzzle collector but I couldn't fit httpclient datacollector data into it. how much is it possible to change what this collector already does and how it collects data?

Screenshot at 2019-09-25 19-24-41
Screenshot at 2019-09-25 19-25-18

@fabpot fabpot closed this as completed Jan 22, 2020
fabpot added a commit that referenced this issue Jan 22, 2020
…n the Web Profiler -> Http Client panel (cristagu)

This PR was squashed before being merged into the 5.1-dev branch (closes #35391).

Discussion
----------

[WebProfilerBundle][HttpClient] Added profiler links in the Web Profiler -> Http Client panel

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | [#33311](#33311)
| License       | MIT
| Doc PR        | -

As per @Neirda24's idea in this [comment](#33311 (comment)), I parsed the response headers collected by the TraceableHttpClient and created profiler links in the HttpClient Web Profiler panel for all the requests having the 'X-Debug-Token' header.

![profiler](https://user-images.githubusercontent.com/26301369/72686050-19a91300-3af9-11ea-998b-db063a3aa358.PNG)

Commits
-------

70e11f9 [WebProfilerBundle][HttpClient] Added profiler links in the Web Profiler -> Http Client panel
fabpot added a commit that referenced this issue Jan 22, 2020
…le (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[HttpClient] collect the body of responses when possible

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Part of #33311
| License       | MIT
| Doc PR        | -

This is missing one thing: the HTML part in the profiler.

![image](https://user-images.githubusercontent.com/243674/72798816-29813e00-3c44-11ea-9586-99c2c6b91640.png)

![image](https://user-images.githubusercontent.com/243674/72798851-3f8efe80-3c44-11ea-973b-7ecc64a5a542.png)

Commits
-------

121f728 [HttpClient] collect the body of responses when possible
@nicolas-grekas nicolas-grekas reopened this Feb 3, 2020
nicolas-grekas added a commit that referenced this issue Feb 3, 2020
This PR was merged into the 5.1-dev branch.

Discussion
----------

[WebProfiler] Improve HttpClient Panel

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | np    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #33311  <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | ?

Wishlist from issue :

- [x] the panel UI could be improved I suppose, e.g. a tooltip on hover in the toolbar
- [ ] putting requests in the "Performance" tab would be great (all timings are already in the collected "info")
- [ ] ~response bodies are not collected yet~ #33315 (comment)
- [ ] when a transport error occurs, (ie the "error" info is populated) we should maybe have a better display too?
- [x] add a copy-as-curl button next to each request

**[EDIT 07 oct 19]**
This PR contains :
- Display a tooltip in Toolbar
- Add a link only for GET Request in profiler panel
![image](https://user-images.githubusercontent.com/13260307/66316799-389be480-e910-11e9-888e-7703c87c7b10.png)

Commits
-------

e2e6bd0 [WebProfiler] Improve HttpClient Panel
@derrabus
Copy link
Member

add a copy-as-curl button next to each request

I could really use that feature now. 🙈

@derrabus
Copy link
Member

Another idea for the profiler: If the API answers in JSON, we could try to parse and pretty-print it. Right now, I receive a string response_content that is a little hard to read.

@Neirda24
Copy link
Contributor

Maybe with a two tabs solution ? One RAW and one PARSED. It could be JSON or XML

@michaelKaefer
Copy link
Contributor

Info: since it got the labels Good first issue and Help wanted I start to work on the remaining tasks.

@michaelKaefer
Copy link
Contributor

I hope someone can guide me on implementing this task: putting requests in the "Performance" tab would be great (all timings are already in the collected "info")

The times in the "Performance" tab are measured by calling Stopwatch::start() and Stopwatch::stop() (so I suppose this has to be done for every request and the collected "info" does not help here).

The Stopwatch::start() can be called in TraceableHttpClient::request() or somewhere in TraceableResponse. But where do we know that the response is complete so that we can call Stopwatch::stop()? I tried to use the on_progress callback but that does not work since we never know if it is the last call of the callback. I also tried Response::getContent() but that does not work if the client code never calls $response->getContent().

@erickr
Copy link
Contributor

erickr commented Oct 26, 2020

@michaelKaefer did you get going on this? I'm also interested to learn more, perhaps we can dig together if you want some assistance or sparring of course.. 😎

@jderusse
Copy link
Member

@michaelKaefer , @erickr the StopWatch has already been implemented #38688

@michaelKaefer
Copy link
Contributor

So I am out here. Current state:

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@derrabus
Copy link
Member

Not yet, @carsonbot!

@carsonbot carsonbot removed the Stalled label Apr 28, 2021
@ankahla
Copy link

ankahla commented Sep 28, 2021

As menstionned here #33311 (comment) guzzle output the request headers (including default headers in configuration).
@nicolas-grekas even with this PR #35407 we still not collecting all headers (headers defined in configuration are missing).
I think we need to expose defaultHeaders and get them from HttpClientTrait directly.

@abdellahrk
Copy link

Hello devs, any issue here still not solved yet? I'll like to jump in.
Thank you.

fabpot added a commit that referenced this issue Mar 9, 2022
…equest as a cURL command (Deuchnord)

This PR was squashed before being merged into the 6.1 branch.

Discussion
----------

[HttpClient][WebProfilerBundle] Add button to copy a request as a cURL command

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

Adding a button to each request in the HttpClient section to copy it as a `curl` command that can be then pasted either in the terminal (on Linux and Mac) or in an application that can parse it (like Insomnia).

Work in progress:
- [x] Make a first PoC of the feature
- [x] Generate the `curl` command
- [x] Added some UX considerations to the button
- [x] Update the tests

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Never break backward compatibility (see https://symfony.com/bc).
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too.)
 - Features and deprecations must be submitted against branch 5.x.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
-->

Commits
-------

67c9146 [HttpClient][WebProfilerBundle] Add button to copy a request as a cURL command
@nicolas-grekas
Copy link
Member Author

I think we're done here, no need to keep this issue open. PR welcome of course if you have ideas to improve the panel!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. HttpClient Keep open
Projects
None yet
Development

No branches or pull requests