Express.js Cannot read property 'req' of undefined

Question

My Express.js app has been throwing Cannot read property 'req' of undefined. In essence, it listens for a GET request, grab the content query, and then reply with a table. Here's the parts that present the problem.

index.js

var panels = require('./modules/panels.js');
app.get('/panel', function (req, res) {
    var user;
    if (user = req.session.user) {
        panels.getContents(req.query.content, user.innId, res.send);
    } else {
        res.sendStatus(401);
    }
});

modules/panels.js

exports.getContents = function(panelName, innId, callback) {
    var response = "";
    switch (panelName) {
        case 'tenants':
            con.query(queryString, queryParams, function(err, rows) {
                if (err) {
                    handle(err);
                } else {
                    if (rows.length == 0) {
                        var tenants = 0;
                        var debtors = 0;
                    } else {
                        var tenants = rows[0].tenants;
                        var debtors = rows[0].length;
                    }
                    response = convertToHTMLTable(rows);
                }
                callback(response); /*THE ERROR POINTS HERE */
            });
            break;

        /* other cases here */

        default:
            response += "Invalid Request";
            callback(response);
    }
}

What did I do wrong? My guess is that I'm not suppose to pass res.send as a callback. So, how can I fix it?

`user = req.session.user` should be `user == req.session.user`, exactly what line does throw the error? — Adam Azad, Jan 23 '17 at 08:26
@AdamAzad No, I want to assign the session to the user while checking if it's truthy. I put a comment up there at the erroneous line. it's in panels.js. — starleaf1, Jan 23 '17 at 08:28
@starleaf1 A single equals in an if is considered bad practice. Most of the times it happens by mistake so linters will flag it. It can also mislead other developers that look at your code. — Nimrod Shory, Jan 23 '17 at 08:33
@starleaf1 ok nice, i had a similar issue already : look [here](http://stackoverflow.com/q/40966223/6320039) for more informations about it. — Ulysse BN, Jan 23 '17 at 08:34
Now I need some explanation as to why one shouldn't pass a method from one module as a callback to a method from another module. — starleaf1, Jan 23 '17 at 08:58
@starleaf1 My answer has been up for a while, and seems to be what most people are looking for when they arrive here; would you consider switching it to the accepted answer, so that people don't get lost in the wall of code in the currently accepted answer and move on before they see mine below? It also resolves your "Note to future reads" problem. Further, I believe it answers your question from January 23rd 2017 here. Cheers! — Kyle Baker, Mar 30 '18 at 17:35

Kyle Baker · Accepted Answer · 2020-04-02T13:06:40.863

I also experienced this error, and after chewing on the error message for a bit and staring at my code for too long, It clicked.

I (and the asker above) had code that looked like this:

  somethingAsync
  .then(res.send) // <-- storing send() divorced from parent object "res"
}

The problem is that when res.send gets called later, it is divorced from its dynamic scope--which means that calls to this inside of the send method, which would normally refer to res, now refer to global. Evidently, res.send contains a reference to this.req, given the error message.

An equivalent scenario:

res = {
    send: function() { 
        console.log(this.originalMessage.req);
    },
    originalMessage: { req: "hello SO" },
};

res.send();
// # "hello SO"
const x = {};
x.send = res.send;
x.send();
// # Uncaught TypeError: Cannot read property 'req' of undefined

Or, to put it another way:

new Promise(_ => _())
.then(_ => console.log(this.msg.req));
// # Uncaught (in promise) TypeError: Cannot read property 'req' of undefined

Two solutions:

handler(req, res) {
  somethingAsync
  .then(() => res.send())
   // ^ send will be called as a method of res, preserving dynamic scope
   // that is, we're storing a function that, when called, calls
   // "res.send()", instead of storing the "send" method of "res" by itself.
}

And a theoretically more idiomatic approach is

handler(req, res) {
  somethingAsync
  .then(res.send.bind(res))
  //^ res is explicitly bound to the saved function as its dynamic scope
}

Pernicious little gotcha of dynamic scoping, that one. Seems like express should ditch the dynamic scoping there, or at least throw a better error...

Just had this issue as well. Thanks! – Chris Deacy Aug 23 '17 at 18:34 — Chris Deacy, Aug 23 '17 at 18:34

score 3 · Answer 2 · answered Jan 23 '17 at 08:28

try this in index.js

panels.getContents(req.query.content, user.innId, res);

and into panels.js

exports.getContents = function(panelName, innId, response) {
    var response = "";
    switch (panelName) {
        case 'tenants':
            con.query(queryString, queryParams, function(err, rows) {
                if (err) {
                    handle(err);
                } else {
                    if (rows.length == 0) {
                        var tenants = 0;
                        var debtors = 0;
                    } else {
                        var tenants = rows[0].tenants;
                        var debtors = rows[0].length;
                    }
                    response.send(convertToHTMLTable(rows));
                }
            });
            break;

        /* other cases here */

        default:
            error += "Invalid Request";
            response.send(error)
    }
}

Note to future readers: There are modifications that I must do before this answer worked, but I can't edit this without revealing too much of my code. — starleaf1, Jan 23 '17 at 08:35

score 2 · Answer 3 · answered Oct 15 '19 at 04:30

When you passed res.send as an argument to the getContents function (panels.getContents(req.query.content, user.innId, res.send);) it is passed in a new variable and you called it callback.

So res.send is now stripped from it's original/supposed-to-be lexical context/binding which is the res object in this case, and it appears that res.send uses the req property in the res object using this.req.

What you've done here is unintentionally changing the value of this for the function send to be the global object.

Solution

there are two ways to fix this problem

panels.getContents(req.query.content, user.innId, () => res.send);

// OR

panels.getContents(req.query.content, user.innId, res.send.bind(res));

in the second solution, you bind the function res.send to always treat any value of this to be the res object (bind it to the res object)

Express.js Cannot read property 'req' of undefined

3 Answers3

Solution