Recursive promise failing

Question

Background

I am trying to connect to a machine using net.Socket from Node.js. Sometimes the connection is up and I succeed and other times the connection is down, so I keep retrying.

Objective

I want to write a reconnect method, that tries to connect to an IP. When it succeeds, it resolves, but if it fails it should keep trying.

Code

    const reconnect = function ( connectOpts ) {
        return new Promise( resolve => {
            connect( connectOpts )
                .then( () => {
                    console.log( "CONNECTED " );
                    resolve();
                } )
                .catch( () => {
                    console.log( "RETRYING" );
                    connect( connectOpts );
                } );
        } );
    };

    const connect = function ( connectOpts ) {
        return new Promise( ( resolve, reject ) => {
            socket = net.createConnection( connectOpts );

            //once I receive the 1st bytes of data, I resolve();
            socket.once( "data", () => {
                resolve();
            } );

            socket.once( "error", err => reject( err ) );
        } );
    };

reconnect({host: localhost, port: 8080})
    .then(() => console.log("I made it!"));

Problem

The problem is that when the connection fails, it only tries one more time, and then it blows up!

Question

How do I change my code so that I keep reconnecting every time it fails? PS: It is important that this functionality is asynchronous !

score 0 · Answer 1 · answered May 31 '17 at 11:28

As you correctly pointed out you code will blow up as you try to connect from the .catch() method.

.catch( () => {
                console.log( "RETRYING" );
                connect( connectOpts );
            } );

There are 2 options to solve this

Option 1: Using ES6 Generator functions

I was facing similar issue recently where I used generator functions to solve my problem. Essentially generator functions allow you pause the execution until the async task has gone to completion. You can then continue the execution based on the results of the async task

Reference- https://eladnava.com/write-synchronous-node-js-code-with-es6-generators/

Option 2- Using deferred promises

You can use something called as deferred promises for this use-case. More details here- https://mostafa-samir.github.io/async-recursive-patterns-pt2/

Also, a similar stackoverlow question here- execute promises recursively nodejs

Hope that helps!

score 0 · Answer 2 · answered May 31 '17 at 12:07

Try this one:

const connect = function(connectOpts) {

  const connectResolver = (resolve) => {
    socket = net.createConnection(connectOpts);
    socket.once("data", resolve);
    socket.once("error", () => 
      // you can try setTimeout() here with specific retry in milliseconds here
      // say like setTimeout(() => connectResolver(resolve), 200);
      // instead of immediate retry in the end of every event loop
      // up to you
      setImmediate(() => connectResolver(resolve)));
  }

  return new Promise(connectResolver);
};

connect({
    host: localhost,
    port: 8080
  })
  .then(() => console.log("I made it!"));

score 0 · Answer 3 · answered Jun 02 '17 at 00:52

Checking out the code, I found this.

Retry will never hit whether resolve or reject. Then UnhandledPromiseRejectionWarning could happen.

Then let's call reconnect itself in the .catch handler. When it gets resolved, settle up the Promise by calling resolve().

    const reconnect = function ( connectOpts ) {
        return new Promise( resolve => {
            connect( connectOpts )
            .then( () => {
                console.log( "CONNECTED " );
                resolve();
            } )
            .catch( () => {
                console.log( "RETRYING" );

                // connect( connectOpts );

                return reconnect( connectOpts )
                .then(() => {
                    resolve();
                });

            } );
        } );
    };

Flame_Phoenix · Accepted Answer · 2017-06-02T11:15:12.997

My solution

After reading all the other solutions, I ended up going with this one:

   const reconnect = async function ( connectOpts ) {
        let done = false;
        while ( !done ) {
            done = await connect( connectOpts )
                .then( () => true )
                .catch( () => false );
        }
    };

const connect = function ( connectOpts ) {
        return new Promise( ( resolve, reject ) => {
            socket = net.createConnection( connectOpts );

            //once I receive the 1st bytes of data, I resolve();
            socket.once( "data", () => {
                resolve();
            } );

            socket.once( "error", err => reject( err ) );
        } );
    };

reconnect({host: localhost, port: 8080})
    .then(() => console.log("I made it!"));

Why?

Generators are overkill. Generators may have a valid use case, but not in here. Using them would invalidate the principle of least astonishment, when the same can be achieved via simpler means.
Deferred promises are a well known anti-pattern. No no.
The usage of setImmediate is highly unnecessary. Instead of asking the loop execution to immediately run the code, one could just use a loop in the majority of cases (like in this one).
Recalling reconnect instead of connect will invalidate make the caller lose the connection to the reconnect call. When reconnect finally succeeds, it will be a different one and the caller of the original request wouldn't be notified.

My solution fixes all of these issues.

But, your solution is blocking!

No, it isn't. In any other language it would be, but in here since I am using the await operator I am actually allowing Node continue the execution loop. Just like a promise.