1

I have this request handler, it looks like this:

router.post('/', function(req,res) { 
  var screencast;
  var channel;

  youtube.get(req.body.videoId).then(function(data) {
    screencast = data.screencast;
    channel = data.channel;
  }).then(function() {
    return connection.beginTransactionAsync();  
  }).then(function() {
    return connection.queryAsync('INSERT IGNORE INTO Channels SET ?', channel);
  }).then(function() {
    return connection.queryAsync('INSERT INTO Screencasts SET ?', screencast)
  }).then(function() {
    return connection.commit();
  }).error(function(e) {
    connection.rollback();
  });
});

Sometimes I want to break the promise chain early and send an error. Here is what I tried:

router.post('/', function(req,res) { 
  var screencast;
  var channel;

  youtube.get(req.body.videoId).then(function(data) {
    if (data === null) {
      res.send('error: video does not exist.');
      return;
    }
    screencast = data.screencast;
    channel = data.channel;
  }).then(function() {
    console.log('trace: starting transaction');
    return connection.beginTransactionAsync();  
  }).then(function() {
    return connection.queryAsync('INSERT IGNORE INTO Channels SET ?', channel);
  }).then(function() {
    return connection.queryAsync('INSERT INTO Screencasts SET ?', screencast)
  }).then(function() {
    return connection.commit();
  }).error(function(e) {
    connection.rollback();
  });
});

While the error is shown, the promise chain demonstrably does not break; the next function is called.

My question is, how do I break the promise chain early to send an error?

Something that I tried was to introduce one more level of indentation:

youtube.get(req.body.videoId).then(function(data) {
  if (data === null) {
    res.send('error: video does not exist.');
    return;
  }
  connection.beginTransactionAsync().then(function() {
    return connection.queryAsync('INSERT IGNORE INTO Channels SET ?', channel);
  }).then(...

I found this to work however, I am apprehensive to use more nesting, as I also need to check whether the video has already been stored in the database before processing, requiring two levels of indentation, maybe more.

I found two other answeres (one) however, they have not been helpful to me. My question relates specifically to Bluebird and Express.

Community
  • 1
  • 1
Angular noob
  • 437
  • 4
  • 11
  • Don't think of it as *breaking*, but rather as *branching* (like `if` statements), and the indentation feels far more natural. As a bonus, if you nest the functions you don't need those ugly global variables any more. – Bergi May 06 '15 at 19:22
  • I am prominently a C# developer - we have had the `async` keyword since 2012 - so it is hard. But thank you for your comment. – Angular noob May 07 '15 at 12:52
  • You may want to look into babel (ES6 transpiler with `async` support) or Bluebirds `Promise.coroutine` then. They strip away all the `then` callback fluff, and allow you to use an early `return` from your function. – Bergi May 07 '15 at 13:28

1 Answers1

3

I think you just need to add a .catch callback to your promise chain and throw whenever you reach an issue that should break out of the promise.

router.post('/', function(req,res) { 
  var screencast;
  var channel;

  youtube.get(req.body.videoId).then(function(data) {
    if (data === null) {
      throw new Error('video does not exist.');
    }
    screencast = data.screencast;
    channel = data.channel;
  }).then(function() {
    console.log('trace: starting transaction');
    return connection.beginTransactionAsync();  
  }).then(function() {
    return connection.queryAsync('INSERT IGNORE INTO Channels SET ?', channel);
  }).then(function() {
    return connection.queryAsync('INSERT INTO Screencasts SET ?', screencast)
  }).then(function() {
    return connection.commit();
  }).error(function(e) {
    connection.rollback();
  }).catch(function(err){
    res.send("error: " + err);
  });
});
Rob M.
  • 35,491
  • 6
  • 51
  • 50
  • This is problematic, OP needs to throw a `Promise.OperationalError` so that the transaction will be rolled back, or you should throw one in the generic catch. A catch-all isn't the best idea here. – Benjamin Gruenbaum May 06 '15 at 22:36
  • 1
    In the particular place I threw the error, no rollback is needed and throwing an `OperationalError` would get caught in the `error` handler, which would roll back a non-existing commit and not send the error message. However, if an underlying error in the SQL library takes place or OP wants to escape the promise chain mid-transaction, yes a `Promise.OperationalError` should be thrown so that it will be caught by the existing `.error` handler and rollback the transaction. Am I missing something? – Rob M. May 06 '15 at 22:51