On Tue, 21 Jan 2025 15:09:08 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> test/jdk/java/net/httpclient/HttpResponseLimitingTest.java line 148: >> >>> 146: .timeout(Duration.ofSeconds(5)) >>> 147: .build(); >>> 148: var handler = >>> BodyHandlers.limiting(BodyHandlers.ofByteArray(), capacity); >> >> In this test we are effectively only testing wrapping of a `ofByteArray()` >> `BodyHandler` with a `limiting()` `BodyHandler` and I think that's OK. >> >> `BodyHandler.ofInputStream()` is a bit special. Perhaps we should also add a >> test which verifies that the returned `InputStream` reaches EOF (and the >> `is.read()` calls complete with -1 eventually) when wrapped with a limiting >> BodyHandler with insufficient capacity. > >> Perhaps we should also add a test which verifies that the returned >> InputStream reaches EOF (and the is.read() calls complete with -1 >> eventually) when wrapped with a limiting BodyHandler with insufficient >> capacity. > > Looking at the implementation of `BodyHandlers.ofInputStream()` it appears > that such cases will result in an IOException being thrown from `read(...)` > calls and not `-1`. So I stand corrected - the new test that we add should > assert that the `IOException` gets thrown from `is.read(...)`. Yes - IOException should be thrown since the limiting subscriber will call onError() with an IOExeception. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1923910774
