2

I need to keep calling remote API until I get the response I need, and I would like to use the Official A+ promises in node.js. Sync psudo code:

params = { remote api call params }
while (true) {
    result = callRemoteApi(params)
    if isGood(result) {
        onSuccess(result)
        break
    }
    params = modify(params)
}

I am using the request-promise lib for requests, so the result might be something like this:

new Promise(function (resolve, reject) {
    var task = request({params})
        .then(function (result) {
            if (isGood(result)) {
                resolve(result);
            } else {
                task = request({new params}).then(this_function);
            }
        });

P.S. This is very similar to https://stackoverflow.com/a/17238793/177275, but I would like a non-q-based implementation.

Community
  • 1
  • 1
Yuri Astrakhan
  • 8,808
  • 6
  • 63
  • 97

2 Answers2

6

Something like this should work well: 

var remoteApiCall = function(num){
    // fake function to resolve promise if random number in range
    return new Promise(function(resolve, reject){
        return ((Math.random()*10) < num)
            ? resolve(true)
            : reject(false);
    })
}

function getUntil(num){
    return remoteApiCall(num).then(function(result){
        if (result) {
            return result  
        } else {
            // call again until you get valid response
            return getUntil(num)
        }
    })
}

getUntil(num).then(...)
aarosil
  • 4,848
  • 3
  • 27
  • 41
2

The following solution addresses a few specific problems:

  • how to break the (otherwise endless) loop
  • how to access the result of a previously failed attempt
  • how to incorporate your params = modify(params)

The problem can be broken down into two sections:

  • a repeater/promise runner (that itself returns a promise so we can hook onSuccess to it)
  • a promise provider - a function that creates promises for the repeater

The repeater would look like this:

function repeatUntilSuccess(promiseProvider) {
    return new Promise(function (resolve, reject) {
        var counter = 0;
        function run(failedResult) {
            var p = promiseProvider(failedResult, counter++);
            if (p instanceof Promise) {
                p.then(resolve).catch(run);
            } else {
                reject(p);
            }
        }
        run();
    });
}

The "loop" happens in this line: p.then(resolve).catch(run);. The repeater keeps calling the promise provider until the promise it returns resolves (in which case the repeater resolves) or until it no longer provides a promise (in which case the repeater rejects).

A promise provider can be any function(previousResult, counter) that returns a promise (or not, if you wish to stop the loop).

var params = {/* ... */};
function nextRequest(previousResult, counter) {
    if (counter >= 10) return "too many attempts";
    if (previousResult) params = modify(params);
    return apiRequest(params);
}

(This setup assumes that apiRequest() returns a promise)

Now you can do this:

repeatUntilSuccess(nextRequest).then(onSuccess).catch(onError);

Since your question includes the side-task of wrapping an HTTP request in a promise:

function apiRequest(params) {
    return new Promise(function (resolve, reject) {
        return request(params).then(function (result) {
            if (isGood(result)) {
                resolve(result);
            } else {
                reject(result);
            }
       });
    });
}

Open the browser console and run the following snippet to see it in action.

function repeatUntilSuccess(promiseProvider) {
    return new Promise(function (resolve, reject) {
        var counter = 0;
        function run(failedResult) {
            var p = promiseProvider(failedResult, counter++);
            if (p instanceof Promise) {
                p.then(resolve).catch(run);
            } else {
                reject(p);
            }
        }
        run();
    });
}

// mockup promise that resoves or rejects randomly after a timeout
function randomPromise(num){
    return new Promise(function(resolve, reject){
        setTimeout(function () {
            var randomNum = Math.floor(Math.random() * num * 10);
            if (randomNum < num) {
                resolve(randomNum);
            } else {
                reject(randomNum);
            }
        }, 300);
    });
}

// promise provider
function nextPromise(prev, i) {
    if (prev) console.info("failed attempt #" + i + ": " + prev);
    if (i >= 5) return "too many attempts:" + i;
    return randomPromise(100);
}

// run it!
repeatUntilSuccess(nextPromise).then(function (result) {
    console.log("success", result);
}).catch(function (result) {
    console.log("failed", result);
});
Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • 1
    http://stackoverflow.com/questions/23803743/what-is-the-deferred-antipattern-and-how-do-i-avoid-it – Benjamin Gruenbaum Feb 19 '15 at 00:41
  • @BenjaminGruenbaum I disagree. My code *looks like* that, it isn't that. (Okay, `apiReques()` could be made [marginally simpler](http://stackoverflow.com/posts/28592714/revisions#rev24bb3ea8-9d60-47f9-ae34-32fa5375fb65), but the rest?) – Tomalak Feb 19 '15 at 06:24
  • **1)** Unfortunately your last version in that Gist isn't equivalent anymore to my original idea. **2)** I don't understand how `Promise.resolve()` as a way to create a new promise is better than `new Promise()`. It's fewer lines, but is it better? (I do understand that `Promise.resolve(p)` is nicer, though.) **3)** I don't subscribe to the idea that rejected promises are like exceptions. If a promise encapsulates a process that can fail naturally (like setting a value and getting back a "not allowed") then I would expect that promise to *reject*, not resolve with a falsey value. – Tomalak Feb 19 '15 at 13:17
  • Maybe I'm wrong with my last point. Assuming I talk to an HTTP API in order to set the value `foo` to 5. That request can fail on multiple levels. It can fail physically (timeout), it can fail on server (404 or 500), it can fail in the application (access denied). I would expect a rejected promise in every one of these cases, because the promise was to set the value `foo` to five, and it didn't work. Resolving the promise with "false" for the "access denied" case does not feel right. – Tomalak Feb 19 '15 at 13:25
  • The last version is not equivalent but the one just before is. `Promise.resolve()` just gives you an empty promise - you don't have to create your own completion source and your code is not concerned with all the wiring of rejections and errors so it's a lot easier to not get wrong - your `apiRequest` function can just be `function apiRequest(){ request(params).then(function(res){ if(!isGood(res)) throw res; return res; });}`. In HTTP APIs I believe that a rejection is for a timeout or an application fail is good but I would not reject on a 404 error myself at the HTTP request layer. – Benjamin Gruenbaum Feb 19 '15 at 13:53
  • 1
    Well, when properly abstracted, `setFoo(5).then(joy).catch(noJoy)` wouldn't even expose the fact that HTTP is involved. When the server replies "no" that's not an HTTP error, but but it *still* means `noJoy` for me. Right? – Tomalak Feb 19 '15 at 14:22
  • Are you talking about an RPC failure here or a protocol failure? When you make an HTTP request the promise wrapper for that should reject if the request itself failed - not if the request was successful and the server returned a response we didn't expect. When wrapping that API in an RPC API you can check if the server did not do what we expected it to and `throw` which would give you that behavior. Anyway - if you want to talk more about this I'm often in the JS room in chat. It kind of got offtopic here. – Benjamin Gruenbaum Feb 19 '15 at 14:24
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/71265/discussion-between-tomalak-and-benjamin-gruenbaum). – Tomalak Feb 19 '15 at 14:56