Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw exceptions instead of being silent, just like Scala #246

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Dec 6, 2023

This was bothering me a bit while reviewing

because even though Scala does not require you to catch checked exceptions, it does throw them at runtime if they occur.
I don't really like being silent and just swallowing the errors and continuing like nothing happened. If people get strange behaviour because some files can't be watched or whatever it's very hard to figure out what's going on, so IMHO better to just fail with an exception. Given that the equivalent Scala code did the same, I guess this exceptions will not really occur in real life anyway.

Also, look at that better-files says about the walk method:

    Files.walk(path, maxDepth, visitOptions: _*) // TODO: that ignores I/O errors?

So the devs were not even sure what's happening (this line was added 8+ years ago to be fair). What happens is at Runtime the IOException will be thrown. What they did now is:
pathikrit/better-files@0a22873#diff-1eed0c8cd6c94c548b033043e1669d94cb999fd88dc17d9bc3db3f6247fb51e2L689
move that in and GitHub issue pathikrit/better-files#590 so they figure out what to do.
Anyway, I think throwing is better now. Actually nothing changes from previous play-file-watch versions...

@mkurz mkurz merged commit d10717c into playframework:main Dec 6, 2023
11 checks passed
@mkurz mkurz deleted the throw_instead_silent branch December 6, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants