saloonphp / saloon

🤠 Build beautiful API integrations and SDKs with Saloon
https://docs.saloon.dev
MIT License
2.03k stars 105 forks source link

Feature | Debug Helper Method #387

Closed Sammyjo20 closed 5 months ago

Sammyjo20 commented 5 months ago

(Updated description)

This PR introduces a new and exciting way to debug Saloon requests! We previously had the debugRequest and debugResponse methods on the connector/request however they required you to define your own callback to debug with, which was cumbersome.

Let's take a look at an example of dumping out the PSR request's raw request body:

$connector->debugRequest(function (PendingRequest $pendingRequest, MessageInterface $psrRequest) {
    dd($psrRequest->getBody()->getContents());
});

That's a lot of code just for debugging! This PR now makes the callable argument nullable and provides a nice default implementation to quickly debug requests and responses. This is all you have to write now:

$connector->debugRequest()->send($request);

Much cleaner! It will output the raw request in an array format using Symfony's Var Dumper library.

Saloon Request (AlwaysThrowRequest) -> array:6 [
  "connector" => "Saloon\Tests\Fixtures\Connectors\TestConnector"
  "request" => "Saloon\Tests\Fixtures\Requests\AlwaysThrowRequest"
  "method" => "GET"
  "uri" => "https://tests.saloon.dev/api/error"
  "headers" => array:2 [
    "Host" => "tests.saloon.dev"
    "Accept" => "application/json"
  ]
  "body" => ""
]

Responses

But wait, there's more! I have also provided a default implementation for debugging responses. This works in exactly the same way:

$connector->debugResponse()->send($request);
Saloon Response (UserRequest) -> array:3 [
  "status" => 200
  "headers" => []
  "body" => "{"name":"Sam"}"
]

Combined

Most of the time you want to write an even shorter method and get both the request and response at the same time. You can now use a new debug() method.

$connector->debug()->send();
Saloon Request (UserRequest) -> array:6 [
  "connector" => "Saloon\Tests\Fixtures\Connectors\TestConnector"
  "request" => "Saloon\Tests\Fixtures\Requests\UserRequest"
  "method" => "GET"
  "uri" => "https://tests.saloon.dev/api/user"
  "headers" => array:2 [
    "Host" => "tests.saloon.dev"
    "Accept" => "application/json"
  ]
  "body" => ""
]
Saloon Response (UserRequest) -> array:3 [
  "status" => 200
  "headers" => []
  "body" => "{"name":"Sam"}"
]

[!NOTE] All of these are available on the request instance too.

Dump & Die

When debugging a common practice is to exit the application after dumping out the useful information. All of the methods now have a "die" argument which can be provided to exit the application.

$connector->debugRequest(die: true);
$connector->debugResponse(die: true);
$connector->debug(die: true);

Todo

Sammyjo20 commented 5 months ago

Random idea: Call it yeehaw() and print a big YEEHAW before a request is being sent?

Screenshot 2024-03-18 at 23 28 09
Sammyjo20 commented 5 months ago

Now with better styling

Screenshot 2024-03-18 at 23 45 55
ClaraLeigh commented 5 months ago

Just replying here instead of twitter.

So what I mean by my suggestion was to update the HasDebugging Trait to have two new functions:


trait HasDebugging
{
    public function dump()
    {
        return debug($this);
    }

    public function dd()
    {
        return debug($this, true);
    }

    ............

So instead of importing it when used, we could do:

$connector->send($request->dump());

Side note in terms of the code itself. I think laravel Octane has a problem with exit, from memory it causes it to crash octane? Can't remember the specifics but I know I've run into it a few times when debugging things

Sammyjo20 commented 5 months ago

I actually really love that idea. We already have the HasDebugging trait so I can add to it, maybe I just need some heavy docs to make sure people don't get confused and when they call it, expect something to happen?

ClaraLeigh commented 5 months ago

I think as long as its documented it'll be fine. People always reach for the docs when trying to debug things like this anyway

This however might make it easy enough to remember for next time instead of having to look it up again (like I always do)

Sammyjo20 commented 5 months ago

I've added it to the trait, is that what you were thinking? I'm still considering it - I might remove it if I feel it could clash too much with Laravel

Sammyjo20 commented 5 months ago

What do you guys think about it also including the response too?

Screenshot 2024-03-19 at 00 51 30
ClaraLeigh commented 5 months ago

Having both would be perfect and would have saved me a lot of time in the past.

Might be counter intuitive having it on the request instead of the connection but thats minor in comparison to the benefit you get here

Sammyjo20 commented 5 months ago

Okay so I've made some more changes - now we have on the connector/request the following:

$connector->debug($die = false); // Outputs request and response
$connector->debugRequest(); // Outputs request (no die)
$connector->debugRequest($closureHandler); // Old functionality
$connector->debugResponse(); // Outputs response (no die)
$connector->debugResponse($closureHandler); // Old functionality

Not even sure if we need the debug() helper if we have this

ClaraLeigh commented 5 months ago

That looks neat and very usable. Likely easy to remember too

Sammyjo20 commented 5 months ago

Thank you for the help with this!

michaeldyrynda commented 5 months ago

What do you guys think about it also including the response too?

If you debug the connector, include as much as possible for the lifecycle.

If you debug the request, it’s typically because I want to see what is being sent, though I can see a scenario where seeing the response (that would otherwise end up triggering the exception handling), would be helpful

Sammyjo20 commented 5 months ago

Hey @ClaraLeigh @michaeldyrynda I have updated the description at the top with the final changes. I have also removed the helper.php file as well as the debug() helper method as I don't think it's needed with the methods described above. Let me know your final thoughts on this! I'll need to write some tests but I'm very pleased with this implementation and I think it's going to be instantly useful.

boryn commented 5 months ago

Hi @Sammyjo20!

Thanks for simplifying debugging!

And what about "entering a debug mode" as using cache or not? For me $connector->debug()->send(); is still too cumbersome ;) If we had something like $this->debugMode we would just with one method like enableDebugMode() display all debugging info, without the need to change the core code ($connector->send();) in different places. Maybe $this->debugMode could be string accepting 'both' / 'request' / 'response'?

PS. Will the old way of debugging be still active? To keep it backwards compatible?

ash-jc-allen commented 5 months ago

Hey @Sammyjo20! I'm loving the newer version of the debugRequest() method. I think it'd be relatively easy for other developers to discover it too with IDE autocomplete. So I'm all for it 😄

Sammyjo20 commented 5 months ago

Hey @boryn thank you for taking a look at this although I don't think I can get it too much simpler than ->debug() 😅! I think if you wanted to debug without adding the method, you could add it via the constructor? I will consider the property or maybe adding a trait.

public function __construct()
{
    $this->debug();
}

Yes the previous functionality will remain 🙌

ClaraLeigh commented 5 months ago

These changes saved me a lot of time this week doing two new api integrations. Very helpful and easy to remember, ty

Sammyjo20 commented 5 months ago

Woohoo! That’s great 😀 thank you for letting me know!