node.js Syslog server recording to MS-SQL database

Question

This 43 line program is designed to listen for Syslog messages on port 514 and record the messages to an MS SQL Server database table.

I would appreciate feedback from experienced node.js developers.

I am just starting to learn node.js. I'm average at JavaScript, still getting my head around closures. I am really struggling to get my head around the node.js event driven programming paradigm.

If I try to access the SQL server connection outside of the sql.open callback function, I get the error that the database connection is closed.

So for this reason I had to place the receive message and save it in database logic inside the sql.open callback function, which seems messy and counter-intuitive, but that is probably due to my not quite fully understanding the event driven model?

Anyway the code works (amazingly!) and seems to be quite efficient, saving many dozens of syslog messages per second to the local MS SQL database with negligible CPU impact...

Are there any fundamental flaws in this code, and any changes that should be made to improve it?

I guess I am amazed that this can be done in just one page of code. node.js is very cool...

var dgram = require("dgram");
var server = dgram.createSocket("udp4");
var sql = require('msnodesql');

var conn_str = "Driver={SQL Server Native Client 11.0}; Server=.; Database=SYSLOG; UID=xxxxxxx; PWD=xxxxxxxx;";

// open the database connection
sql.open(conn_str, function (err, conn) {
    if (err) {
        console.log("error", err);
        return;
    }
    //database connection succeeded, log a mesage into the syslog.incoming table
    conn.queryRaw("insert into Incoming(Source,Message) values('LOCAL','node.js syslog server started')");

    //create an event listener for when a syslog message is recieved
    server.on("message", function (msg, rinfo) {
      //sanitise the data by replacing single quotes with two single-quotes
      var message = msg.toString().replace(/'/g, "''") 
      var src = rinfo.address.toString().replace(/'/g, "''")
      var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";

      //send the SQL to the database
      conn.queryRaw(s, function (err, results) {
        if (err) {
            console.log(s);
            console.log(err);
            return;
        }
      });
    }); // end of server.on("message") listener

    //create an event listener to tell us that the has successfully opened the syslog port and is listening for messages
    server.on("listening", function () {
      var address = server.address();
      console.log("server listening " + address.address + ":" + address.port);  

    });

    //bind the server to port 514 (syslog)
    server.bind(514);

});// end of sql.open()

Answer 1

From a once over:

• Comments should be worth the space they take up, my favourite offender is this one:

  // open the database connection
  sql.open(conn_str, function (err, conn) {
  

• I want to point to is msnodesql maintained?
• I also want to point out that storing uid/pwd in your script can be dangerous, I hope this is not a production database, which does not have sensitive data
• As Nelson Menezes mentioned, you should look into using prepared statements

• In this block, you should either use 1 comma separated var or semicolons:

  var message = msg.toString().replace(/'/g, "''") 
  var src = rinfo.address.toString().replace(/'/g, "''")
  var s = "insert into Incoming(Source,Message) values('"+src+"','"+message+"')";
  

• In a script of this size I would consider merging some statements, this

  console.log(s);
  console.log(err);
  return;
  

  might as well be

  return console.log(s, '\n', err);
  

• });// end of sql.open() <- Overkill comments

Still, all in all a solid script.