rootvector2 commented on PR #630:
URL: https://github.com/apache/commons-csv/pull/630#issuecomment-5194538489

   Agreed on caller-supplied readers; the new `testTryWithResourcesParse*` 
tests cover that case, so I dropped that part.
   
   The case I was after is narrower: `parse(Path)` and `parse(URL)` allocate 
the stream themselves via `Files.newInputStream(path)` and `url.openStream()`. 
When `createHeaders()` throws, the caller never gets a parser back, so there is 
nothing to put in a try-with-resources block and the stream the library opened 
is unreachable. Applying the same rule, the method that allocated it now closes 
it on failure; caller-supplied readers and streams are untouched. Reworked the 
PR that way and rebased onto master. Your new tests still pass, and 
`testParseUrlClosesInputStreamWhenHeaderIsInvalid` fails without the change.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to