guzzle: Allowing verbose output from RequestException

Hi everyone,

Firstly, we’ve been using Guzzle extensively and it’s been a delight. One thing that was going through my mind is that if one wishes to get a verbose message from RequestExceptions, it’s currently hardcoded to consume up to 120 bytes from the stream.

It would be excellent if one could pass on the desired length from the exception message, specifically for logging in our particular case. Since I know PR’s are warmly welcomed I would love to implement that and submit my PR, but first it would be good to know whether there’s any interest in that sort of enhancement.

So basically HandlerStack::create() could be transformed into the following:

public static function create(callable $handler = null, $messageLength = 120)
{
    ...
    $stack->push(Middleware::httpErrors($messageLength), 'http_errors');
    ...
    ...

And then passed downstream to Middleware, etc.

Looking forward to reading your opinion about it.

Cheers, Yuval

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 4
  • Comments: 15 (7 by maintainers)

Most upvoted comments

Closing as it won’t happen as described in this issue. See the referenced one above (#1828).

Sorry to hear about your issues. Although Guzzle can’t be blaimed at all for them (I will explain that later), there is a demand, and we should probably answer that.

It has already been explained that simply removing that limit is not an option, as it would cause other issues (mainly memory related ones). I am not specifically against raising that limit, but that wouldn’t be a solution: in some cases 300 chars would be enough, in some cases not. There is no ideal number that we could pick. Also, memory footprint must be thought about as well.

Another thing that we could do is placing some sort of indication that the body is truncated (like appending (truncated) to the end of the message).

Unfortunately most of these issues are caused by:

  • improper/missing logging
  • improper logging of HTTP errors

The Exception’s message might be a source for you, but ultimately it’s the response which contains a body, which might even be a 10k long JSON object, or just a line of error message. Obviously you don’t want to log that 10k JSON, so in this case you should extract the necessary/useful information from the body, and properly log it instead of the exception message. Think about the body in the message as a convenience thing, but it shouldn’t be relied on. The reason for this is Guzzle cannot have any expectations about the size and structure of the body, as such we cannot do the extraction mentioned above instead of you. In each and every case this is the developer’s responsibility.

I know that everyone has his/her own use case, but we cannot tailor Guzzle to that, we need to keep it as generic as possible.

Unfortunately, $exception->getResponse() and Psr7/str($exception->getResponse) both return the truncated error text

What if you try the following?

(string) $exception->getResponse()->getBody()

This MUST NOT truncate anything. If it does, that’s either a bug (highly doubt that) or something else truncates your body.

Hope this helps.

so a user of “lib X” now needs to become proficient in the guzzle API & it’s exceptions just because “lib X” happens to use guzzle under the hood? that’s not user friendly - I’m not a guzzle user and my first (indirect) experience with it was spending 30 minutes hunting down a truncated exception message.

if the exception had not contained even a truncated message body then it would have been even less useful - if you want to expose non-200 responses as Exceptions in your API that’s fine but then you are obliged to expose all error info from said reponses.

I understand you don’t want to allow unbounded input (reading the code I did not realise $body was some kind of stream wrapper) but 120 bytes is less than a tweet and we live in a world where server’s generally have Gigs of RAM.

In the case of such exception messages you should consider something more sane that offers much less likelyhood of truncating error information from a 3rd party - 1Kb, for instance, is still a tiny amount of memory and would mean the likelyhood of truncation in this context is much less likely.

the memory footprint of an Exception should be a secondary concern with regard to exposing [error] information to developers - RAM is cheap compared to developer time … I think you should prioritize the latter and significantly bump up your 120 byte limit.

I am no expert in all of this - so please forgive my shallow depth of knowledge in this area. However, I want to pass on what truncating the error response to 120 characters has cost me in the last couple weeks.

I have been chasing some bugs in my system that do no log anything in the API server logs. One of the more notorious of these has been the API server crashing out with no error log on “Class ‘classname’ not found” errors. For whatever reason there is nothing logged on the server when this happens and the response I get back within my browser page is a 500 Internal Server Error with a reasonable error message - except that the truncate at 120 characters happens to truncate between the o and t in the word ‘not’ in “Class not found” (and sometimes even earlier like the middle of the class name). So, I had no pointer to where in the large pile of code the error was occurring. It just so happens these responses are json encoded and there is bunches of extra text including “Symfony\Component\Debug\Exception\FatalThrowableError” - that takes up the bulk of the first 120 characters. By the way, these errors are not exposed by wrapping the instantiation of a new class object within try/catch - the PHP just bails out a 500 Internal Server Error rather than handing the exception to the catch handler.

I thought the truncation was coming from PHP itself and the setting for log error message length so I went through the systems changing the max error message length from 1024 to unlimited; I was not prepared for the number of places that had to be changed. After all those changes nothing was different. Pulled some of my remaining hair out, swore a lot and then buckled down to find the problems. Each one eventually turned out to be a missing \ or single \ that should have been \. I spent two or three days searching and stepping through lots of code looking for these little buggers. Everything was working… until today when another one cropped up. Not sure how that could be because “I fixed them all!”.

Well before I made my eyes bleed looking through all that code again I got the crazy idea in my head today to search through all of the files in the system looking for the text “truncate” and there it was in Guzzle. I never expected that. So I looked at the code and decided to change it to the read the whole stream and bam - I get an error message that gives me a filename and a line number in that file where the error occurred. Yep, yet another \ that should have been a \ and of course within a string built on the fly so it was not as obvious as the prior hard coded mistakes. Within 2 minutes it was fixed, working and in test. Prior to this simple change each of these bugs were taking me one to two hours to track down.

With the packing and nesting of packages and sometimes deep directory structures of todays systems I believe that 120 characters is just too short of a length. From reading this thread I understand the desire to bound the length but I do not see the value in 120 characters. Also, if the sender believes that they need to send 10K of error response then why does Guzzle, or any other intermediate package, feel that they know better and that the error message should be shorter? I hate to leave this modification to the Guzzle package code in there, and it will not be in the production system, but I need to see enough error message to be able to quickly isolate a problem and fix it. I have gone back and put the length restriction back in but I set the length to 8K and would not hesitate to make that 32K or more if or when I need to do so. I am sure that there are some systems out there that might struggle with that but I am fortunately not working in such limited systems.

As was suggested I am wrapping the guzzle get call within a try/catch block and logging the error. Unfortunately, $exception->getResponse() and Psr7/str($exception->getResponse) both return the truncated error text not the full response so my log does not have the full response either. If I am not doing this correctly please let me know.

I understand that having the error truncate length being an option when creating a Guzzle client would not help if/when Guzzle is not being used directly but included through another package. What about a config/guzzle.php file to contain a max error length setting?

Please do not register this as a complaint! I believe that Guzzle is great and I am very happy that it was and continues to be available. Thanks to you and everyone else that takes the time to create great packages for the community to use!

you should consider that Guzzle is likely being used as an indirect dependency … which means the user probably has no control over how it is used … in such cases you are stuck with the truncated output exposed by RequestException … and that in turn is a big PITA when the diagnostic info you need from the original/underlying error response is found after the arbitrary 120 byte cutoff.

personally I see no reason to truncate here at all - it’s a premature optimization that just causes pain.