twilio-java icon indicating copy to clipboard operation
twilio-java copied to clipboard

Repeated calls to com.twilio.http.Response.getContent() will result in an empty string

Open Ashpants opened this issue 4 years ago • 5 comments

Issue Summary

Repeated calls to com.twilio.http.Response.getContent() will result in an empty string being returned for all calls after the first. The content read from the stream is not saved to the content field: See Response.getContent() after reading from the stream.

The usage of the getContent is outlined in the example of "Get a Composition Media File" https://www.twilio.com/docs/video/api/compositions-resource

Steps to Reproduce

  1. Create the same example code as Get a Composition File with the noRedirect Client
  2. Examine the response from the restClient and call getContent() on the response, notice you will recieve a json String
  3. Call getContent() a second time on the response object, notice you will get an empty string this time.

Code Snippet

public static void main( String[] args ) throws IOException{
    // Disable HttpClient follow redirect by default
    HttpClientBuilder clientBuilder = HttpClientBuilder.create();
    clientBuilder.disableRedirectHandling();

    // Initialize the client
    TwilioRestClient restClient = new TwilioRestClient
        .Builder(API_KEY_SID, API_KEY_SECRET)
        .httpClient(new NetworkHttpClient(clientBuilder))
        .build();

    // Retrieve media location
    String compositionSid = "CJXXXX";
    Request request = new Request(
        HttpMethod.GET,
        Domains.VIDEO.toString(),
        "/v1/Compositions/" + compositionSid + "/Media?Ttl=3600",
        restClient.getRegion());

    Response response = restClient.request(request);
    String responseContent = response.getContent(); // This will get the string
    JSONObject json = new JSONObject(response.getContent()); // This will throw an exception because content is an empty string
    String mediaLocation = json.getString("redirect_to");

Technical details:

  • twilio-java version: 8.11.0
  • java version:

Ashpants avatar May 17 '21 22:05 Ashpants

Hello @Ashpants,

While I try to reproduce the error, I have a few questions:

  1. (this is more a reminder to myself) Could this be the issue?
  2. What is the use case for repeated calls here? Are you looking for updated content?

With best regards,

Elmer

thinkingserious avatar May 19 '21 21:05 thinkingserious

@thinkingserious

We stubbled across this use case when we tried to consume the com.twilio.http.Response Object multiple times. We wanted to log the API requests/responses associated with twilio to easily debug any problems.

The integration worked fine until we hooked up our logging system, which consumed the content in the Response object. Once the "content" is consumed and read out of the stream, it is no longer available. We were seeing the content in our logs, but the second call to response.getContent() produced an empty string.

It feels like a weird artifact of using the Response object, not realizing it's utilizing a stream underneath that is read once.

I would have expected the response objects to read the content out of the stream and assign it to the "content" field so it could be re-accessed. I see that the content field is checked first and used, but not set upon stream read. https://github.com/twilio/twilio-java/blob/2e5959c5c34788a422395c2edf19a6446b780c89/src/main/java/com/twilio/http/Response.java#L77

Ashpants avatar May 20 '21 03:05 Ashpants

Thank you for the detailed response @Ashpants!

Ah, I see. So if we were to set the content here before we return the data from the stream, the behavior would be as expected. Correct?

thinkingserious avatar May 21 '21 05:05 thinkingserious

Correct, that would fix the problem we encountered.

Ashpants avatar May 21 '21 21:05 Ashpants

Thank you for the confirmation!

This issue has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

thinkingserious avatar May 24 '21 23:05 thinkingserious

Fixed by #706

childish-sambino avatar Sep 06 '22 13:09 childish-sambino