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

[9.x] Add new throw helper methods to the HTTP Client #45704

Merged
merged 6 commits into from
Jan 19, 2023

Conversation

WendellAdriel
Copy link
Contributor

@WendellAdriel WendellAdriel commented Jan 18, 2023

The goal is to add four new methods to the HTTP Client:

$response = Http::post(/* ... */);

$response->throwIfStatus(500); // Only throws an exception if the HTTP response code is 500

$response->throwUnlessStatus(200); // Throws an exception if the HTTP response code is not 200

$response->throwIfClientError(); // Only throws an exception if the HTTP response code is >= 400 and < 500

$response->throwIfServerError(); // Only throws an exception if the HTTP response code is >= 500

Sometimes we want to only throw an exception in our application for specific codes of the response, so these method add an easy way to accomplish that.

The throwIf method only throws the exception if two conditions are true: The condition passed resolves to true AND if the HTTP response code is >= 400

…ific status code is returned from the HTTP response
@ankurk91
Copy link
Contributor

This type of helper (alias) methods are making the framework bulky and increasing learning curve.

$response->throwIf($callback)

is already sufficient.

@WendellAdriel
Copy link
Contributor Author

I managed to work on a simpler approach for the implementation that I wanted.
Now it's ready!

@ankurk91 I understand your point, but the methods I'm adding are to decrease work for the developers. It's a syntax sugar to make it easier to throw exceptions based on the response HTTP status code.

@WendellAdriel WendellAdriel changed the title [9.x] Add throwIfStatus method to HTTP Client [9.x] Add throwIfStatus and throwUnlessStatus methods to the HTTP Client Jan 18, 2023
@WendellAdriel WendellAdriel marked this pull request as ready for review January 18, 2023 12:43
@WendellAdriel
Copy link
Contributor Author

If this has any chances to be approved and merged and additional work is needed for docs or something like that, just let me know that I'll be happy to work on the needed updates as well 😉 💪🏼

@WendellAdriel
Copy link
Contributor Author

WendellAdriel commented Jan 18, 2023

@ankurk91 I think that I was not clear about my explanation for this implementation. I'll try to explain it a little better:

The throwIf method only throws the exception if two conditions are true: The condition passed resolves to true AND if the HTTP response code is >= 400

These methods I added are for only throwing an exception when the HTTP response code is the one passed to the throwIfStatus method OR throwing an exception for all other HTTP response codes except the one passed to the throwUnlessStatus method.

So they are for different purposes than the throwIf method.

@ankurk91
Copy link
Contributor

Thanks, you gave me a reason.

@taylorotwell
Copy link
Member

What if you want to throw if the status is any 500 level status code?

@WendellAdriel
Copy link
Contributor Author

@taylorotwell we can do it on two ways. Adding additional parameters to the methods I created or create two other ones called throwIfClientError for 40X codes and throwIfServerError for 50X codes. What do you think?

@WendellAdriel WendellAdriel changed the title [9.x] Add throwIfStatus and throwUnlessStatus methods to the HTTP Client [9.x] Add new throw helper methods to the HTTP Client Jan 19, 2023
@WendellAdriel
Copy link
Contributor Author

@taylorotwell I added a new commit adding the two methods I mentioned:

  • throwIfClientError
  • throwIfServerError

If you feel like it's needed I can add four more methods:

  • throwIfStatusIn that will receive an array of codes to throw an exception
  • throwIfStatusBetween that will receive two params and throw an exception for all codes between both of them
  • throwUnlessStatusIn that will be the opposite of the throwIfStatusIn
  • throwUnlessStatusBetween that will be the opposite of the throwIfStatusBetween

If you think that's not needed or that I can work on those other four methods on a new PR it's all good for me. I'll be happy to help in all cases. 😉 💪🏼

@taylorotwell
Copy link
Member

Added support for callables:

$this->factory->get('http://foo.com/api/400')->throwIfStatus(fn ($status) => $status === 500);

@taylorotwell taylorotwell merged commit 80a66e6 into laravel:9.x Jan 19, 2023
@WendellAdriel
Copy link
Contributor Author

Added support for callables:

$this->factory->get('http://foo.com/api/400')->throwIfStatus(fn ($status) => $status === 500);

@taylorotwell amazing addition, I didn't think of this at first! Thanks for all the help on getting this merged! 💪🏼

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

Successfully merging this pull request may close these issues.

3 participants