OpAsync error handler ignores errors #45

Merged
merged 2 commits into from Aug 12, 2012

Projects

None yet

1 participant

@justmoon
Collaborator

Just noticed this while borrowing the OpAsync class for my own native binding.

Consider this code:

  template <class T> static void AsyncCallback(uv_work_t* req) {
    /* ... */

    Handle<Value> error = Null();
    Handle<Value> result = Null();

    op->Result(error, result);

    if (error.IsEmpty() && result.IsEmpty() && !op->status_.ok())
      error = Exception::Error(String::New(op->status_.ToString().c_str()));

    /* ... */
  }

The intention here seems to be: If op->Result did not set either an error nor a result and if the leveldb status is not ok, then issue an exception.

However, this doesn't work, because a handle with a value of Null() is not empty.

Handle<Value> error = Null();
error.IsEmpty() // false

Instead of IsEmpty() we want to use error.IsEmpty() || error->IsNull(). So the code above should be:

  template <class T> static void AsyncCallback(uv_work_t* req) {
    /* ... */

    Handle<Value> error = Null();
    Handle<Value> result = Null();

    op->Result(error, result);

    if ((error.IsEmpty() || error->IsNull()) && (result.IsEmpty() || result->IsNull()) &&
        !op->status_.ok())
      error = Exception::Error(String::New(op->status_.ToString().c_str()));

    /* ... */
  }

Once I fixed this problem, I got a bunch of errors from the test suite. Fortunately all of them were related to NotFound errors, so I decided to simply ignore those for the purposes of the automatic error handling.

@justmoon justmoon merged commit 95057fd into my8bird:master Aug 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment