verbose message for incorrect value errors
related to this https://github.com/logchange/logchange/pull/282#discussion_r1660187287
The change introduced in PR #282 is causing the issue described in issue #317. Specifically, the problem lies in this part of the code:
private List<ChangelogEntry> getEntries(File versionDirectory) {
return getEntriesFiles(versionDirectory)
.sequential()
.map((file) -> {
try {
return YMLChangelogEntry.of(getEntryInputStream(file), file.getPath());
} catch (YMLChangelogEntryConfigException e) {
log.warning(e.getMessage());
return null;
}
})
.filter(Objects::nonNull)
.map(YMLChangelogEntry::to)
.collect(Collectors.toList());
}
In my opinion, it's better not to handle this exception and instead allow the process to crash, so the user is immediately aware of the issue and can fix the problematic file. The current approach silently skips the invalid file, which may lead to unnoticed errors.
I suggest reverting the code to:
private List<ChangelogEntry> getEntries(File versionDirectory) {
return getEntriesFiles(versionDirectory)
.sequential()
.map((file) -> YMLChangelogEntry.of(getEntryInputStream(file), file.getPath()))
.map(YMLChangelogEntry::to)
.collect(Collectors.toList());
}
This change will make the error handling more transparent and help users address configuration issues promptly. @NikodemMarek @marwin1991
@witx98 Is there an option, to throw exception after running all files ?
Now IMO process will stop on the first wrong file, but I think it would be nice to have information about all wrong entries.
Probably:
.map((file) -> {
try {
return YMLChangelogEntry.of(getEntryInputStream(file), file.getPath());
} catch (YMLChangelogEntryConfigException e) {
log.warning(e.getMessage());
return null;
}
})
will brake lint command ? @NikodemMarek
I believe solution laki this should address the requirement to collect errors from all files before throwing an exception:
private List<ChangelogEntry> getEntries(File versionDirectory) {
List<Exception> exceptions = new ArrayList<>();
List<ChangelogEntry> entries = getEntriesFiles(versionDirectory)
.sequential()
.map((file) -> {
try {
return YMLChangelogEntry.of(getEntryInputStream(file), file.getPath());
} catch (YMLChangelogEntryConfigException e) {
exceptions.add(e);
return null;
}
})
.filter(Objects::nonNull)
.map(YMLChangelogEntry::to)
.collect(Collectors.toList());
if (!exceptions.isEmpty()) {
throw new YMLChangelogException(exceptions);
}
return entries;
}
public class YMLChangelogException extends RuntimeException {
public YMLChangelogException(List<Exception> exceptions) {
super(toString(exceptions));
}
private static String toString(List<Exception> exceptions) {
StringBuilder sb = new StringBuilder();
sb.append("Errors found:\n");
exceptions.forEach(exception -> sb.append(exception.getMessage()).append("\n"));
return sb.toString();
}
}
@marwin1991
@witx98 exactly!
great suggestion @witx98, I'm gonna apply it when I get the time