my8bird/node-leveldb — pull request #45 by justmoon (2012-08-05 00:18:37+00:00)
Title: OpAsync error handler ignores errors

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

Consider this code:

``` cpp
template static void AsyncCallback(uv_work_t* req) {
/* ... */

Handle error = Null();
Handle 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.

``` cpp
Handle error = Null();
error.IsEmpty() // false
```

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

``` cpp
template static void AsyncCallback(uv_work_t* req) {
/* ... */

Handle error = Null();
Handle 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.