Calling methods from other files with Node.js

Question

I'm failing to require methods from my ./db/index.js into my server.js file to select data from the database and display it.

The /db/index.js is like this:

'use strict';

const pgp = require('pg-promise')();
const pg = pgp(process.env.DATABASE_URL);

let select = () => {
    pg.any('SELECT username, status FROM status')
        .then(function(data){
            for (var item of data) {
                return item.username + "'s status is " + item.status;
            }
        })
        .catch(function(err) {
            return 'Error: ' + err.message || err;
        });
};

module.exports = () => {
    select
};

and I want to call it in from a different file:

'use strict';

const port = process.env.PORT || 3000;
const bodyParser = require('body-parser');
const express = require('express');
const app = express();
const db = require('./db/');

app.use(bodyParser.urlencoded({extended: true}));

app.post('/logdash', function(req, res, next) {
    res.status(200).send(db.select());
});

app.listen(port, function() {
    console.log('Server is running on port', port);
});

I'm using Heroku, and like this, watching the logs, no error is shown in both terminal and Slack (it's a slash command). I can't find help on how to properly separate the functions. How can I call this select method and any other one from a different file?

score 1 · Answer 1 · edited May 23 '17 at 11:58

1

The code in your module is asynchronous. You can't return a value directly. Instead, you should return the promise and then use the promise from the caller to obtain the final asynchronous value.

For further discussion of this general concept see this answer:

How do I return the response from an asynchronous call?

Change your code to this (see embedded comments):

'use strict';

const pgp = require('pg-promise')();
const pg = pgp(process.env.DATABASE_URL);

let select = () => {
    // return the promise here
    return pg.any('SELECT username, status FROM status')
        .then(function(data){
            return data.map(function(item) {
                return item.username + "'s status is " + item.status;
            });
        })
        .catch(function(err) {
            // to keep this an error, we have to rethrow the error, otherwise
            // the rejection is considered "handled" and is not an error
            throw 'Error: ' + err.message || err;
        });
};

// export the function
module.exports.select = select;

And call it like this:

'use strict';

const port = process.env.PORT || 3000;
const bodyParser = require('body-parser');
const express = require('express');
const app = express();
const db = require('./db/');

app.use(bodyParser.urlencoded({extended: true}));

app.post('/logdash', function(req, res, next) {
    db.select().then(function(data) {
        res.status(200).json(data);
    }).catch(function(err) {
        // add some sort of error response here
        res.status(500).json(err);
    });
});

app.listen(port, function() {
    console.log('Server is running on port', port);
});

Summary of changes here:

In select(), return the promise
In the .catch() in select(), rethrow the error to keep it a rejected promise. If you add a handler for a .catch() and don't either rethrow or return a rejected promise, then the error is handled and the promise becomes resolved.
You need to fix your for loop. It should not be doing a return inside the for loop with no conditional checks. That code is probably just wrong (though I'm not sure what you intended to do).
When you call db.select(), use a .then() handler to get teh final resolved value.
Add an error handler for the db.select() promise.
Change the exports so that db.select() is your function.
Revised the way you were referencing data in the for loop so it will actually fetch the desired property.

edited May 23 '17 at 11:58

Community

1
1

answered May 26 '16 at 00:36

jfriend00

683,504
96
985
979

1

It is not enough, his method will always resolve with `undefined`. – vitaly-t May 26 '16 at 01:31
According to Heroku's logs, db.select is not a function. – May 26 '16 at 01:34
@GabrielMFernandes - Yeah, you also have to change your exports. so that `db.select()` is your function. See my modified answer. – jfriend00 May 26 '16 at 01:58
@vitaly-t - I don't know what you mean. The select method in my answer returns a promise. – jfriend00 May 26 '16 at 02:03
@GabrielMFernandes - Also remember, you need to fix your `for` loop as what it is currently doing looks wrong, but I don't know what you expect it to accomplish. I have made one fix to it, but if you want more help with that, then please describe what you are trying to accomplish with that loop. – jfriend00 May 26 '16 at 02:04
@jfriend00 I referred to what you marked later as `// WHY do you have a return statement inside a for loop, makes no sense`. That code does in fact make no sense. So I assumed he wanted a `map` logic, which is what I showed in my answer. Sorry, if my `map` idea looks unusual (I am the author of `pg-promise`) ;) – vitaly-t May 26 '16 at 02:05
@vitaly-t - Yes, and my answer tells the OP in several places that they need to fix that `for` loop, but they have not provided ANY info on what it is supposed to do so there's only so far we can go without more info. You made a guess as to what the OP intended to do. I prefer to ask them to explain what they are trying to accomplish. – jfriend00 May 26 '16 at 02:05
The `for` loop was suppose to iterate over the result set and return it, because I want to display in the Slack chat all the usernames and status each one has set at that moment. That's it. – May 26 '16 at 02:08
@GabrielMFernandes - Assuming `data` was an array of objects, I've edited the code to make your promise resolve with an array of status strings. You don't want to directly `res.send()` that array. I don't know if you want to turn it into JSON or HTML. I've modified my answer to return the array as JSON with `res.json()`. – jfriend00 May 26 '16 at 02:15
@jfriend00, thank you for your time, but vitaly-t helped me! – May 26 '16 at 02:27
1

@GabrielMFernandes - So you learned nothing from all the programming faults I explained in your code? I'm trying to teach you and explain where you went wrong, not just write free code for you. – jfriend00 May 26 '16 at 02:30
@jfriend00 I feel bad. Does this make me an enabler? :) There, a plus, 'cos I like your answer ;) – vitaly-t May 26 '16 at 02:41
1

@vitaly-t - You have a better solution because you know that code better, but we should endeavor to explain/teach, not just write code for them. – jfriend00 May 26 '16 at 02:52
I haven't only copied it. You two taught me more of Node.js and I appreciate it. – May 26 '16 at 03:50

vitaly-t · Accepted Answer · 2017-06-25T18:40:40.613

There are many problems in your code, some of them listed in the previous answer by @jfriend00.

I will only add that you also do not return any data from the method when it is successful.

Considering how many errors you got there, rather than re-iterating them, I will give you a corrected code example instead.

The database module:

'use strict';

const pgp = require('pg-promise')();
const db = pgp(process.env.DATABASE_URL);

let select = (req, res, next) =>
    db.map('SELECT username, status FROM status', null, row=> {
        return row.username + "'s status is " + row.status;
    })
        .then(data=> {
            res.status(200).send(data);
        })
        .catch(err=> {
            res.status(500).send(err.message || err);
        });

module.exports = {
    select
};

And the HTTP service file:

'use strict';

const port = process.env.PORT || 3000;
const bodyParser = require('body-parser');
const express = require('express');
const app = express();
const db = require('./db/');

app.use(bodyParser.urlencoded({extended: true}));

app.post('/logdash', db.select);

app.listen(port, function () {
    console.log('Server is running on port', port);
});

The code is based on pg-promise v.4.3.x (upgrade, if you have an older one).

I wouldn't say it is a good approach to organizing your code, but at least it is a working example. You can check out pg-promise-demo for a complete application example that may give you a better idea of how to organize your database code.

API references: map

Hi @vitaly-t, the error `db.map is not a function` is showing now. — , May 26 '16 at 02:10
@GabrielMFernandes as stated in the answer, `The code is based on pg-promise v.4.3.x (upgrade, if you have an older one).` — vitaly-t, May 26 '16 at 02:12
That was it. Like this, it's working beautifully. I've taken a look at the repository you suggested me to look, and it's very interesting. Thank you. — , May 26 '16 at 02:26

score 0 · Answer 3 · answered May 26 '16 at 02:29

A few things. I would make sure that your select function returns a Promise. I would also handle the promise in your route. This way you can properly send the appropriate status codes and responses.

db/index.js

'use strict';

const pgp = require('pg-promise')();
const pg = pgp(process.env.DATABASE_URL);

let select = () => {
  return pg.any('SELECT username, status FROM status')
}

module.exports = () => {
    select
};

server.js

'use strict';

const port = process.env.PORT || 3000;
const bodyParser = require('body-parser');
const express = require('express');
const app = express();
const db = require('./db/');

app.use(bodyParser.urlencoded({extended: true}));

app.post('/logdash', function(req, res, next) {
    db.select()
        .then((data) => {
          res.status(200).json(data)
        })
        .catch((error) => {
          res.status(500).json(error)
        })
});

app.listen(port, function() {
    console.log('Server is running on port', port);
});

I did not test this, but it should do the trick.

Isn't there a problem with the exports. You export a function, but never call it. — jfriend00, May 26 '16 at 02:52
But, isn't `db` a function that when called return an object with `select` in it? Shouldn't they change the exports to just return an object or use `db().select()`. — jfriend00, May 26 '16 at 03:26

Calling methods from other files with Node.js

3 Answers3