Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Incomplete
    • Affects Version/s: 3.0 RC3
    • Fix Version/s: None
    • Component/s: HttpClient
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: Other

      Description

      Whenever I use getResponseBody or getResponseBodyAsString I get a warning in the
      log that I shouldn't do this. Either deprecate these methods or accept that
      there are valid usecases for using them and remove the warning.

      In my case I usually get the response to do stuff with it in memory. That
      usually requires having either a bytearray or a string (i.e. it is a design
      choice, I accept the memory overhead). I suppose that's why these methods are
      part of the API (and I suspect they are used frequently).

      IMHO in general components like httpclient should always be silent and rely on
      proper exception handling so that the user can decide whether to log exceptions,
      errors, etc. It certainly shouldn't try to educate the programmer in through
      logging messages.

      Now I need to write code to avoid the log filling with useless warnings (my
      customers hate noisy logs). I shouldn't have to that.

        Issue Links

          Activity

          Hide
          Roland Weber added a comment -

          We probably should reduce the level of this message from "warn" to "info".
          If users configure logging of HttpMethodBase to get rid of this message,
          they would disable more important warnings too.

          Jilles, if you feel your code is already logging everything worth logging,
          you don't have to write code to avoid getting messages from HttpClient.
          Just raise the default log level. See our logging guide for details:
          http://jakarta.apache.org/commons/httpclient/logging.html

          cheers,
          Roland

          Show
          Roland Weber added a comment - We probably should reduce the level of this message from "warn" to "info". If users configure logging of HttpMethodBase to get rid of this message, they would disable more important warnings too. Jilles, if you feel your code is already logging everything worth logging, you don't have to write code to avoid getting messages from HttpClient. Just raise the default log level. See our logging guide for details: http://jakarta.apache.org/commons/httpclient/logging.html cheers, Roland
          Hide
          Michael Becke added a comment -

          Hi Jiles,

          This warning message is there for a good reason. It's there to let you know why
          you're getting out of memory exceptions Below is the code where this message
          comes from:

          int limit =
          getParams().getIntParameter(HttpMethodParams.BUFFER_WARN_TRIGGER_LIMIT, 1024*1024);
          if ((contentLength == -1) || (contentLength > limit))

          { LOG.warn("Going to buffer response body of large or unknown size. " +"Using getResponseAsStream instead is recommended."); }

          This message is only printed when the response contains no content length (i.e.
          could be of any size) or when it is larger than a configurable maximum size.
          Please give the BUFFER_WARN_TRIGGER_LIMIT param a try.

          Also, as Roland mentions perhaps INFO would be better, but this may also end up
          in your logs.

          Mike

          Show
          Michael Becke added a comment - Hi Jiles, This warning message is there for a good reason. It's there to let you know why you're getting out of memory exceptions Below is the code where this message comes from: int limit = getParams().getIntParameter(HttpMethodParams.BUFFER_WARN_TRIGGER_LIMIT, 1024*1024); if ((contentLength == -1) || (contentLength > limit)) { LOG.warn("Going to buffer response body of large or unknown size. " +"Using getResponseAsStream instead is recommended."); } This message is only printed when the response contains no content length (i.e. could be of any size) or when it is larger than a configurable maximum size. Please give the BUFFER_WARN_TRIGGER_LIMIT param a try. Also, as Roland mentions perhaps INFO would be better, but this may also end up in your logs. Mike
          Hide
          Jilles van Gurp added a comment -

          (In reply to comment #2)
          > Hi Jiles,
          >
          > This warning message is there for a good reason. It's there to let you know why
          > you're getting out of memory exceptions Below is the code where this message
          > comes from:
          >
          > int limit =
          > getParams().getIntParameter(HttpMethodParams.BUFFER_WARN_TRIGGER_LIMIT,
          1024*1024);
          > if ((contentLength == -1) || (contentLength > limit))

          { > LOG.warn("Going to buffer response body of large or unknown size. " > +"Using getResponseAsStream instead is recommended."); > }

          >
          > This message is only printed when the response contains no content length (i.e.
          > could be of any size) or when it is larger than a configurable maximum size.
          > Please give the BUFFER_WARN_TRIGGER_LIMIT param a try.
          >
          > Also, as Roland mentions perhaps INFO would be better, but this may also end up
          > in your logs.
          >
          > Mike
          >

          I fully understand why you in general buffering should be carefully considered
          but I hope you understand that doing something with the response is an extremely
          common (if not the most common) thing to do when using httpclient.

          I'd still prefer no log message at all. Once again, components like this should
          IMHO be totally silent unless you debug them and throw exceptions when bad stuff
          happens. An exception if the trigger limit is exceeded would be a nice solution.
          That way you get no outofmemory exception, a clean log and you can increment the
          limit if you know what you are doing. An optional setting of no limit would be
          nice too. I suppose that's not feasible for 3.0 given this stage of development
          but it would be nice for 3.x.

          Show
          Jilles van Gurp added a comment - (In reply to comment #2) > Hi Jiles, > > This warning message is there for a good reason. It's there to let you know why > you're getting out of memory exceptions Below is the code where this message > comes from: > > int limit = > getParams().getIntParameter(HttpMethodParams.BUFFER_WARN_TRIGGER_LIMIT, 1024*1024); > if ((contentLength == -1) || (contentLength > limit)) { > LOG.warn("Going to buffer response body of large or unknown size. " > +"Using getResponseAsStream instead is recommended."); > } > > This message is only printed when the response contains no content length (i.e. > could be of any size) or when it is larger than a configurable maximum size. > Please give the BUFFER_WARN_TRIGGER_LIMIT param a try. > > Also, as Roland mentions perhaps INFO would be better, but this may also end up > in your logs. > > Mike > I fully understand why you in general buffering should be carefully considered but I hope you understand that doing something with the response is an extremely common (if not the most common) thing to do when using httpclient. I'd still prefer no log message at all. Once again, components like this should IMHO be totally silent unless you debug them and throw exceptions when bad stuff happens. An exception if the trigger limit is exceeded would be a nice solution. That way you get no outofmemory exception, a clean log and you can increment the limit if you know what you are doing. An optional setting of no limit would be nice too. I suppose that's not feasible for 3.0 given this stage of development but it would be nice for 3.x.
          Hide
          Roland Weber added a comment -

          Hello Jilles,

          > I'd still prefer no log message at all. Once again, components like this
          > should IMHO be totally silent unless you debug them and throw exceptions
          > when bad stuff happens.

          Let me repeat: if you want HttpClient to be totally silent, then disable
          HttpClient logging. By default, messages with a level higher than "INFO"
          will be logged. Just switch them off unless you debug. This default comes
          from the logging component you use, not from HttpClient. We can't change
          it for you.

          When bad stuff happens, exceptions will be thrown. When questionable stuff
          happens, such as in this case, no exception will be thrown because
          HttpClient is able to deal with the situation. It would not be reasonable
          to throw an exception in this case, because then every application would
          need exception handling code for it. And we would have to add an API to
          perform the same action without an exception being thrown. You do not really
          expect us to duplicate the whole API, just to have methods that will not
          throw "questionable" exceptions in addition to those that do?

          cheers,
          Roland

          Show
          Roland Weber added a comment - Hello Jilles, > I'd still prefer no log message at all. Once again, components like this > should IMHO be totally silent unless you debug them and throw exceptions > when bad stuff happens. Let me repeat: if you want HttpClient to be totally silent, then disable HttpClient logging. By default, messages with a level higher than "INFO" will be logged. Just switch them off unless you debug. This default comes from the logging component you use, not from HttpClient. We can't change it for you. When bad stuff happens, exceptions will be thrown. When questionable stuff happens, such as in this case, no exception will be thrown because HttpClient is able to deal with the situation. It would not be reasonable to throw an exception in this case, because then every application would need exception handling code for it. And we would have to add an API to perform the same action without an exception being thrown. You do not really expect us to duplicate the whole API, just to have methods that will not throw "questionable" exceptions in addition to those that do? cheers, Roland
          Hide
          Jilles van Gurp added a comment -

          >
          > Let me repeat: if you want HttpClient to be totally silent, then disable
          > HttpClient logging. By default, messages with a level higher than "INFO"
          > will be logged. Just switch them off unless you debug. This default comes
          > from the logging component you use, not from HttpClient. We can't change
          > it for you.
          >

          I know I can work around this bug by writing extra code or by imposing
          additional configuration on my sysadmins (who are generally clueless about such
          things).In fact all my programs include a "shut up httpclient section". My point
          is that I shouldn't have to do that. I use dozens of libraries (including lots
          of apache stuff), httpclient is the only one to abuse my log in this way. No
          production software should have this kind of warnings.

          The default comes from another fine apache product by the way, tomcat. I have no
          wish to change it (sensible default) since life is hard enough already for the
          sysadmins.

          Show
          Jilles van Gurp added a comment - > > Let me repeat: if you want HttpClient to be totally silent, then disable > HttpClient logging. By default, messages with a level higher than "INFO" > will be logged. Just switch them off unless you debug. This default comes > from the logging component you use, not from HttpClient. We can't change > it for you. > I know I can work around this bug by writing extra code or by imposing additional configuration on my sysadmins (who are generally clueless about such things).In fact all my programs include a "shut up httpclient section". My point is that I shouldn't have to do that. I use dozens of libraries (including lots of apache stuff), httpclient is the only one to abuse my log in this way. No production software should have this kind of warnings. The default comes from another fine apache product by the way, tomcat. I have no wish to change it (sensible default) since life is hard enough already for the sysadmins.
          Hide
          Ortwin Glück added a comment -

          Back from short vacation. "getResponseBody always logs messages" is not true.

          Set the parameter BUFFER_WARN_TRIGGER_LIMIT high enough for your needs.
          If necessary increase the log level on the respective class. See Log4J
          Documentation.
          No the log level should not be INFO. It is an important warning and is therefore
          correctly logged with WARN level.

          Ortwin Glück

          Show
          Ortwin Glück added a comment - Back from short vacation. "getResponseBody always logs messages" is not true. Set the parameter BUFFER_WARN_TRIGGER_LIMIT high enough for your needs. If necessary increase the log level on the respective class. See Log4J Documentation. No the log level should not be INFO. It is an important warning and is therefore correctly logged with WARN level. Ortwin Glück
          Hide
          Oleg Kalnichevski added a comment -

          Jilles,
          I second Odi's opinion. Besides, the use of HttpMethod#getResponseBody and
          HttpMethod#getResponseBodyAsString is strongly discouraged. It is advised to use
          HttpMethod#getResponseBodyAsStream instead. See HttpClient optimization guide
          for details:

          <http://jakarta.apache.org/commons/httpclient/performance.html#Request/Response%20entity%20streaming>

          Oleg

          Show
          Oleg Kalnichevski added a comment - Jilles, I second Odi's opinion. Besides, the use of HttpMethod#getResponseBody and HttpMethod#getResponseBodyAsString is strongly discouraged. It is advised to use HttpMethod#getResponseBodyAsStream instead. See HttpClient optimization guide for details: < http://jakarta.apache.org/commons/httpclient/performance.html#Request/Response%20entity%20streaming > Oleg
          Hide
          Jilles van Gurp added a comment -

          I don't think we'll end up agreeing. I fundamentally believe httpclient is
          misbehaving here and shouldn't. Otherwise it is great stuff and much appreciated
          BTW.

          I somehow always end up having to do stuff in memory, which requires doing a
          getResponseBody(). I can't optimize this need away, I need to buffer. I do non
          trivial stuff which is hard to do in a streaming fashion (and I agree, if you
          can do it streaming: do so). I'm not alone, that's why these methods are in the
          API. If you accept that simple fact you'll see it is bad practice to keep
          printing stuff in the log whenever people choose to use the non deprecated api.

          The only good solution is not logging. The log is not an educational tool. If it
          is an error throw exceptions otherwise shut up. Simple principle that shouldn't
          be violated by any 3rd party software IMHO. Httpclient log warnings are always a
          problem in any production software. Don't require addiotional code,
          configuration, whatever for normal behavior. The negative effects of buffering
          are properly documented in the javadoc and tutorials, that should be enough.

          But I'll shut up now, we apparently disagree and it's not an important issue to
          me. Please don't post another workaround here I knew about all of them (and have
          practiced them) except for the BUFFER_WARN_TRIGGER_LIMIT which should probably
          in the javadoc of these methods. I'll probably end up setting it to a very large
          number in all my software. Again my point is that I shouldn't have to, not that
          I don't know how to fix it.

          (In reply to comment #7)
          > Jilles,
          > I second Odi's opinion. Besides, the use of HttpMethod#getResponseBody and
          > HttpMethod#getResponseBodyAsString is strongly discouraged. It is advised to use
          > HttpMethod#getResponseBodyAsStream instead. See HttpClient optimization guide
          > for details:
          >
          >
          <http://jakarta.apache.org/commons/httpclient/performance.html#Request/Response%20entity%20streaming>
          >
          > Oleg

          Show
          Jilles van Gurp added a comment - I don't think we'll end up agreeing. I fundamentally believe httpclient is misbehaving here and shouldn't. Otherwise it is great stuff and much appreciated BTW. I somehow always end up having to do stuff in memory, which requires doing a getResponseBody(). I can't optimize this need away, I need to buffer. I do non trivial stuff which is hard to do in a streaming fashion (and I agree, if you can do it streaming: do so). I'm not alone, that's why these methods are in the API. If you accept that simple fact you'll see it is bad practice to keep printing stuff in the log whenever people choose to use the non deprecated api. The only good solution is not logging. The log is not an educational tool. If it is an error throw exceptions otherwise shut up. Simple principle that shouldn't be violated by any 3rd party software IMHO. Httpclient log warnings are always a problem in any production software. Don't require addiotional code, configuration, whatever for normal behavior. The negative effects of buffering are properly documented in the javadoc and tutorials, that should be enough. But I'll shut up now, we apparently disagree and it's not an important issue to me. Please don't post another workaround here I knew about all of them (and have practiced them) except for the BUFFER_WARN_TRIGGER_LIMIT which should probably in the javadoc of these methods. I'll probably end up setting it to a very large number in all my software. Again my point is that I shouldn't have to, not that I don't know how to fix it. (In reply to comment #7) > Jilles, > I second Odi's opinion. Besides, the use of HttpMethod#getResponseBody and > HttpMethod#getResponseBodyAsString is strongly discouraged. It is advised to use > HttpMethod#getResponseBodyAsStream instead. See HttpClient optimization guide > for details: > > < http://jakarta.apache.org/commons/httpclient/performance.html#Request/Response%20entity%20streaming > > > Oleg
          Hide
          Ortwin Glück added a comment -

          Jilles,

          A final statement. Use of these methods is risky becaue they have a very simple
          implementation. That's why they issue the warning. If you need to buffer anyway
          use getResponseAsStream and copy the stuff into a byte array or so. Then you
          have fully control over checking for out of memory conditions.

          If you continue to use getResponseAsString you expose your software to a denial
          of service attack. Just to quiet the logs is bad. It is like hiding bad code
          from your customer. Write better code and the message disappears.

          Ortwin Glück

          Show
          Ortwin Glück added a comment - Jilles, A final statement. Use of these methods is risky becaue they have a very simple implementation. That's why they issue the warning. If you need to buffer anyway use getResponseAsStream and copy the stuff into a byte array or so. Then you have fully control over checking for out of memory conditions. If you continue to use getResponseAsString you expose your software to a denial of service attack. Just to quiet the logs is bad. It is like hiding bad code from your customer. Write better code and the message disappears. Ortwin Glück

            People

            • Assignee:
              Unassigned
              Reporter:
              Jilles van Gurp
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development