BREAKING(ext/net): improved error code accuracy #25383
Conversation
lucacasonato
force-pushed
the
interrupted_and_busy
branch
2 times, most recently
from
d74412c
to
39eb431
Compare
ext/net/ops_unix.rs
Outdated
| .map_err(|_| bad_resource("Listener has been closed"))?; | ||
| .map_err(|_| { | ||
| std::io::Error::new( | ||
| std::io::ErrorKind::Interrupted, |
There was a problem hiding this comment.
Interrupted is not really an appropriate error code in this case. Do you have something like Aborted ?
The reason is that EINTR is used very specifically for cases where a syscall is interrupted by a signal and should be retried. That semantic spills over into rust, as per the docs:
This operation was interrupted.
Interrupted operations can typically be retried.
There was a problem hiding this comment.
You suggested to use Interrupted here: #13075 (comment). We do not have an Aborted error.
I can introduce a new Closed error, and use that everywhere we throw a BadResource error - BadResource is not descriptive anyway.
There was a problem hiding this comment.
Closed seems reasonable - BadResource is a catch-all that is completely non-descriptive to users. Maybe we could make Closed extends BadResource to limit the impact as there are a lot of checks for Deno.errors.BadResource.
There was a problem hiding this comment.
We discussed this offline: we're reverting back to BadResource to behave like the EBADF that libuv returns in this case. We are keeping the Busy changes though.
lucacasonato
force-pushed
the
interrupted_and_busy
branch
from
39eb431
to
f7e71b1
Compare
Fixes #13086