[REVIEW] PSR-18 HTTP-Client

[Sara Golemon]

As of today, with a unanimous vote from the working group, we formally begin the REVIEW phase of the proposed PSR-18 (HTTP Client) specification. The proposed specification is in the fig-standards repository at the following locations: 

- Specification:  https://github.com/php-fig/fig-standards/blob/master/proposed/http-client/http-client.md 

- Metadocument:  https://github.com/php-fig/fig-standards/blob/master/proposed/http-client/http-client-meta.md 

During the Review phase, acceptable changes to the specification include wording, typographical fixes, and clarifications. If you feel major changes are necessary, please bring your arguments to the list under this thread. If any major changes are considered, we will return to the Draft phase. 

The Review period will end no sooner than 19 Sep 2018 at 11:59pm.  At that time, if the working group can demonstrate two viable trial implementations, and no need for major changes, I will call for an Acceptance Vote. 

-Sara Golemon

PSR-18 Sponsor

[Oscar Otero]

Hello.

I just saw the proposed specification and would like to congratulate to the working group for the great job. I only have a doubt related with the ClientException. Why does it extends to Throwable? In my mind it makes more sense to extend to Exception, because it represents an exception, not an error or other throwable class. This could also cause confusion in a try/catch that only catches Exception. I’m sure this was discussed and there’s a good reason for that, just I’m curious about it.

Thanks!

Oscar Otero

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/3e454eb9-5088-40fb-b280-1d82725b512c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Woody Gilk]

On Wed, Aug 22, 2018 at 1:59 PM Oscar Otero <oscar...@gmail.com> wrote:
>
> Hello.
> I just saw the proposed specification and would like to congratulate to the working group for the great job. I only have a doubt related with the ClientException. Why does it extends to Throwable? In my mind it makes more sense to extend to Exception, because it represents an exception, not an error or other throwable class. This could also cause confusion in a try/catch that only catches Exception. I’m sure this was discussed and there’s a good reason for that, just I’m curious about it.

The ClientException is an interface, and Throwable is an interface.
Exception is a class and an interface cannot extend a class.

I have to assume this is the reason.
--
Woody Gilk
https://shadowhand.me

>
> Thanks!
> Oscar Otero
>
> El 22 ago 2018, a las 20:43, Sara Golemon <p...@golemon.com> escribió:
>
> As of today, with a unanimous vote from the working group, we formally begin the REVIEW phase of the proposed PSR-18 (HTTP Client) specification. The proposed specification is in the fig-standards repository at the following locations:
>
> - Specification: https://github.com/php-fig/fig-standards/blob/master/proposed/http-client/http-client.md
> - Metadocument: https://github.com/php-fig/fig-standards/blob/master/proposed/http-client/http-client-meta.md
>
> During the Review phase, acceptable changes to the specification include wording, typographical fixes, and clarifications. If you feel major changes are necessary, please bring your arguments to the list under this thread. If any major changes are considered, we will return to the Draft phase.
>
> The Review period will end no sooner than 19 Sep 2018 at 11:59pm. At that time, if the working group can demonstrate two viable trial implementations, and no need for major changes, I will call for an Acceptance Vote.
>
> -Sara Golemon
> PSR-18 Sponsor
>
> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/3e454eb9-5088-40fb-b280-1d82725b512c%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/C5077FA0-7DF9-4763-816D-1BB6ACAF4684%40gmail.com.

[Oscar Otero]

Ah, got it!

And, according with the php docs: PHP classes cannot implement the Throwable interface directly, and must instead extend Exception. 

Thanks for clarification.

    

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAGOJM6KTyb3pYaA0LizsFWfrb-zgASAk0kPDJ_b-3%3DoveEdYjQ%40mail.gmail.com.

[Stefano Torresi]

Hello PSR-18 WG,

According to our Naming Conventions bylaw, section 1: Interfaces MUST be suffixed by "Interface".
I know that this style has been contested before, but we have an established convention in place, so we should stick to it.

Anyone who doesn't like this can circumvent it via import aliases (which I do all the time, by the way).

On a different, opinionated, note, I don't particularly like the main method signature: public function sendRequest(RequestInterface $request): ResponseInterface;

The resulting consumer code would look like:

$client->sendRequest($request);

which IMHO is redundant and harder to read; I would much more prefer just:

$client->send($request);

The parameter is explicitly typed already, so I don't think that the additional hint to "Request" is needed in the name of the method.

This is obviously a small detail, and it can probably be argued either way; IIRC it has even been discussed already within the WG, but there are no references in the metadoc, so I'd like the WG to either rename the method or add some sound reasoning to the metadoc about the current, more verbose, name.

That's my 2c!  

Thanks for your time,
Stefano

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/41A79104-E56E-4E11-9691-E3D342A9199E%40gmail.com.

[Matthew Weier O'Phinney]

On Sat, Aug 25, 2018 at 4:21 AM Stefano Torresi <ste...@torresi.io> wrote:

Hello PSR-18 WG,

According to our Naming Conventions bylaw, section 1: Interfaces MUST be suffixed by "Interface".
I know that this style has been contested before, but we have an established convention in place, so we should stick to it.

Anyone who doesn't like this can circumvent it via import aliases (which I do all the time, by the way).

That's a good catch, and likely requires we drop back to DRAFT status briefly to address, as it's a substantive change (change in interface name).

On a different, opinionated, note, I don't particularly like the main method signature: public function sendRequest(RequestInterface $request): ResponseInterface;

The resulting consumer code would look like:

$client->sendRequest($request);

which IMHO is redundant and harder to read; I would much more prefer just:

$client->send($request);

The parameter is explicitly typed already, so I don't think that the additional hint to "Request" is needed in the name of the method.

This is obviously a small detail, and it can probably be argued either way; IIRC it has even been discussed already within the WG, but there are no references in the metadoc, so I'd like the WG to either rename the method or add some sound reasoning to the metadoc about the current, more verbose, name.

There is a reason, but evidently it wasn't spelled out in the metadoc, and should be.

It's to allow compatibility with existing clients.

Guzzle and a few others already use `send()`. As such, if they are to adopt this standard, they would need to break backwards compatibility in order to be compatible. By defining `sendRequest()`, we ensure they can adopt without any immediate BC breaks.

Fellow WG members: who would like to document this for a patch to the metadoc? Again, like the above notice about the interface suffix, we'd need to return to DRAFT status and do another vote to enter REVIEW.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAFojS1tOnNsEp6JajY-mEcef4TmWP0zRDnWHmoYYkMWj5eY5NQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Guillaume Messier]

Hi everyone,

Can we have some news about the review process and possibly the acceptance vote? 

I don't want to rush anybody but my team is really impatient to replace our own interface by the PSR-18 one for a library that needs a HttpClient instance to work.
It's the only remaining dependency that doesn't use a PSR interface (thanks to PSR-3, 6, 7, 11, 16 and 17 ... quite a lot).

Oh! And also, good work!

[Sara Golemon]

On Wednesday, October 3, 2018 at 12:11:53 PM UTC-5, Guillaume Messier wrote:

Can we have some news about the review process and possibly the acceptance vote? I don't want to rush anybody but... 

A reasonable, excellent, and timely question!

Just poked Tobias and modulo a small tweak to the meta doc, we think we can open the acceptance vote soon.

Regarding the naming issue with the "Interface" suffix; We're viewing that as a non-substantive change to the proposal to bring it in line with PSR naming requirements and not necessitating a return to draft phase.  See also: https://github.com/php-fig/fig-standards/pull/1069 

My team is really impatient to replace our own interface by the PSR-18 one for a library that needs a HttpClient instance to work.

It's the only remaining dependency that doesn't use a PSR interface (thanks to PSR-3, 6, 7, 11, 16 and 17 ... quite a lot).

That's excellent to hear!  Positive real-world use testimonials mean a lot in terms of believing that we're on the right track.

 

-Sara 

[Steven Maguire]

Hi all, 

Congrats on getting this out in the wild. It's great to see this PSR join the other HTTP PSRs to help standardize HTTP communication. I am aware that I am late with any feedback, so perhaps this is just a question for clarification. 

I am curious why the RequestExceptionInterface does not also include a getResponse method. Unless I am missing something (or likely some conversation elsewhere) it seems that if the request failed after connecting with a server that there would be a response. That response might include an error status code and perhaps a body, yeah? It seems, at least to me, very practical to expect that a Calling Library would likely use this information to handle the failed response.

Steven

[Stefano Torresi]

Hello Steven,

I am curious why the RequestExceptionInterface does not also include a getResponse method. Unless I am missing something (or likely some conversation elsewhere) it seems that if the request failed after connecting with a server that there would be a response. 

Nope, that's not what it means. Here is an abstract from https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-18-http-client.md#error-handling:

A Client MUST throw an instance of Psr\Http\Client\ClientExceptionInterface if and only if it is unable to send the HTTP request at all or if the HTTP response could not be parsed into a PSR-7 response object.

If a request cannot be sent because the request message is not a well-formed HTTP request or is missing some critical piece of information (such as a Host or Method), the Client MUST throw an instance of Psr\Http\Client\RequestExceptionInterface.

So, in a nutshell, the exception means exactly that there is no response.

Cheers,
Stefano

[Steven Maguire]

Hey Stefano, 

Thanks for the response and yes, that does make sense. Maybe I was staring at the screen too long yesterday...

Steven