7

I have this function right here:

function createInsta(email, userid) {
  if (!fs.existsSync('users/' + email + '/' + userid)) {
    fs.mkdir('users/' + email + '/' + userid)
    fs.writeFileSync(('users/' + email + '/' + userid +  '/instagram.json'), fs.readFileSync("data/data.json", "utf-8"))
    console.log("insta folder created");
  }
  console.log("Initializing Data")
  var data = fs.readFileSync('users/' + email + '/' + userid +  '/instagram.json', "utf-8")
  data = JSON.parse(data);
  var result;
  request(("https://instagram.com/" + userid + "/?__a=1"), function(error, response, body) {
    var res = JSON.parse(body);
    result = res.user.followed_by.count
    if (error) {
      console.log(err)
    }
  })
  while (result === undefined) {
    deasync.sleep(100)
  }
  data.STARTING_COUNTS[0].DAY = result;
  data.STARTING_COUNTS[0].WEEK = result;
  data.STARTING_COUNTS[0].MONTH = result;

}

right after the console prints outs Initializing Data, the console says Segmentation fault

I have no clue why this is, my code does not seem like anything could cause such an error.

Any pointers? Thanks

7
  • 9
    "Any pointers" - pun intended?
    – joews
    Oct 18, 2016 at 21:30
  • I don't like the look of -> while (result === undefined) {deasync.sleep(100) } And why is it needed?
    – Keith
    Oct 18, 2016 at 21:31
  • @Keith I used it as a simple and quick hack around the async model of nodejs for this function, Ive used it in other functions with no problem, I doubt that its the cause of the segfault
    – Vikaton
    Oct 18, 2016 at 21:36
  • @joews Not when I first posted this, but thats pretty funny haha
    – Vikaton
    Oct 18, 2016 at 21:36
  • I would say it's the most likely candidate for the seg fault,. github.com/abbr/deasync/issues/48 In theory, it should be impossible to get a seg fault using node.js in pure form, as javascript is inside a sandbox, but your deasync is a c++ hack, that certainly can seg fault. Node.Js is designed from the ground up to be async, trying to force into sync mode is only asking for trouble,..
    – Keith
    Oct 18, 2016 at 21:42

2 Answers 2

5

As promised (pardon the pun), here is getting started into converting your code into Promise / await.

This is kind of a start of making your code into using promises, there is more error checking needed etc, but I've deliberately done as little to the code for now too show changes, and we can refactor as we go on.

eg. all that fs.existsSync etc, want's making into promises, so we can get rid of all the sync stuff. Your node.js App will love you for it.

Later, we can do more. Promise's built into browser/node are fine, but I also find a Promise lib can make things easier, I would suggest bluebird -> http://bluebirdjs.com/docs/getting-started.html It might be worth having a good read there too, bluebird has some useful utility functions like promisify that will make your fs.func's easier.

So I think this should do for now, and we'll do another refactor later.

//lets mark this function as async..
async function createInsta(email, userid) {
  return new Promise(function (resolve, reject) {
    if (!fs.existsSync('users/' + email + '/' + userid)) {
      fs.mkdir('users/' + email + '/' + userid)
      fs.writeFileSync(('users/' + email + '/' + userid +  '/instagram.json'), fs.readFileSync("data/data.json", "utf-8"))
      console.log("insta folder created");
    }
    console.log("Initializing Data")
    var data = fs.readFileSync('users/' + email + '/' + userid +  '/instagram.json', "utf-8")
    data = JSON.parse(data);    
    request(("https://instagram.com/" + userid + "/?__a=1"), function(error, response, body) {
      if (error) {
        console.log(err)
        return reject(error);
      }
      var res = JSON.parse(body);
      var result = res.user.followed_by.count
      data.STARTING_COUNTS[0].DAY = result;
      data.STARTING_COUNTS[0].WEEK = result;
      data.STARTING_COUNTS[0].MONTH = result;
      resolve(data); //i'm assuming data is what your wanting to return
    })
  });
}

//to be able to use await our function also needs to be async
async function run() {
 var data = await createInsta('bob@bob.com');
}

4
  • thanks! I actually did look into bluebird before reading this and refactored my callback code into promises : gist.github.com/Vikaton/18cf0891e8c00ff2d7cbfdbfdca83e26 However yours looks a lot more concise, is your code better than the promisified one?
    – Vikaton
    Oct 20, 2016 at 0:08
  • Promisified, is just a term used by Bluebird to convert Node callback's into Promise based functions. I've added a new post that shows this in action.
    – Keith
    Oct 20, 2016 at 12:49
  • Where you're doing fs.mkdir, that should either be fs.mkdirSync, or a callback should be passed. Apr 27, 2019 at 9:24
  • 1
    @jonschlinkert Well spotted, yes. I'd just copied the OP's code here and I was just refactoring, the second post I did below does things much better. Not sure why I didn't promisify exists though.
    – Keith
    Apr 29, 2019 at 9:28
4

Ok, lets refactor our last post.

There was only one reason we use the new Promise() constructor, and that was because the request is Async. In fact I made a slight mistake really as some errors won't be handled in the correct way. What I should have done is move the new Promise() to just before the request.

Now one great thing about Promises is that error's will propagate down the chain, in our example so far we have a console.log() inside our request object. What would be nice is that all un-handled errors are handled in one place. In this example the run() function seems a nice place.

If you now look at your createInsta function you would think it's using all sync function, but in fact apart from existsSync everything else is async. Like mentioned in the comments, existSync really wants removing anyway,. Instead just create the directory and catch the error in a try catch, or even better make a utility function say called tryMkDir, that does this (promise based of course).. :)

One last note, await / async is a pretty new javascript, I think it's ES7, and I don't believe it's yet even finalised. But you can use it now, by using something like Babel.

//lets make a Promise based request.
//There is an NPM module that would do this and more
//but this is a good example of making existing callback
//functions into a Promise.

async function promRequest(url) {
  return new Promise(function(resolve, reject) {
    request(url, function(error, response, body) {
      if (error) return reject(error);
      //promises can only return one thing
      //so use an object literal to return more
      resolve({
        response:response,
        body: body
      });
    });
  });
}

//lets also promisify some Node stuff.
var Promise = require('bluebird');

//there is a promisifyall, but I like to be
//specific about what I promisify.
var fsWriteFile = Promise.promisify(fs.writeFile),
    fsReadFile = Promise.promisify(fs.readFile),
    fsMkdir = Promise.promisify(fs.mkdir);


//because promRequest is already a promise
//we can now get rid of the new Promise contructor here.
//The only part of this function that is now async
//is existsSync, the exists() in node is depreciated
//due to potential race conditions, really this 
//should be altered to just try and create dirctory
//and catch error if exists.
async function createInsta(email, userid) {
  if (!fs.existsSync('users/' + email + '/' + userid)) {
    await fsMkdir('users/' + email + '/' + userid);
    await fsWriteFile(('users/' + email + '/' + userid +  '/instagram.json'), await fsReadFile("data/data.json", "utf-8"));
    console.log("insta folder created");
  }
  console.log("Initializing Data")
  var data = await fsReadFile('users/' + email + '/' + userid +  '/instagram.json', "utf-8")
  data = JSON.parse(data);    
  var res = JSON.parse(
    await promRequest("https://instagram.com/" + userid + "/?__a=1").body);
  var result = res.user.followed_by.count
  data.STARTING_COUNTS[0].DAY = result;
  data.STARTING_COUNTS[0].WEEK = result;
  data.STARTING_COUNTS[0].MONTH = result;
  return data;
}

//to be able to use await our function also needs to be async
async function run() {
  //lets catch all unahandled errors here.
  try {
    var data = await createInsta('bob@bob.com');
  } catch (e) {
    //fantasic any error's can now be logged
    //eg. fsMkdir might have failed and it's still
    //going to be logged here..
    console.log(e);
  }
}

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.