Waiting for a function to finish in node before displaying received data

Question

I'm trying to send back results data from my API call in Node. I have an HTML form with a search bar set up that takes the search term and fires off a search with that term.

Right now the data gets sent back and is empty on the first click, but when I click again, the data from the previous call is sent back to my HTML page, so I know there's something going on in regards to callbacks. I could use a setTimeout on the res.json() but that seems a little...juvenile.

I am pretty new to Node and I know the solution is rather simple...but I can't quite seem to figure it out.

Promises, callbacks, async. Maybe I'm just a little confused on where they go...

var result;
var Questrade = require("questrade");
var qt = new Questrade("env.KEY")
qt.account = "env.ACCOUNT";

qt.on("ready", function(err) {
  if (err) {
    console.log(err);
  }
});

function searchQt() {
  qt.search(searchTerm, function(err, symbols) {
    if (err) {
      console.log(err);
    }

    result = symbols;
  });
}

app.get("/search", function(req, res) {
  searchTerm = req.query.symbol;

  searchQt();

  res.json(result);
});

I'm expecting to receive my data back on time.

Trott · Accepted Answer · 2019-02-15T01:35:19.840

Easiest and most incremental solution would be to move res.json(result) to the callback where the value of result is received.

function searchQt(res) {  // <-- note added argument
  qt.search(searchTerm, function(err, symbols) {
    if (err) {
      console.log(err);
    }
    result = symbols;
    res.json(result);   // here it is!
  });
}

app.get("/search", function(req, res) {
  searchTerm = req.query.symbol;

  searchQt(res); <-- note added argument

  // res.json(result);  <-- commented out and moved above
});

You could also make things work with promises or async/await, but since you're already using callbacks, this is the most incremental way to get to working code.

While you're in there, you should remove result from the outer scope. You don't need or want that value preserved between requests. So just res.json(symbols); and get rid of the results variable entirely.

And passing res as an argument like that above is a bit of a code smell. Let's move the call to qt.search() directly into the callback for app.get() to clean things up a tiny bit. This eliminates the searchQt() function entirely:

app.get("/search", function(req, res) {
  searchTerm = req.query.symbol;

  qt.search(searchTerm, function(err, symbols) {
    if (err) {
      // handle error here somehow. maybe something like this?
      console.log(`An error occurred: ${err}`);
      res.status(500).end('An error occurred');
      return;
    }
    res.json(symbols);
  });
});

There are still some other improvements possible here. (I'm not sure if the ready event needs to fire before a search is tried, for example. So there might be a race condition there.) Promises or async/await might make this more readable. (People seem to like those better than deeply-nested callbacks. I personally don't mind the nesting, but I get it.) But that hopefully gets you going in the right direction. (And hopefully I didn't add any bugs that weren't there before!)

@MadWard Added material to eliminate the `res` passing thing. Additional feedback welcome. — Trott, Feb 15 '19 at 01:28
Second snippet edited to include error handling + return, and to use `symbols` rather than `result`. Thanks for the comments. — Trott, Feb 15 '19 at 01:36

Waiting for a function to finish in node before displaying received data

1 Answers1