-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Fix possibly unhandled exception in disk monitor #1458
Fix possibly unhandled exception in disk monitor #1458
Conversation
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.
Looks good.
I would reduce the scope of the extra comment, though.
639f24b
to
c13dae7
Compare
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.
No further testing needed here, assuming we exhaustively covered all call sites of file_size
.
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.
Leaving the rest to CI.
1dc8881
to
33dcc05
Compare
@@ -36,7 +36,12 @@ recursive_size(const std::filesystem::path& root_dir) { | |||
return caf::make_error(ec::filesystem_error, err.message()); | |||
for (const auto& f : dir) { | |||
if (f.is_regular_file()) { | |||
const auto size = f.file_size(); | |||
const auto size = f.file_size(err); |
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 about this bug. My fault 😞
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.
No worries, we missed it during review. Your PRs are super appreciated!
📔 Description
This make
recursive_size
return an error instead of throw a (then unhandled) exception. The underlying root cause will need to be eliminated in a separate PR—it's not that simple to fix.📝 Checklist
🎯 Review Instructions
File-by-file.