Displaying MySQL table data using Node.js

Question

I'm trying Node.js for the first time, everything works, but i'm not sure that my approach is right, can you tell me if it's ok?

In this test, i've tried to get sample data from mysql db and print it on http response.

I didn't even understand how to call the res.end(); if i have many async tasks working ^^

//IMPORTS
var http = require('http');
var mysql = require('mysql');
//---------------------------------------------------

//HTTP RESPONSE
http.createServer(function (req, res) {
    fetchActors(res);
}).listen(8080);
//--------------------------------------------------- 

//MISCS

//Database connection string
var db = mysql.createConnection({
  host: "localhost",
  user: "root",
  password: "*********",
  database: "sakila"
});

//Connection to database
db.connect(function(err) {
  if (err) throw err;
  console.log("Connected to Database!");
});

//-----------------------------------------------------

//FUNCTIONS

//Exectues queries on declared db (it can be extended if you want to use more than one db)
function executeQuery(sql, cb) {
    db.query(sql, function (err, result, fields) {
        if (err) throw err;
        cb(result);
    });
}

//Prints actors table
function fetchActors(res){
        executeQuery("SELECT * FROM actor", function(result){
        res.write("<table>");
        res.write("<tr>");
        for(var column in result[0]){
            res.write("<td><label>" + column + "</label></td>");
        }
        res.write("</tr>");
        for(var row in result){
            res.write("<tr>");
            for(var column in result[row]){
                res.write("<td><label>" + result[row][column] + "</label></td>");       
            }
            res.write("</tr>");         
        }
        res.write("</table>");
    });
}

Answer 1

Some thoughts:

• You should consider using const when declaring your dependencies instead of var
• The listener function you are passing to your http.createServer() call should really only serve as a generic request handler which, as you grow your application, should probably hold request routing logic. Your current approach limits what your server can do to only calling the fetchActors() function. Since it is such a key function, you also may want to consider making it a named function vs. a closure, such that you would have something like http.createServer(requestHandler); This would make the requestHandler function itself much more easy to unit test if you get to the point of embracing that approach in your application development.
• Consider setting a constant for your port that you can refer to and potentially change more easily than having to get to where the listen() function is called.
• You should really not be using root mysql user in any application. Define a user for the application and use it.
• You should ideally inject things like database credentials into the environment, rather than hard-coding them in your code.
• You are kind of in an in-between state with regards to managing you DB connection(s). Right now, you have a single connection that would be reused to satisfy all incoming requests, meaning that if you have high request concurrency, you could have a bottleneck of queries piling up against that connection. I would suggest you use a connection pool (something supported directly some mysql package) if you want to share connections across request (even if that "pool" consists of only a single connection right now), or you make per-request connections to your database (i.e. move the call to connect() into the request handling process).
• I am not seeing what value you derive from your executeQuery() function. You are just throwing an error (something you are then not handling at all in your calling fetchActors() function, ans something you should probably not be doing anyway since you are working within an asynchronous function). You are not using fields parameter at all, so you are actually removing information that a caller may need or could have if they just worked with mysql query() function directly.
• Since you are not handling errors at all either in executeQuery(), fetchActors(), or via listener for error event on the connection itself, you have introduced fragility in your application. If you had an error at any of these stages in query execution, it would end up being uncaught, thus stopping your entire node process!!! You need to understand the difference between synchronous and asynchronous error handling in NodeJs (this is a good resource for learning. Whenever you are working with a package, you should strive to understand it's error handling approach. Look at the error handling section of the documentation for mysql package. Make sure you have facility in a web application like this to deal with unhandled errors (both synchonous and asynchronous) so that they won't inadvertently kill your process (unless that is desired behavior) and such that you can still deliver a meaningful message (i.e. HTTP 500 response) to the calling client.
• You should strive to decouple the data retrieval/business logic portions of your application from the display rendering aspects. Your fetchActors method here is really bad in this regard. Ideally, upon successful retrieval of data from database, you should hand the returned data structure to a separate method (ideally using some sort of display templating) for rendering to HTML. That way you might ultimately get better reuse from your methods, for example is you wanted to return JSON, XML, HTML, etc. renderings derived from single method to retrieve the data.
• Don't use SELECT * queries. They are usually wasteful of bandwidth, and can actually introduce fragility into your application if you make changes to the table over time.
• You have a bunch of unnecessary comments that you really should just drop, as the code should be (and generally is) self-explanatory.

Answer 2

First of all i don't think it is a good idea to send parameters as a string to a function which might be used at different parts of your code and and also can have different different parameters according to the requirements, So i have modified your function a bit and changed string parameter to an object instead so that if some one needs to change the parameters length then he don't need to modified the function at every single place where he used this function

//Exectues queries on declared db (it can be extended if you want to use more than one db) just changed the sql parameter

function executeQuery(options, cb) {
if(!options || !options.sql)
  throw (new Error('Invalid sql statement')); 
db.query(options.sql, function (err, result, fields) {
            if (err) throw err;
            cb(result);
        });
    }

and call the function as follows

    //Prints actors table
    function fetchActors(res){
            var options = {
               sql : 'SELECT * FROM actor'
            }
            executeQuery(options, function(result){
            res.write("<table>");
            res.write("<tr>");
            for(var column in result[0]){
                res.write("<td><label>" + column + "</label></td>");
            }
            res.write("</tr>");
            for(var row in result){
                res.write("<tr>");
                for(var column in result[row]){
                    res.write("<td><label>" + result[row][column] + "</label></td>");       
                }
                res.write("</tr>");         
            }
            res.write("</table>");
        });
    }

Apart form that all the sql related issues has already been mentioned by @Mike Brant and you should follow his recommendation but i guess you are the beginner, so you must learn how to make code more useable and bug free.