Do we need to consume HttpURLConnection's error stream when IOException thrown

android

Solution

If you are not interested in displaying the error message to the user, close the `InputStream` or invoke `disconnect` on `HttpURLConnection` in `finally` block without reading the error message. This is what you see in most of the examples.

I came across following comment in one of the source code, while browsing the implementation of HttpURLConnection. That could be the reason why connections are closed without reading all data.

This should be invoked when the connection is closed unexpectedly to invalidate the cache entry and to prevent the HTTP connection from being reused. HTTP messages are sent in serial so whenever a message cannot be read to completion, subsequent messages cannot be read either and the connection must be discarded.

According to Android's implementation of `HttpURLConnection`, in case of exception:

- If error is not read and the `InputStream` is closed, the connection will be considered as not reusable and closed down.

- If you read the error and then close the `InputStream`, connection is considered as reusable and is added to the connection pool.

You can see in the below image, variable `connection` & `connectionReleased` are set to `null` and `true` respectively, as soon as all data is read. Note that `getErrorStream` returns the `InputStream`, so it is valid in exception scenario also.

Code analysis : Let's look at the FixedLengthInputStream one of the specialized `InputStream` implementation. Here is the `close` method implementation:

 @Override public void close() throws IOException {
    if (closed) {
        return;
    }
    closed = true;
    if (bytesRemaining != 0) {
        unexpectedEndOfInput();
    }
 }

Instance variable `bytesRemaining` contains byte count still available on the `InputStream` to be read. Here is the unexpectedEndOfInput method implementation:

protected final void unexpectedEndOfInput() {
    if (cacheRequest != null) {
        cacheRequest.abort();
    }
    httpEngine.release(false);
}

Here is the release method implementation. Calling `disconnect` on `HttpURLConnection` instance leads the call to this `release` method with `false` as parameter. The last `if` check ensures whether connection need to be closed down or added to the connection pool for reuse.

public final void release(boolean reusable) {
    // If the response body comes from the cache, close it.
    if (responseBodyIn == cachedResponseBody) {
        IoUtils.closeQuietly(responseBodyIn);
    }
    if (!connectionReleased && connection != null) {
        connectionReleased = true;
        // We cannot reuse sockets that have incomplete output.
        if (requestBodyOut != null && !requestBodyOut.closed) {
            reusable = false;
        }
        // If the headers specify that the connection shouldn't be reused, don't reuse it.
        if (hasConnectionCloseHeader()) {
            reusable = false;
        }
        if (responseBodyIn instanceof UnknownLengthHttpInputStream) {
            reusable = false;
        }
        if (reusable && responseBodyIn != null) {
            // We must discard the response body before the connection can be reused.
            try {
                Streams.skipAll(responseBodyIn);
            } catch (IOException e) {
                reusable = false;
            }
        }
        if (!reusable) {
            connection.closeSocketAndStreams();
            connection = null;
        } else if (automaticallyReleaseConnectionToPool) {
            HttpConnectionPool.INSTANCE.recycle(connection);
            connection = null;
        }
    }
}

The code shared by you, in which the IOException is handled, error stream is read and then closed, ensures the Connection is reusable and is added to the connection pool. The moment all data is read from `InputStream` the `Connection` is added to the connection pool. Here is the `read` method implementation of `FixedLengthInputStream` :

@Override public int read(byte[] buffer, int offset, int count) throws IOException {
        Arrays.checkOffsetAndCount(buffer.length, offset, count);
        checkNotClosed();
        if (bytesRemaining == 0) {
            return -1;
        }
        int read = in.read(buffer, offset, Math.min(count, bytesRemaining));
        if (read == -1) {
            unexpectedEndOfInput(); // the server didn't supply the promised content length
            throw new IOException("unexpected end of stream");
        }
        bytesRemaining -= read;
        cacheWrite(buffer, offset, read);
        if (bytesRemaining == 0) {
            endOfInput(true);
        }
        return read;
    }

When `bytesRemaining` variable becomes 0, endOfInput is called which will futher call `release` method with `true` parameter, which will ensures the connection is pooled.

protected final void endOfInput(boolean reuseSocket) throws IOException {
        if (cacheRequest != null) {
            cacheBody.close();
        }
        httpEngine.release(reuseSocket);
    }

Problem

Original source