Description
First, I'm not sure what the design goal behind this component is, and I acknowledge that I did not take part in any prior discussions so I apologize in advance if I raise some concerns that have already be discussed.
That being said, I would argue that the goal of any HTTP client implementation that is called "HTTP Client" (and not, for example, "JsonAwareHttpClient") is to implement the protocol and stick to it so that people using it can leverage their knowledge of HTTP. It was also, if I am not mistaken, one of the main goals of Symfony 2.0.
In this optic, I would start by stating that HTTP has no concept of what an array is (it is clearly expressed in https://tools.ietf.org/html/rfc7230#section-3.3). By introducing the notion of an array in the interfaces of this component, we clearly deviate from the HTTP spec, which is in itself a red flag for me.
It's not only a matter of being "pure", it is also a matter of not misleading people. Symfony is a reference of good design and architecture in the PHP community, and with good reason. I often see junior developers take for granted what they can find in Symfony. It's often "not a bad thing" because the components have been so thoroughly designed, but in this particular case, it would be a problem to have people think that HTTP can transport "arrays" or any sort of structured data, when in fact it can not. I see this as a first step toward a design philosophy which I think does not match what Symfony ought to be (of course, I acknowledge that the framework philosophy is decided by the core team, I am merely giving my opinion here).
Apart from these considerations, I also think the current design has one huge technical flaw: from a library point of view, if I type-hint on HttpClientInterface
, the behavior of ResponseInterface->toArray()
is not defined.
The builtin implementation is hardcoded to work with JSON data, but there is nothing telling me this (or whatever format is supported) when what I get is an already instanciated object, and there is no standard exception for a format that is not supported. Last but not least, the JsonException implements the TransportExceptionInterface, which do not make much sense. It is not a transport problem, it is a decoding problem.
If the team is confident that the toArray method should indeed continue to exist, I would consider at least creating a DecodingException
or something that would be the base exception for all decoding related problem. This way the developer can catch this and act accordingly. This exception should expose a way to know what exactly went wrong (format not supported, error while decoding, etc).
The ideal scenario for me would be to move this decoding business out of the HTTP Client. It could be for example included in the mime component, or just in a sub-namespace of the HTTP Client. Another possibility could be to offer specialized interfaces like JsonAwareHttpClient
on which you could type hint and be absolutely sure what exact format is supported by the client.
Best regards,