-
Notifications
You must be signed in to change notification settings - Fork 71
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
Produce a better error message when files are found that cannot be processed #238
Conversation
val lines = try { | ||
Source.fromFile(file).getLines().toVector | ||
} catch { | ||
case t: Throwable => throw new RuntimeException(s"Couldn't load '$file'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add the original Throwable
as the cause (to avoid swallowing the stack)?
Also, can we be more selective on what throwables to catch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 on adding the original Throwable
.
Not sure if we want to be more selective: the error I had a hard time tracking down was an java.nio.charset.MalformedInputException
. I guess we could catch java.io.IOException
explicitly, but I'd say this catch is useful whatever the exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about those pesky fatal errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say those are unlikely to be caused by any particular file and so it'd be fine to let those bubble up without getting in the way (and causing allocations etc)? Would be happy to include them if you prefer though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant should we avoid trying to handle them? e.g should we use NonFatal
?
Anyways we can always forward-fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, quite right, will update!
No description provided.