1

Very much a nodejs noob, trying to make sense of promises, await, async. I promise you I did my due diligence research (spent the entire day studying to get the code hereunder. I'm still not entirely sure everything is as it should be and can find no reference that has the exact same thing (or close enough) to what I'm trying to do here.

Thanks for any help.

General structure:

function msg() is waiting for 4 functions to complete, the 4 api calls (code only shows one): function redditPromise().

redditPromise() calls async function redditGet() -> that's the one that will call reddit API and in the meantime save the API data to database. (function saveToDb())

var nodeSocialApi = require('node-social-api');
var Socstat = require('../proxy').Socstat;

exports.index = function (req, res, next) {

/* SAVES DATA TO MONGODB */
function saveToDb(website,total) {
     //** Start : NewAndSave
     Socstat.newAndSave(website, total, "random", function (err, socstat) { // newAndSave -> proxy/socstat.js
          if (err) {
               return next(err);
          }
     });
     //** End : NewAndSave
}
/* END SAVES DATA TO MONGODB */

/* GET DATA FROM REDDIT API */
const reddit = new nodeSocialApi.Reddit(); // no auth needed

async function redditGet() {
   let result;

   await reddit.get('r/about/about.json')
   .then((data) => {

   // callback value for promise
   result = data.data.subscribers;
   saveToDb("reddit",result);

   }) // end then
   .catch(err => console.log(err));

   return result;
}
/* END : GET DATA FROM REDDIT API */

/* REDDIT PROMISE (all the others look the same) */
function redditPromise() {
     return new Promise(resolve => {
         resolve(redditGet());
     });
}
/* END : REDDIT PROMISE (all the others look the same) */

/* ONE FUNCTION THAT WAITS FOR ALL PROMISED FUNCTIONS */
async function msg() {
  const [a, b, c,d] = await Promise.all([githubPromise(), twitterPromise(), redditPromise(), facebookPromise()]);

  console.log(a + " " + b + " " + c  + d);
}
/* END: ONE FUNCTION THAT WAITS FOR ALL PROMISED FUNCTIONS */

msg();

}; // END exports
Wayfarer
  • 620
  • 6
  • 16
  • Are you getting an error while running the code? – raghu Nov 29 '17 at 04:38
  • Is there a problem with the code? It seems like the code would work. Maybe this should better be posted at [codereview.SE] – Bergi Nov 29 '17 at 04:39
  • Why does `twitterPromise` call `redditGet`? And really, this whole function is superfluous - you could (and should) call `redditGet` directly and would get a promise with exactly the same result. – Bergi Nov 29 '17 at 04:40
  • 1
    Your `saveToDb` function does not return a promise. It really should, so that you can await it if you need to. (And if not, you [should at least handle errors from it](https://stackoverflow.com/a/32385430/1048572)). – Bergi Nov 29 '17 at 04:43
  • There is idd no problem with the code, but that doesn't mean I'm not expecting any problems in the future. Like for example , as you pointed out, what if the database call fails. Thanks so much for taking the time. I'll spend another couple hours with the link you provided to write promises for the DB call (and other optimisations proposed by @raghu and @HMR) – Wayfarer Nov 29 '17 at 14:30

2 Answers2

2

The only function available in the code you posted is twitterPromise, I would suggest as Bergi to return promises:

const saveToDb = (website,total) =>
  //** Start : NewAndSave
  new Promise(
    (resolve,reject) =>
      Socstat.newAndSave(
        website, 
        total, 
        "random", 
        (err, socstat) => // newAndSave -> proxy/socstat.js
          (err)
            ? reject(err)
            : resolve(socstat)
      )
  );

//redditGet does not deal with errors, 
//  the caller has to deal with the errors (rejected promise)
const redditGet = async () => {
  const data = await reddit.get('r/about/about.json');
  await saveToDb("reddit",data.data.subscribers);
  //just return a promise:
  return data.data.subscribers;
};

//facebookGet does not deal with errors, 
//  the caller has to deal with the errors (rejected promise)
const facebookGet = async () => {  
  const response = await facebook.get('1118720888180564/fields=fan_count');
  console.log(response.fan_count);
  const dbResult = await saveToDb("facebook",response.fan_count)
  console.log("db entry succesfull -> " + dbResult);
  return response.fan_count;
};
//msg does not deal with errors, the caller of msg does
const msg = () =>
  Promise.all([githubPromise(), twitterPromise(), redditPromise(), facebookPromise()])

//calling msg and dealing with the error and result
msg()
.then(
  results =>
    console.log("got resullts",results)
  ,reject =>
    console.error("rejected",reject)
);

If you understand promises you could investigate async await, it has an easier syntax for people used to sync script (try catch) but in the end it will return a promise.

HMR
  • 37,593
  • 24
  • 91
  • 160
  • 1
    @RahulPrajapati I did't mean a library when talking about async but the es6 async await syntax for functions returning asynchronously: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function – HMR Nov 29 '17 at 06:58
  • 1
    Wow HMR , Bergi and @Rahul. Thanks so much. So much new stuff in here I can learn off of. Like for example the use of that underscore in _ => data.data.subscribers Your solution is definitely more elegant and less verbose than mine. Accepting answer. – Wayfarer Nov 29 '17 at 19:33
  • @RahulPrajapati Nope, that library is useless with promises. – Bergi Nov 30 '17 at 03:35
  • @HMR OP already used `async`/`await`, and it simplifies those functions that use `then` a lot. In here: `async function redditGet() { const data = await reddit.get('r/about/about.json'); const result = data.data.subscribers; await saveToDb("reddit", result); return result; }` – Bergi Nov 30 '17 at 03:56
  • @Bergi I think it would be better for someone to learn promises. async await is to satisfy C# syntax but actually still returns promises so one needs to use `Promise.all`, `Promise.race` and combinations to implement; for example parallel and throttled processing. Async await only deals with error handling in a traditional side affect way. So it may "simplify" things but actually in the end it doesn't. – HMR Nov 30 '17 at 04:14
  • 1
    @HMR Sure you can't do without promises, but it's a great syntactic sugar for `then` callbacks. – Bergi Nov 30 '17 at 04:32
  • 1
    @Bergi Yes, agree. Syntax does look a bit nicer so I updated the answer. Since the syntax sugar results in functions returning promises I thought i'd be better not to introduce `async await` syntax until OP has a better understanding of promises. Usually don't use that syntax on promise questions that show OP does not understand enough about promises. – HMR Nov 30 '17 at 05:00
  • agree with your sentiment (about I need more understanding about Promises before I even want to start optimising). Nonetheless I want to thank you again for your much valued input. really appreciated. – Wayfarer Dec 02 '17 at 02:42
0

Hoping I'm doing right by writing this as a new "answer". Basically just want to show that I'm not taking the help here for granted. Been working hard to pay my dues.

Based on the comments (@HMR) I tried converting some of my functions to async/await. I guess I'm not certain about 2 things: 1) is my final return response.fan_count waiting for the saveToDb to complete? I tried putting return response.fan_count inside the saveToDb.then promise, but that did not work. 2) Should I convert my saveToDb.then() structure to an async/await aswell ?

const facebookGet = async () => {  

facebookGet ()  {
    //just return a promise:
    let response; // outside to be able to return in the end
    try {
         response = await facebook.get('1118720888180564/fields=fan_count');
         console.log(response.fan_count);
         saveToDb("facebook",response.fan_count)
         .then(dbResult => {
              console.log("db entry succesfull -> " + dbResult); // returns the database object
         })
         .catch(err => console.error(err));
    }
    catch (err) {
         console.log("fetch failed ", err);
    }
return response.fan_count;
}

Answering my own question 1)

putting a "return" before "saveToDb" ... allows me to write "return response.fan_count" inside the ".then"

Wayfarer
  • 620
  • 6
  • 16
  • 2) if you don't want to wait for the database-saving (as currently), you shouldn't use `await` here. – Bergi Nov 30 '17 at 03:59
  • 1
    updated my answer to use the async await syntax. I assume facebookGet and redditGet do not want to deal with errors but caller will. Since you are using `Promise.all` some may resolve and some may reject, if you want to undo actions when some functions fail then it gets a bit more complicated. You have to write undo functions and see what resolved (wait for all to resolve or fail). `Promise.all` does not reject with all values but reject handler is called as soon as one fails. – HMR Nov 30 '17 at 04:57