-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Remove backtrace from ECR errors #4851
Conversation
Maybe add a comment to explain the purpose of the rescue? |
b7fafc6
to
007c23c
Compare
I've restricted the rescue, to rescue only |
I think we should only rescue some Errno's, specifically |
@RX14 done. I rescued |
src/ecr/process.cr
Outdated
begin | ||
puts ECR.process_file(filename, buffer_name) | ||
rescue ex : Errno | ||
raise ex unless {Errno::ENOENT, Errno::EISDIR}.includes?(ex.errno) |
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 rather reverse this:
if {Errno::ENOENT, Errno::EISDIR}.includes? ex.errno
STDERR.puts ex.message
exit 1
else
raise ex
end
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.
why?
In my coding-style I tend to discard "invalid" values (early returns), then have the content of what want to do in the method/rescue/anything.
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.
That makes sense when the method body is long, but here's it's just two statements, making the if
clearer. Once it gets to 5-7+ lines i'd agree with you.
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.
This is a rescue block, it shouldn't get that long... ;)
Rescure only Errno::ENOENT & Errno::EISDIR errors, as thoses are the most common errors in this case.
@RX14 I've updated to use long-form if in the rescue block. |
Ready to 🚢 |
Beautify ECR errors for issues like #4577, by removing the stacktrace.
Before:


After: