Skip to content

Update http_client.rst #14174

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

Closed
wants to merge 2 commits into from
Closed

Conversation

ThomasLandauer
Copy link
Contributor

Integrating the findings of #12522 (comment)

  • I removed the code block cause it didn't show any real code but just explanations. However, if you think it's not clear enough now, I could write another code block showcasing all options.
  • The part in the middle (the one I didn't change) I don't understand ;-) So I left it as is.

Integrating the findings of symfony#12522 (comment)

* I removed the code block cause it didn't show any real code but just explanations. However, if you think it's not clear enough now, I could write another code block showcasing all options.
* The part in the middle (the one I didn't change) I don't understand ;-) So I left it as is.
@ThomasLandauer
Copy link
Contributor Author

ThomasLandauer commented Sep 4, 2020

Hold on, just noticed that this is not true:

The preferred way to handle this is to call $response->getStatusCode(), cause this
will prevent subsequent getHeaders() and getContent() calls from throwing an
exception.

So what's the purpose of getStatusCode() then?

My code throws an exception on the dump line:

if (302 === $response->getStatusCode())
{
    dump($response->getHeaders());
}

@ThomasLandauer
Copy link
Contributor Author

I guess that ->toArray() behaves the same as getHeaders() and getContent()? So it should me mentioned too.

// the response of this request will be a 403 HTTP error
$response = $client->request('GET', 'https://httpbin.org/status/403');
The preferred way to handle this is to call ``$response->getStatusCode()``, cause this
will prevent subsequent ``getHeaders()`` and ``getContent()`` calls from throwing an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will throw all the time (unless false is passed), but that will prevent the destructor from throwing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start with the easy question: I guess that ->toArray() behaves the same as getHeaders() and getContent()? So it should me mentioned too?

Now the hard: So you're saying that getHeaders(true) throws always? But it's not the destructor but some other code part that is issuing the exception (which doesn't make a difference to the user)? So which difference does calling getStatusCode() then make at all?

Sorry, but every time I think I've got it, I realize that I haven't ;-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, toArray also has the $throw argument
All the methods that have this argument throw accordingly, whether you called getStatusCode() or not.
On destruction, if you didn't call getStatusCode(), an exception will be thrown when the status is >= 300.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I still don't get it :-( Here's the code sample from https://symfony.com/doc/4.4/http_client.html#handling-exceptions

// the response of this request will be a 403 HTTP error
$response = $client->request('GET', 'https://httpbin.org/status/403');

// this code results in a Symfony\Component\HttpClient\Exception\ClientException
// because it doesn't check the status code of the response
$content = $response->getContent();

Could you please show me the full code that "because it doesn't check the status code of the response" refers to?

Adding `toArray`
ThomasLandauer added a commit to ThomasLandauer/symfony-docs that referenced this pull request Oct 29, 2020
I started from scratch after symfony#14174

* The most important change is that the old text "your code is expected to handle it" didn't make it clear *how* people are supposed to handle it.
* I omitted the note that a `Symfony\Component\HttpClient\Exception\ClientException` is thrown. Is this important? Or is the existing `Symfony\\Contracts\\HttpClient\\Exception\\HttpExceptionInterface` enough?
@ThomasLandauer
Copy link
Contributor Author

Closing in favor of #14499

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

Successfully merging this pull request may close these issues.

4 participants