3

Assume I have the following scenario -

async.series(
  [
    function (cbi) {
      students.getAll('student', function (err, response) {
        if (err) {
          logger.error(err);
        }
        cbi(err, response);
      });
    },
    function (cbi) {
      students.deleteAll('student', function (err, response) {
        if (err) {
          logger.error(err);
        }
        cbi(err, response);
      });
    },
    function (cbi) {
      teachers.getAll('teacher', function (err, response) {
        if (err) {
          logger.error(err);
        }
        cbi(err, response);
      });
    },
    function (cbi) {
      teachers.deleteAll('teacher', function (err, response) {
        if (err) {
          logger.error(err);
        }
        cbi(err, response);
      });
    };
  ]
);

And I want a graceful cleanup when SIGTERM is sent. That is a clean up of all the students or all teachers whichever is in progress when the signal was sent should complete and the next one should not begin.

function (cbi) {
  students.getAll('student', function (err, response) {
    if (err || GLOBAL_VAR_SIGTERM === true) {
      logger.error(err);
    }
    cbi(err, response);
  });
}

I was thinking that I should set a global variable to keep track of the SIGTERM signal.

process.on('SIGTERM', function onSigterm () {
  GLOBAL_VAR_SIGTERM = true;
}

Is there any better way to break an async series to break a SIGTERM signal?

3 Answers 3

3
+100

As @adamrights points out in his answer, the main problem in your code is you didn't call cbi(err, response) with a truthy err 1st param, which is essential to stop async.series continuing to next task in the queue.

Now your code should work, but you have a repeating pattern in your code that goes:

function (cbi) {
  students.getAll('student', function (err, response) {
    // these 3 lines appear in every callback function
    if (GLOBAL_VAR_SIGTERM) err = new Error("SIGTERM: Aborting remaining tasks");
    if (err) logger.error(err);
    cbi(err, response);
    // end of repeat pattern
  });
}

Your callback passed to every async task always do the same 3-liner thing. We know the DRY rule, it's always a good idea to extract the repeating pattern into another function to reuse it as much as possible.

So instead of repeatedly declaring anonymous functions, you should declare a factory function.

function callbackFactory(cbi) {
  return function(err, response) {
    if (GLOBAL_VAR_SIGTERM) err = new Error("SIGTERM: Aborting remaining tasks");   
    if (err) logger.error(err);
    cbi(err, response);
  }
}

// use arrow function to write more concise code
async.series(
  [
    cbi => students.getAll('student', callbackFactory(cbi)),
    cbi => students.deleteAll('student', callbackFactory(cbi)),
    cbi => teachers.getAll('teacher', callbackFactory(cbi)),
    cbi => teachers.deleteAll('teacher', callbackFactory(cbi)),
  ]
);

Advance Topic: use decorator to handle cross cutting concern

Let's explore this topic a bit more. Obviously, abort early on receiving SIGTERM is a cross cutting concern that should be separated from business logic. Suppose your business logic varies from task to task:

async.series(
  [
    cbi => students.getAll('student', (err, response) => {
      if (err) {
        logger.error(err);
        return cbi(err);
      }
      updateStudentCount(response.data.length)  // <- extra work
      cbi(err, response);
    }),
    cbi => teachers.getAll('student', (err, response) => {
      if (err) {
        logger.error(err);
        return cbi(err);
      }
      updateTeacherCount(response.data.length)  // <- different extra work
      cbi(err, response);
    })
  ]
);

Because the callback is varying, it can be difficult to be extracted into a factory function like before. From this perspective, we'd better inject the abort-early behavior to each task, keep it easy to write normal business logic.

This is where decorator pattern comes handy. But global variable isn't the best tool to implement it, we'll use event listener.

The basic interface of the decorator looks like:

// `task` will be things like `cbi => students.getAll('student', ... )`
function decorateTaskAbortEarly(task) {
  return (originalCbi) => {
    ...
    task(originalCbi)
  }
}

Below is our implementation checklist:

  • we're going to call originalCbi if we receive SIGTERM
  • but when we don't receive SIGTERM, the originalCbi is still callable inside callback of any async task like normal
  • if originalCbi is ever called once, we should unsubscribe from SIGTERM to prevent memory leak

The implementation:

function decorateTaskAbortEarly(task) {
  return (originalCbi) => {
    // subscribe to `SIGTERM`
    var listener = () => originalCbi(new Error("SIGTERM: Aborting remaining tasks"));
    process.once('SIGTERM', listener);

    var wrappedCbi = (err, response) => {
      // unsubscribe if `cbi` is called once
      process.off('SIGTERM', listener);
      return originalCbi(err, response);
    };
    // pass `cbi` through to `task`
    task(wrappedCbi);
  }
}

// Usage:
async.series(
  [
    cbi => students.getAll('student', (err, response) => {
      if (err) {
        logger.error(err);
        return cbi(err);
      }
      updateStudentCount(response.data.length)
      cbi(err, response);
    }),
    cbi => teachers.getAll('student', (err, response) => {
      if (err) {
        logger.error(err);
        return cbi(err);
      }
      updateTeacherCount(response.data.length)
      cbi(err, response);
    })
  ].map(decorateTaskAbortEarly)  // <--- nice API
);
2
  • 1
    The if (err) logger.error(err); should simply be placed in the callback of async.series, not in each individual callback.
    – Bergi
    Apr 27, 2019 at 13:17
  • @Bergi I know, but I just want to keep the irrelevant part as-is, reduce noise so important part gets more highlight.
    – hackape
    Apr 27, 2019 at 13:22
3

If you want to respond to the SIGTERM event from within the async.series() then you are correct, the easiest way would be to track with a global variable.

But you need to set the first parameter (error-first callback) in the cbi(err, response) function to true in order to break the series.

so:

if (err || GLOBAL_VAR_SIGTERM === true) {
  logger.error(err);
}

should be more like:

if (err) logger.error(err);
if (GLOBAL_VAR_SIGTERM) err = new Error("SIGTERM: Aborting remaining tasks");
// You could just do err = true
// But best practice is to use an Error instance.

Then because cbi(err, response) will be called with err value equal totrue the remaining tasks will not be run.

1

I liked the other answers. This is another way to achieve the same. I am using my own example:

var async = require('async');
var ifAsync = require('if-async')
var GLOBAL_VAR_SIGTERM = false;

async.series({
    one: ifAsync(notsigterm).then(function (callback) {
        setTimeout(function () {
            console.log('one');
            callback(null, 1);
        }, 1000);
    }),
    two: ifAsync(notsigterm).then(function (callback) {
        setTimeout(function () {
            console.log('two');
            callback(null, 2);
        }, 1000);
    }),
    three: ifAsync(notsigterm).then(function (callback) {
        setTimeout(function () {
            console.log('three');
            callback(null, 3);
        }, 1000);
    }),
    four: ifAsync(notsigterm).then(function (callback) {
        setTimeout(function () {
            console.log('four');
            callback(null, 4);
        }, 1000);
    }),
}, function (err, results) {
    if (err) {
        //Handle the error in some way. Here we simply throw it
        //Other options: pass it on to an outer callback, log it etc.
        throw err;
    }
    console.log('Results are ' + JSON.stringify(results));
});

process.on('SIGTERM', function onSigterm () {
    console.log('SIGTERM caught');

  GLOBAL_VAR_SIGTERM = true;
});

function notsigterm(callback) {
    if (!GLOBAL_VAR_SIGTERM) return callback(null, true)
    else return callback(null, false)
}

I am using a package called ifAsync which allows you to use a predicate notsigterm to decide if a callback should be invoked or not. If notsigterm returns true then the callback will be invoked else it will be skipped. This is similar answer to others but somehow I find this cleaner. Let me know if you have questions.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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