[HTTPASYNC-81] Callback-Behaviour regarding connection-failures and cancellation during transfer.

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 4.0.1
• Fix Version/s: 4.1-beta1
• Assignee: Unassigned
• Reporter: Markus Kull
• Created: 04/Aug/14 14:16
• Updated: 09/Nov/15 11:57
• Resolved: 06/Aug/14 08:52

Description

The current behaviour regarding the callback-methods in HttpAsync-RequestProducer/ResponseConsumer seems surprising to me. I am not quite sure wether this is an actual bug or just a misunderstanding by me. Some javadocs would be helpful.

1) In case of unknown targethost, only close() is called on producers+consumers, but not failed() with the exception. Somehow inconvenient because the callbacks dont see the result-exception.

    @Test
    public void testFailedBeforeClose() throws Exception {
        CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
        client.start();

        HttpGet getMethod = new HttpGet("http://doesnotexist.example.org/");
        HttpAsyncRequestProducer producer = HttpAsyncMethods.create(getMethod);

        final AtomicBoolean closed = new AtomicBoolean(false);
        final AtomicBoolean cancelled = new AtomicBoolean(false);
        final AtomicBoolean failed = new AtomicBoolean(false);
        
        HttpAsyncResponseConsumer<String> consumer = new HttpAsyncResponseConsumer<String>() {

            @Override
            public void close() throws IOException {
                closed.set(true);
            }

            @Override
            public boolean cancel() {
                cancelled.set(true);
                return false;
            }

            @Override
            public void failed(Exception ex) {
                failed.set(true);
            }
            
            public void responseReceived(HttpResponse response) throws IOException, HttpException {
            }

            public void consumeContent(ContentDecoder decoder, IOControl ioctrl) throws IOException {
            }

            public void responseCompleted(HttpContext context) {
            }

            public Exception getException() {
                return null;
            }

            public String getResult() {
                return "result";
            }

            @Override
            public boolean isDone() {
                return false;
            }
        };

        Future<String> future = client.execute(producer, consumer, null, null);

        try {
            future.get();
            Assert.fail();
        } catch (ExecutionException e) {
            Assert.assertTrue(e.getCause() instanceof UnknownHostException);
        }
        Thread.sleep(1000); // give the httpclient time to clean up
        
        Assert.assertTrue(closed.get());
        Assert.assertFalse(cancelled.get());
        Assert.assertTrue(failed.get()); // FAILS! because failed() is not called. Seems to be the same for producer
        
        client.close();
    }

2) If the responseconsumer cancels the response (by isDone() returning true prematurely), then close() is never called on producers or consumers. This could become a resource leak with e.g. ZeroCopyConsumers/Producers. Connections seem to be cleaned up, though.

    @Test
    public void testCloseOnConsumerCancel() throws Exception {
        CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
        client.start();

        HttpGet getMethod = new HttpGet("http://www.example.org/");
        HttpAsyncRequestProducer producer = HttpAsyncMethods.create(getMethod);

        final AtomicBoolean closed = new AtomicBoolean(false);
        final AtomicBoolean cancelled = new AtomicBoolean(false);
        final AtomicBoolean failed = new AtomicBoolean(false);
        
        HttpAsyncResponseConsumer<String> consumer = new HttpAsyncResponseConsumer<String>() {

            @Override
            public void close() throws IOException {
                closed.set(true);
            }

            @Override
            public boolean cancel() {
                cancelled.set(true);
                return false;
            }

            @Override
            public void failed(Exception ex) {
                failed.set(true);
            }
            
            public void responseReceived(HttpResponse response) throws IOException, HttpException {
            }

            public void consumeContent(ContentDecoder decoder, IOControl ioctrl) throws IOException {
            }

            public void responseCompleted(HttpContext context) {
            }

            public Exception getException() {
                return null;
            }

            public String getResult() {
                return "result";
            }

            @Override
            public boolean isDone() {
                return true; // cancels fetching the response-body
            }
        };

        Future<String> future = client.execute(producer, consumer, null, null);

        future.get();
        Thread.sleep(1000); // give the httpclient time to clean up

        Assert.assertTrue(future.isCancelled());
        // Assert.assertTrue(cancelled.get()); // unclear wether it should be set, because the consumer itself cancelled
        Assert.assertFalse(failed.get()); 
        Assert.assertTrue(closed.get()); // FAILS! the consumer wasnt closed
        
        client.close();
    }