Move res.json(req.body);
into the callback function.
Apart from the scoping problem: It is asynchronous, so in your code it will be called long after res.json(req.body)
runs.
app.get('/api/etsy/getListings', function(req, res) {
bEtsy.getAllListings(req, res, function(err, body) {
res.json(body);
//console.log(body);
});
});
A more general piece of advice (or two or three pieces), aside from the problem at hand:
What helps me with such situations and "callback thinking" is to almost never use inline callback functions: Write code only one layer deep (plus one layer for the module pattern of course), avoid callback hell! Name all callbacks and write them all on the same (top) level.
function allListingsReceived(err, body, res) {
res.json(body);
//console.log(body);
}
function getListings(req, res) {
// ASYNC
bEtsy.getAllListings(req, res, allListingsReceived);
}
//ASYNC
app.get('/api/etsy/getListings', getListings);
This allows me to get a much better overview over the actual call sequence. In this case, when getAllListings
is called you know it is asynchronous - in my own code I add a clear comment (like I did above). So I know anything I were to write after that async function would not be able to access anything that async function is supposed to get me. IMHO such a comment is important - in Javascript there is no way to know if a callback function is asynchronous. Usually it is, but if it's synchronous and you expect asynchronism you may get into trouble too! So I think it's better to write it as a comment (always the exact same short string throughout the whole project), a formalized code annotation. Which by the way leads to another problem: When you write functions that accept a callback function, make sure they always call it either synchronously or asynchronously, never both ways (some functions use cached values and are able to return a result right away instead of starting an async. network request).
Basically, the written structure does not reflect the runtime situation with this style - but this is okay, since the runtime situation is completely flexible anyway (if you want to change which callback function you use, or add another one in between, do you really want to shift around tons of lines of code instead of just exchanging a name? Not to mention an increase in the ease of reusability). This is much easier to read in longer callback-style code files then several layers deep nested asynchronous functions IMHO. Avoid functions inside functions, apart from the module pattern, as much as possible.
Having named functions also is much better for debugging, stack traces are much easier to read.
A note: My example code leaves one issue open: if this is inside a module (or class), those would be internal functions, and you may have to make sure about the correct context/scope (where this
points to, if you access object member variables this way from inside those functions). It works the same when those functions are on the prototype though. So this is just a general concept example that disregards this side issue.
Another note: When writing in this style variable that previously were available to an inner function via a closure - in this example res
- now have to be made available as function parameters when calling the callback function. That adds some complexity - but on the other hand forces you to create clean(er) APIs in your own code. Personally I don't like excessive reliance on closures to pass arguments. I'm too stupid, I prefer to have a clean interface definition by having all parameters a function uses in its header. Apparently I'm not alone, that is one of the advantages most often touted for functional programming :) An alternative to filling the header with arguments that is "clean" too are object properties under this
. My small example looks a little "procedural", but it only served to illustrate one single point. Of course this belongs into a larger context of modular programming.