Hello @Chen Liang <[email protected]>, Thanks. I can agree that, after pulling up the source code, it becomes obvious what occurred. That said, I do like your suggestion of the simple String "closed" to be used as the exception message. If you are ok with that, I can start replacing once I get a JBS number.
As for the documentation, I think the documentation is fine. Looking at the stack trace, I didn't think that the place to look was Collectors.flatMapping and how it should be used. Had I realized that from looking at that stack trace, I would have caught on quickly. On Mon, Nov 24, 2025, 10:45 PM Chen Liang <[email protected]> wrote: > Hello, David, > I don't think this is necessary - the cause of the exception is already > quite clear from the stack trace. I checked a few use sites of the no-arg > constructor of IllegalStateException - many of them are in the same > situation, that they are easy to understand once we see their location in > code. I assume such a use of no-arg ISE constructor is widely accepted. > > For error messages, sometimes it is not feasible to be too detailed - I > would think if we do add any, something like "closed" would be sufficient. > Printing too many details in exceptions and log messages can sometimes > constitute security risks. > > For the stream part, I believe both Files.walk and Stream.flatMap do their > job - walk specifies you should close after use, and flatMap specifies the > returned streams are closed once their elements are all taken. However, I > do agree that there are possible documentation enhancements, that: > 1. Stream factories should specify if their elements are still usable > after the streams are closed, and > 2. Stream should note that they should be closed after a terminal > operation (which is the fault you made here) > > Regards, > Chen Liang > ------------------------------ > *From:* core-libs-dev <[email protected]> on behalf of David > Alayachew <[email protected]> > *Sent:* Monday, November 24, 2025 1:38 PM > *To:* core-libs-dev <[email protected]> > *Subject:* Add better exception message to FileTreeIterator.hasNext()? > > Hello @core-libs-dev <[email protected]>, > > I was writing a simple script today -- find how many Java files are in > each of my top level folders of my current directory. > > So, I ran the following code. > > Files > list(Path.of(".")) > filter(Files::isDirectory) > collect > > Collectors.groupingBy( > Function.<Path>identity(), > Collectors > .flatMapping > ( > (final Path root) -> > { > try (final Stream<Path> streamWalker = > Files.walk(root)) > { > return > streamWalker > .filter(Files::isRegularFile) > .filter(file -> > file.toString().endsWith(".java")) > ; > } > catch (final Exception exception) > { > throw new RuntimeException(exception); > } > }, > Collectors.counting() > ) > ) > ; > > Now, there is a not-so-obvious bug -- when using a flatMapping Collector, > no need for try-with-resources. > > The above code threw the below stack trace. > > | Exception java.lang.IllegalStateException > | at FileTreeIterator.hasNext (FileTreeIterator.java:100) > | at Iterator.forEachRemaining (Iterator.java:132) > | at Spliterators$IteratorSpliterator.forEachRemaining > (Spliterators.java:1939) > | at AbstractPipeline.copyInto (AbstractPipeline.java:570) > | at AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:560) > | at ForEachOps$ForEachOp.evaluateSequential (ForEachOps.java:153) > | at ForEachOps$ForEachOp$OfRef.evaluateSequential > (ForEachOps.java:176) > | at AbstractPipeline.evaluate (AbstractPipeline.java:265) > | at ReferencePipeline.forEach (ReferencePipeline.java:632) > | at Collectors.lambda$flatMapping$0 (Collectors.java:483) > | at Collectors.lambda$groupingBy$0 (Collectors.java:1113) > | at ReduceOps$3ReducingSink.accept (ReduceOps.java:169) > | at ReferencePipeline$2$1.accept (ReferencePipeline.java:197) > | at Iterator.forEachRemaining (Iterator.java:133) > | at Spliterators$IteratorSpliterator.forEachRemaining > (Spliterators.java:1939) > | at AbstractPipeline.copyInto (AbstractPipeline.java:570) > | at AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:560) > | at ReduceOps$ReduceOp.evaluateSequential (ReduceOps.java:921) > | at AbstractPipeline.evaluate (AbstractPipeline.java:265) > | at ReferencePipeline.collect (ReferencePipeline.java:723) > | at (#4:5) > > > Not obvious what is wrong here. > > Now, the exception message is being thrown by the hasNext() method of the > FileTreeIterator class. After looking at the source code of that class, > it's clear what the problem is. > > > https://github.com/openjdk/jdk/blob/jdk-26%2B25/src/java.base/share/classes/java/nio/file/FileTreeIterator.java#L98 > > It throws an exception because the FileTreeWalker contained within the > FileTreeIterator is closed (presumably by the try-with-resources). However, > the thrown exception message is empty. This exact same check-then-throw > logic is duplicated all over FileTreeIterator. > > I propose that all such instances of check-then-throw be provided with > exception messages along the lines of "Cannot check for more elements > because the file walker is already closed!". I am not picky on the wording. > Just something to inform me that the issue is something is closed, as > opposed to just a generic IllegalStateException. I would even accept an > empty message body if the exception type communicated the issue. > > Thank you for your time. > David Alayachew >
