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

Produce a better error message when files are found that cannot be processed #238

Merged
merged 3 commits into from
Aug 22, 2018

Conversation

raboof
Copy link
Contributor

@raboof raboof commented Aug 21, 2018

No description provided.

val lines = try {
Source.fromFile(file).getLines().toVector
} catch {
case t: Throwable => throw new RuntimeException(s"Couldn't load '$file'")
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

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!

@dwijnand dwijnand merged commit 6f4187b into lightbend-labs:master Aug 22, 2018
@raboof raboof deleted the catchLoadErrors branch March 15, 2019 08:52
@dwijnand dwijnand added this to the 0.4.0 milestone Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants