Remove backtrace from ECR errors #4851
Conversation
|
Maybe add a comment to explain the purpose of the rescue? |
bew
force-pushed
the
clean-ecr-error
branch
2 times, most recently
from
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.
I'd rather reverse this:
if {Errno::ENOENT, Errno::EISDIR}.includes? ex.errno
STDERR.puts ex.message
exit 1
else
raise ex
endThere was a problem hiding this comment.
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.
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.
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: