Catch and propagate UncheckedIOException in FileFinder #3603
Conversation
tobiasdiez
added
the
status: ready-for-review
| @@ -93,8 +94,8 @@ private boolean matches(String filename, String citeKey) { | |||
| if (Files.exists(directory)) { | |||
| try (Stream<Path> files = Files.find(directory, Integer.MAX_VALUE, isFileWithCorrectExtension)) { | |||
| result.addAll(files.collect(Collectors.toSet())); | |||
| } catch (IOException e) { | |||
| LOGGER.error("Problem in finding files", e); | |||
| } catch (UncheckedIOException e) { | |||
There was a problem hiding this comment.
That's interesting, why/when does this Exception occur?
There was a problem hiding this comment.
To be honest, I have no idea. The documentation babbles something about "a late exception that is thrown when the stream is finished". I guess the file is found, added to the stream but when a bit later toSet is called it is no longer there. I also don't understand why it has to be an unchecked exception instead of a normal IOException.
There was a problem hiding this comment.
I refactored the code slightly to get rid of these exception handling in streams.
I think this is should work better now because the stream is no longer directly closed and then accessed. But I could be mistaken.
There was a problem hiding this comment.
I don't think that works (see below). But thanks for the other fix, that's a nice improvement over my hacky code.
| } catch (UncheckedIOException e) { | ||
| throw new IOException("Problem in finding files", e); | ||
| } | ||
| result.addAll(Files.find(directory, Integer.MAX_VALUE, isFileWithCorrectExtension).collect(Collectors.toSet())); |
There was a problem hiding this comment.
Can you please explain this change and why you removed the try-catch as the point of this PR was to actually catch this UncheckedIOException. Since Files.find still returns a stream, this exception can still occur, right?
There was a problem hiding this comment.
Your code did not compile, you can't throw an exception inside a lambda/stream. That's impossible.
I did not realize that the unchecked exception can still occur. But we should just log it. I mean, what else can we do about it?`We can't rethrow it.
There was a problem hiding this comment.
I just pushed a new solution. I think this kind of catch-rethrow should work.
|
I merge this now so that it can serve as the base for #3623. |
Catch UncheckedIOException and converted it to a usual IOException. Similarly for an
PatternSyntaxException. These are ones of the rare exceptions that were never handled but are reported in azure.gradle localizationUpdate?