execute promises recursively nodejs

Question

the following function creates new folder on my server via xmlrpc

var createFolder = function(folder_name) {
  var defer = Q.defer();
  client.methodCall('create_folder', [sessionID, folder_name], function(err, resp) {
    if (err) {
      if (err.responseString && err.responseString.match('already exist')) {
        //call the same function recursively with folder_name+Math.round(Math.random()*100)
      } else {
        defer.reject(err);
      }
    } else {
      defer.resolve(folder_name);
    }
  });
  return defer.promise;
}

The functions creates a new folder successfully However, if folder already exists i want to fire this function again recursively with new folder name and then return it in promise so that whenever this function is called it'll return the folder name doesn't matter how many times it was executed

something like

createFolder('directory').then(function(resp){
 console.log(resp);// may return directory || directory1 .... etc
});

**EDIT ** so i manged to achieve this by passing the defer object let me know if there are more elegant ways of achieving this

var createFolder = function(folder_name,defer) {
  defer =defer ||  Q.defer();
  client.methodCall('create_folder', [sessionID, folder_name], function(err, resp) {
    if (err) {
      if (err.responseString && err.responseString.match('already exist')) {
        return createFolder(folder_name+Math.round(Math.random()*100,defer)
      } else {
        defer.reject(err);
      }
    } else {
      defer.resolve(folder_name);
    }
  });
  return defer.promise;
}

"*i manged to achieve this by passing the defer object*" - a horrible idea. It would even have been better to do `defer.resolve(createFolder(folder_name+Math.floor(Math.random()*100)));` — Bergi, Aug 20 '15 at 13:08
thanks I'll try to refactor the code and follow your approach — ahhmarr, Aug 20 '15 at 13:12

score 4 · Answer 1 · edited May 31 '17 at 04:45

4

Never do any logic in plain (non-promise) callbacks. Promisify at the lowest level:

var defer = Q.defer();
client.methodCall('create_folder', [sessionID, folder_name], function(err, resp) {
  if (err) defer.reject(err);
  else defer.resolve(folder_name);
});
return defer.promise;

Or much simpler with Q.ninvoke:

return Q.ninvoke(client, 'methodCall', 'create_folder', [sessionID, folder_name]);

Now we can start implementing our recursion. It's quite simple with a then callback, from which you can return another promise. In your case:

function createFolder(folder_name) {
  return Q.ninvoke(client, 'methodCall', 'create_folder', [sessionID, folder_name])
    .catch(function(err) {
      if (err.responseString && err.responseString.match('already exist')) {
        return createFolder(folder_name+Math.floor(Math.random()*100));
      } else {
        throw err;
      }
    });
}

edited May 31 '17 at 04:45

Quentin Roy

7,677
2
32
50

answered Aug 20 '15 at 13:04

Bergi

630,263
148
957
1,375

From what I have seen from OP's code, I don't think methodCall throw anything. As a result I do not think this code works. – Quentin Roy Aug 20 '15 at 13:44
@QuentinRoy: Well if there's an error the promise is rejected - it doesn't have to be a `throw`n exception. And we catch those errors that indicate already existing folders, trying again then. – Bergi Aug 20 '15 at 13:47
Yes but, as far as I understood, error is given as first argument of the callback provided to `methodCall` (I can't say I like this API, but it seems it is how it works here). As a result catch won't be called. This is not an exception. – Quentin Roy Aug 20 '15 at 15:30
@QuentinRoy: Yes, that's just nodejs callback ("nodeback") conventions. If an error is given as the first argument, the promise *is* rejected and `catch` callback *will* be called – Bergi Aug 20 '15 at 15:50

Quentin Roy · Accepted Answer · 2017-05-31T04:55:21.703

1

Here is a bad simple way of solving your problem:

var createFolder = function(folder_name) {
  var defer = Q.defer();
  client.methodCall('create_folder', [sessionID, folder_name], function(err, resp) {
    if (err) {
      if (err.responseString && err.responseString.match('already exist')) {
        //call the same function recursively with folder_name+Math.round(Math.random()*100)
        defer.resolve(createFolder(folder_name+Math.round(Math.random()*100)));
      } else {
        defer.reject(err);
      }
    } else {
      defer.resolve(folder_name);
    }
  });
  return defer.promise;
}

However, defer is considered bad practice. Here is a very nice article about promises.

You should favor something like:

var createFolder = function(folder_name) {
  return Q.Promise(function(resolve, reject){
     client.methodCall('create_folder', [sessionID, folder_name], function(err, resp) {
        if (err) {
          if (err.responseString && err.responseString.match('already exist')) {
            //call the same function recursively with folder_name+Math.round(Math.random()*100)
            resolve(createFolder(folder_name+Math.round(Math.random()*100)));
          } else {
            reject(err);
          }
        } else {
          resolve(folder_name);
        }
      });
  });
}

EDIT: as noted by @Bergi, this is still not right and hard to debug. Any potential errors thrown from the callback of methodCall won't actually reject the promise and will most likely be swallowed (even though this callback seems very little error-prone, it might evolve). Please refer to his answer for a better way of doing this.

Also, see the official Q doc here.

edited May 31 '17 at 04:55

answered Aug 20 '15 at 13:03

Quentin Roy

7,677
2
32
50

I did retract my downvote after you've fixed the `return` into `resolve()`, but still neither of these solutions should be favoured. – Bergi Aug 20 '15 at 13:33
@Bergi, Can you explain why? – Alexis Métaireau Aug 20 '15 at 13:45
1

@AlexisMétaireau: For one, OP was looking for an elegant way :-) Deferreds hardly are considered that these days. One might even argue that using them together with a promise call resembles the [deferred antipattern](http://stackoverflow.com/q/23803743/1048572). And last, there are some minor subtleties, like throw safety in callbacks, e.g. when `err.responseString` doesn't have a `match` method. – Bergi Aug 20 '15 at 13:51
@Bergi I agree with you. That is exactly why I proposed a second version using the Promise pattern. I kept the first though as I am not here to tell OP how he should code but to help him solve his problem. About match things, we are talking about node.js so I do not think it is an issue. – Quentin Roy Aug 20 '15 at 15:24
@QuentinRoy: whether you use deferreds or the `Promise` constructor doesn't make much difference here, it's both the same explicit promise construction. I'm not talking about the nodejs environment, but more about error-prone-ness (is that a word?). Maybe `.responseString` is not a string (unlikely) or the `createFolder` reference is no more a function after a refactoring… None of such exceptions would be caught. – Bergi Aug 20 '15 at 15:56
like you said, that's a very nice article about Promises. +1 for that. – Ninja Coding May 19 '17 at 19:35
1

@Bergi, I see your point. Any errors thrown in the callback of `methodCall` will be swallowed. – Quentin Roy May 31 '17 at 04:37

execute promises recursively nodejs

2 Answers2

Linked