Skip to content

Commit

Permalink
Improve error messages when unavailable.
Browse files Browse the repository at this point in the history
  • Loading branch information
rolftimmermans committed Dec 16, 2019
1 parent d764484 commit a4b290d
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 17 deletions.
6 changes: 3 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export interface Writable<
*
* Only **one** asynchronously blocking call to {@link send}() may be executed
* simultaneously. If you call {@link send}() again on a socket that is in the
* mute state it will return a rejected promise with an `EAGAIN` error.
* mute state it will return a rejected promise with an `EBUSY` error.
*
* The reason for disallowing multiple {@link send}() calls simultaneously is
* that it could create an implicit queue of unsendable outgoing messages.
Expand Down Expand Up @@ -229,12 +229,12 @@ export interface Readable<M extends object[] = Message[]> {
*
* Only **one** asynchronously blocking call to {@link receive}() can be in
* progress simultaneously. If you call {@link receive}() again on the same
* socket it will return a rejected promise with an `EAGAIN` error. For
* socket it will return a rejected promise with an `EBUSY` error. For
* example, if no messages can be read and no `await` is used:
*
* ```typescript
* socket.receive() // -> pending promise until read is possible
* socket.receive() // -> promise rejection with `EAGAIN` error
* socket.receive() // -> promise rejection with `EBUSY` error
* ```
*
* **Note:** Due to the nature of Node.js and to avoid blocking the main
Expand Down
5 changes: 4 additions & 1 deletion src/observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,10 @@ Napi::Value Observer::Receive(const Napi::CallbackInfo& info) {
if (!ValidateOpen()) return Env().Undefined();

if (poller.Reading()) {
ErrnoException(Env(), EAGAIN).ThrowAsJavaScriptException();
ErrnoException(Env(), EBUSY,
"Observer is busy reading; only one receive operation may be in progress at "
"any time")
.ThrowAsJavaScriptException();
return Env().Undefined();
}

Expand Down
13 changes: 10 additions & 3 deletions src/socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ void Socket::WarnUnlessImmediateOption(int32_t option) const {
bool Socket::ValidateOpen() const {
switch (state) {
case State::Blocked:
ErrnoException(Env(), EBUSY).ThrowAsJavaScriptException();
ErrnoException(Env(), EBUSY, "Socket is blocked by a bind or unbind operation")
.ThrowAsJavaScriptException();
return false;
case State::Closed:
ErrnoException(Env(), EBADF).ThrowAsJavaScriptException();
Expand Down Expand Up @@ -510,7 +511,10 @@ Napi::Value Socket::Send(const Napi::CallbackInfo& info) {
if (!ValidateOpen()) return Env().Undefined();

if (poller.Writing()) {
ErrnoException(Env(), EAGAIN).ThrowAsJavaScriptException();
ErrnoException(Env(), EBUSY,
"Socket is busy writing; only one send operation may be in progress "
"at any time")
.ThrowAsJavaScriptException();
return Env().Undefined();
}

Expand Down Expand Up @@ -575,7 +579,10 @@ Napi::Value Socket::Receive(const Napi::CallbackInfo& info) {
if (!ValidateOpen()) return Env().Undefined();

if (poller.Reading()) {
ErrnoException(Env(), EAGAIN).ThrowAsJavaScriptException();
ErrnoException(Env(), EBUSY,
"Socket is busy reading; only one receive operation may be in "
"progress at any time")
.ThrowAsJavaScriptException();
return Env().Undefined();
}

Expand Down
9 changes: 5 additions & 4 deletions src/util/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ static inline Napi::Error CodedException(
}

/* This mostly duplicates node::ErrnoException, but it is not public. */
static inline Napi::Error ErrnoException(const Napi::Env& env, int32_t error) {
static inline Napi::Error ErrnoException(
const Napi::Env& env, int32_t error, const char* message = nullptr) {
Napi::HandleScope scope(env);
auto exception = Napi::Error::New(env, ErrnoMessage(error));
auto exception = Napi::Error::New(env, message ? message : ErrnoMessage(error));
exception.Set("errno", Napi::Number::New(env, error));
exception.Set("code", Napi::String::New(env, ErrnoCode(error)));
return exception;
Expand All @@ -54,13 +55,13 @@ static inline constexpr const char* ErrnoMessage(int32_t errorno) {
case EFAULT:
return "Context is closed";
case EAGAIN:
return "Socket temporarily unavailable";
return "Operation was not possible or timed out";
case EMFILE:
return "Too many open file descriptors";
case ENOENT:
return "No such endpoint";
case EBUSY:
return "Socket is blocked by async operation (e.g. bind/unbind)";
return "Socket is busy";
case EBADF:
return "Socket is closed";
case EADDRINUSE:
Expand Down
4 changes: 2 additions & 2 deletions test/unit/socket-bind-unbind-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
assert.instanceOf(err, Error)
assert.equal(
err.message,
"Socket is blocked by async operation (e.g. bind/unbind)",
"Socket is blocked by a bind or unbind operation",
)
assert.equal(err.code, "EBUSY")
assert.typeOf(err.errno, "number")
Expand Down Expand Up @@ -127,7 +127,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
assert.instanceOf(err, Error)
assert.equal(
err.message,
"Socket is blocked by async operation (e.g. bind/unbind)",
"Socket is blocked by a bind or unbind operation",
)
assert.equal(err.code, "EBUSY")
assert.typeOf(err.errno, "number")
Expand Down
4 changes: 2 additions & 2 deletions test/unit/socket-close-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
await promise
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(err.message, "Socket temporarily unavailable")
assert.equal(err.message, "Operation was not possible or timed out")
assert.equal(err.code, "EAGAIN")
assert.typeOf(err.errno, "number")
}
Expand All @@ -48,7 +48,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
await promise
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(err.message, "Socket temporarily unavailable")
assert.equal(err.message, "Operation was not possible or timed out")
assert.equal(err.code, "EAGAIN")
assert.typeOf(err.errno, "number")
}
Expand Down
42 changes: 40 additions & 2 deletions test/unit/socket-send-receive-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,50 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
assert.ok(false)
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(err.message, "Socket temporarily unavailable")
assert.equal(err.message, "Operation was not possible or timed out")
assert.equal(err.code, "EAGAIN")
assert.typeOf(err.errno, "number")
}
})

it("should throw error when attempting to send concurrently", async function() {
sockA.sendTimeout = 20
const done = sockA.send(null).catch(() => null)
try {
sockA.send(null)
assert.ok(false)
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(
err.message,
"Socket is busy writing; only one send operation may be in progress at any time",
)
assert.equal(err.code, "EBUSY")
assert.typeOf(err.errno, "number")
} finally {
await done
}
})

it("should throw error when attempting to receive concurrently", async function() {
sockA.receiveTimeout = 20
const done = sockA.receive().catch(() => null)
try {
sockA.receive()
assert.ok(false)
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(
err.message,
"Socket is busy reading; only one receive operation may be in progress at any time",
)
assert.equal(err.code, "EBUSY")
assert.typeOf(err.errno, "number")
} finally {
await done
}
})

it("should copy and release small buffers", async function() {
if (process.env.SKIP_GC_TESTS) this.skip()
const weak = require("weak-napi")
Expand Down Expand Up @@ -303,7 +341,7 @@ for (const proto of testProtos("tcp", "ipc", "inproc")) {
assert.ok(false)
} catch (err) {
assert.instanceOf(err, Error)
assert.equal(err.message, "Socket temporarily unavailable")
assert.equal(err.message, "Operation was not possible or timed out")
assert.equal(err.code, "EAGAIN")
assert.typeOf(err.errno, "number")
}
Expand Down

0 comments on commit a4b290d

Please sign in to comment.