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

Does not handle like the native Laravel HTTP Client watcher #2

Open
ZacharyDuBois opened this issue Sep 21, 2023 · 2 comments
Open

Comments

@ZacharyDuBois
Copy link

Hi,

First off, this is a life saver for working with third party libraries that interact with APIs. Thank you!

Just something I noticed, it puts query params in the payload section instead of in the URL.

Below is an example from Laravel's HTTP client wraper:

    Http::withQueryParameters(['foo' => 'bar'])->asForm()->post('http://example.com', [
        'alice' => 'bob',
    ]);
Screenshot 2023-09-21 at 09 31 42

And this is that same request done with this library:

    $client = app(\GuzzleHttp\Client::class);

    $client->post('http://example.com', [
        'form_params'  => [
            'alice' => 'bob',
        ],
        'query' => [
            'foo' => 'bar',
        ],
    ]);
Screenshot 2023-09-21 at 09 36 15

I also see the additions that were attempted to resolve some minor short comings of the native watcher but having separated "redact" configuration from the native Telescope ones is kind-of redundant and violates DRY and KISS. They are already defined within the Telescope service providers and should be used.

If I get any free time, I'll contribute back to this as this is a super super nice package to have.

@huzaifaarain
Copy link
Owner

Hi @ZacharyDuBois

Thank you for the compliment, much appreciated.

The reason behind putting the query params into the payload section is to have better readability, I have worked with some API that has a lot of query parameters and it is difficult and time-consuming for me (if not for others) to look at the URI string and parsing the query params on the fly while looking at it.

Can you please elaborate on the following or at least point me to where I am violating the DRY or KISS?

Thank you again.

@ZacharyDuBois
Copy link
Author

ZacharyDuBois commented Sep 21, 2023

I assume you are referring to OData :) That is what I am dealing with right now. Yeah it'd be nice to have both the native HTTP watcher and this library be able to have a separate tab or space for query params. Maybe an option to flip between the two (ie; native like experience vs customized experience).

As for the other misc behavior I've noticed, form requests do not break out correctly like the native watcher. Puts them all into one line, still URL encoded.

The KISS/DRY is you have separated the configs for filtering/redacting parameters. So if I set within the service provider the following:

        Telescope::hideRequestParameters([
            '_token',
            'signature',
            'access_token',
            'refresh_token',
        ]);

        Telescope::hideRequestHeaders([
            'cookie',
            'x-csrf-token',
            'x-xsrf-token',
            'access_token',
            'refresh_token',
        ]);

        Telescope::hideResponseParameters([
            'access_token',
            'refresh_token',
        ]);

But these appear to have no affect on anything happening with the saved telescope entry. Looking into the config, I need to respecify all these values in a separate area (which could lead to simple mistakes of updating in two places).

Again, more so opinionated, but I will attempt to come up with some stuff when I have time.

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

No branches or pull requests

2 participants