guzzle / psr7

PSR-7 HTTP message library
MIT License
7.85k stars 2 forks source link

Support non standard HTTP status codes #604

Open boskos-q opened 4 months ago

boskos-q commented 4 months ago

Description

So I found PR #334 that was closed, and I didn't wanted to "necro" it. As mentioned in the comments there rfc7231 should be respected, but regardless that PR got declined? In provided RFC it's mentioned:

 The status-code element is a three-digit integer code giving the
 result of the attempt to understand and satisfy the request.

 HTTP status codes are extensible.  HTTP clients are not required to
 understand the meaning of all registered status codes, though such
 understanding is obviously desirable.

RFC defines de-facto standard status codes and which are registered with IANA source. A side from those codes servers CAN return other status codes as well, but they must be 3 digits long, meaning that status codes 6xx, 7xx, 8xx, 9xx are also valid. Checking PSR7, there is no clear definition that status codes >= 600 || <= 999 can't be supported, but rather just links to the previously mentioned RFC and IANA. This is not respecting mentioned RFC fully, and is supporting only de-facto standard status codes while ignoring non standard, but RFC supported status codes.

Example

Just by googling a bit for status codes that are in "non supported range", you'll stumble upon multiple major service providers that uses them. Also some banking systems use those status codes as well to handle some edge cases while providing a reason phrase as well.

Additional context

By being one of the default packages used by major frameworks, and a lot of other packages as dependency, you make a big impact on how we develop our apps for the web. Having this limitation is really bad an having to do a workaround while having this package installed is just .. By providing actual context in the description section, I strongly believe that mentioned PR should be re-opened and merged.

boskos-q commented 4 months ago

@GrahamCampbell can I get your input on this? Thanks

GrahamCampbell commented 1 month ago

Thanks for the question. What's the use case here? I know the RFC exists, but in practice, Guzzle and cURL tends to follow what the browsers do, which unfortunately is not what the RFC says in many cases.

boskos-q commented 1 month ago

cURL can handle status codes >= 600 & <= 999 while Guzzle can't handle those and throws exception.

There are systems in place which utilize these response status codes (which are valid by the provided RFC) but not "standard" by IANA. One of those is one banking system for which I did integration in Laravel (which uses Guzzle by default). That system returned >= 600 status codes depending of error type, but Guzzle was unable to handle those so I had to fallback to the cURL for it. This system is one of many out there that utilizes such response codes, but whoever uses Laravel or Guzzle itself would not be able to make an integration. Guzzle should not be the breaking point in this, but it should rather support it all the way up to 999 status code and let developer handle those edge cases with 600+ status codes.

Aside of that, this package (guzzle/psr7) doesn't fully follow PSR7 standard because of it (doesn't respect mentioned RFC).

As a solution, I'd propose to re-open and merge #334 which fixes this issue.

Let me know what you think. Thanks

GrahamCampbell commented 1 month ago

The PR cannot be re-opened (because the upstream branch is deleted), but I will re-consider this for the next major version.