strongloop / modern-syslog

modern-syslog
Other
49 stars 19 forks source link

Check that cb is a function, not non-empty #17

Closed sam-github closed 8 years ago

sam-github commented 8 years ago

This triggers a bug on node 6.x, see https://github.com/haraka/Haraka/issues/1546 and https://github.com/strongloop/modern-syslog/pull/16, and this seems to work on older nodes as well as 6.

@bnoordhuis, does this look correct to you? I'm somewhat confused by https://github.com/nodejs/nan/issues/104.

sam-github commented 8 years ago
diff --git a/core.cc b/core.cc
index e4904e3..4786530 100644
--- a/core.cc
+++ b/core.cc
@@ -30,7 +30,7 @@ class Worker: public Nan::AsyncWorker {
         void HandleOKCallback() {
             Nan::HandleScope scope;

-            if(callback->GetFunction()->IsFunction())
+            if(callback)
                 callback->Call(0, NULL);
         };

@@ -80,7 +80,10 @@ NAN_METHOD(SysLog) {

     int priority = info[0]->Int32Value();
     char* message = NULL;
-    Nan::Callback *callback = new Nan::Callback(info[2].As<Function>());
+    Nan::Callback *callback = NULL;
+
+    if (info[2]->IsFunction())
+        callback = new Nan::Callback(info[2].As<Function>());

     if(node::Buffer::HasInstance(info[1])) {
         message = dupBuf(info[1]);
@@ -90,7 +93,7 @@ NAN_METHOD(SysLog) {

     if (message) {
         Nan::AsyncQueueWorker(new Worker(callback, priority, message));
-    } else if(callback->GetFunction()->IsFunction()) {
+    } else if(callback) {
         callback->Call(0, NULL);
     }

also seems to work.

sam-github commented 8 years ago

Thanks for the comments, Ben, and nice to hear from you, even if only at this distance. Maybe I'll see you in Europe some day, hope its going well for you out there in the wilds of northern Holland :-)

bnoordhuis commented 8 years ago

Thanks, Sam. You should come visit when you're in Europe, life on a farm is great!