14

I have a basic Node JS server which is designed to be used as an API, I've created a log and database module and I've started adding other modules to deal with different request types.

I'm using Express.js and node-mysql

When I visit /v1/group I get the following error -

TypeError: Cannot read property 'database' of undefined
    at Group.getAll (C:\code\javascript\node\api\api\v1\groups.js:12:23)
    at callbacks (C:\code\javascript\node\api\node_modules\express\lib\router\index.js:161:37) ...

So I guess after recieving a request and calling group.getAll() that this is undefined but I don't understand why, is there a way to set this or have I structured my application all wrong?

sever.js

"use strict";

var Express = require('express');
var Log = require('./database/log');
var Database = require('./database/database');
var dbConfig = require('./dbconfig.json');

var Group = require('./api/v1/groups');


//Init express
var app = new Express();

//Init log and database
var log = new Log();
var database = new Database(dbConfig, log);

var initCallback = function() {
    //Init routes
    var group = new Group(database, log);

    //Group routes
    app.get('/v1/group', group.getAll);
    app.get('/v1/group/:id', group.getByID);

    app.listen(3000);
    log.logMessage("INFO", "Listening on port 3000");
};

//Test database connection
database.getConnection(function(err, connection) {
    if (err) {
        log.logMessage("FATAL", "Error connecting to database, check database is running and the dbconfig.json file is present and correct.");
        process.exit(1);
    }
    connection.end();

    initCallback();
});

database.js

"use strict";

var mysql = require('mysql');


var Database = function(dbConfig, log) {
    this.connected = false;
    this.log = log;

    this.log.logMessage("INFO", "Connecting to database with: Host - " + dbConfig.dbhost + ", Database port - " + dbConfig.dbport + ", Database name - " + dbConfig.dbname + ", User " + dbConfig.dbuser + ", Password length - " + dbConfig.dbpass.length);

    this.pool  = mysql.createPool({
        host : dbConfig.dbhost,
        user : dbConfig.dbuser,
        port: dbConfig.dbport,
        password : dbConfig.dbpass,
        database: dbConfig.dbname
    });
};

Database.prototype.getConnection = function() {
    var args = arguments;
    return this.pool.getConnection.apply(this.pool, arguments);
};

module.exports = Database;

groups.js

"use strict";

var Group = function(database, log) {
    this.database = database;
    this.log = log;
};

Group.prototype.getAll = function(req, res) {
    console.log(this); // --> undefined

    var query = 'SELECT * FROM invgroups WHERE published = 1';

    this.database.getConnection(function(err, connection) { // --> error line
        if (err) { res.send(500, "Database error"); }

        connection.query(query, function(err, results) {
            if (err) { res.send(500, "Database error"); }
            res.send(results);
        });

        connection.end();
    });

};


Group.prototype.getByID = function(req, res) {
    console.log(this);
    res.send({name: "Group Item 1"});
};

module.exports = Group;
Sam
  • 2,771
  • 2
  • 28
  • 41

1 Answers1

34

You need to properly bind the function.

app.get('/v1/group', group.getAll);

only passes the getAll function as a handler, but the function itself has no concept of this. this is decided based on the context that is bound, or based on how the function is called. This blog post is useful for understanding how function context works.

app.get('/v1/group', group.getAll.bind(group));
loganfsmyth
  • 156,129
  • 30
  • 331
  • 251
  • Worked perfectly, thanks very much, am I structuring this the right way or should I be going about it another way? – Sam Mar 24 '13 at 22:55
  • @Sam That's a tougher question. I'd say just research, find some blog posts by other people that have written APIs and see what works best. I'm sure there are some projects on github that you could find. I haven't done much API work so I can't say much. – loganfsmyth Mar 24 '13 at 22:59
  • I've done a fair bit of research but everyone seems to have a slightly different way of doing things in Node. I guess I'll go this way and see what happens, as long as there's not something glaringly wrong with my implementation, thanks very much – Sam Mar 24 '13 at 23:04
  • 2
    sometimes I hate javascript. While I understand the way 'this' binds to the object that a method is called on, it is rather unintuitively self-contradicting at times – Felipe Jan 21 '18 at 03:48