Move CloseableHttpResponse inside nested try with resources

Question

I have the following code using try with resources with CloseableHttpResponse

CloseableHttpResponse response = null;
try (CloseableHttpClient httpClient = HttpClients.custom().build()){    
    //code...
    response = httpClient.execute(target, post);
    String responseText = EntityUtils.toString(response.getEntity());   
} catch (Exception e) {
    logger.error("Failed sending request", e);
} finally {
    if (response != null) {
        try {
            response.close();
        } catch (IOException e) {
            logger.error("Failed releasing response", e);
        }
    }
}

Can I safely replace with nested try with resources:

try (CloseableHttpClient httpClient = HttpClients.custom().build()){
    URIBuilder uriBuilder = new URIBuilder(url);
    HttpHost target = new HttpHost(uriBuilder.getHost(), uriBuilder.getPort(), uriBuilder.getScheme());
    HttpPost post = new HttpPost(uriBuilder.build());
    try (CloseableHttpResponse response = httpClient.execute(target, post)) {
        String responseText = EntityUtils.toString(response.getEntity());   
    }
} catch (Exception e) {
    logger.error("Failed sending request", e);
}

Or is it better to use a single try with resources block:

try (CloseableHttpClient httpClient = HttpClients.custom().build();
    CloseableHttpResponse response = getResponse(httpClient, url)) {

Sometime refactoring to single block is problematic, so I wanted to know the a nested/additional block is a valid solution.

score 1 · Accepted Answer · answered Jan 02 '19 at 13:29

1

HttpClient never returns a null HttpResponse object. The first construct is simply not useful. Both the second and the third constructs are perfectly valid

answered Jan 02 '19 at 13:29

ok2c

26,450
5
63
71

Is there an issue with using nested/additional try-with-resources block ? – Ori Marko Jan 02 '19 at 13:56
No, there is not. However generally one should not create a new instance of HttpClient for each and every request. It is _extremely_ wasteful and inefficient. – ok2c Jan 03 '19 at 14:40
Are you referring using `PoolingHttpClientConnectionManager` as in https://stackoverflow.com/questions/29959772/spring-configurable-high-performance-metered-http-client-instances – Ori Marko Jan 03 '19 at 14:56
The connection pool with persistent connections is certainly one thing that one should not just throw away, but also other expensive operations such SSL/TLS context initialization should ideally only be done one at run-time – ok2c Jan 03 '19 at 15:07
Is your comment about SSL relevant also for 1 request per hour(s)? – Ori Marko Jan 03 '19 at 15:21
Probably not, but why not keeping and re-using HttpClient instance for all subsequent requests? – ok2c Jan 03 '19 at 15:25

Move CloseableHttpResponse inside nested try with resources

1 Answers1