haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.02k stars 662 forks source link

Last event where transaction exist #1297

Closed ricardopolo closed 8 years ago

ricardopolo commented 8 years ago

We have a plugin with multiple events. In the first events it saves the attachments in the hard disk and store their path in transaction.notes.

We would like to delete the attachments before the transaction ends, even if the transaction was ok, or if other hook returns return next(DENY)

I tried to use hook disconnect but transaction is null .

So in which is the last event that runs in a transaction, even if its ok or DENY? (To always delete the files)

Thanks

cc @baudehlo @smfreegard @msimerson

baudehlo commented 8 years ago

Just hook to both deny and data_post.

Your problem though is likely related to truly knowing when you've saved your attachment vs the transaction finishing. You need to do some hoop jumping when saving attachments and pause the connection.

See the issue a couple of numbers back on what I posted about that.

On Jan 14, 2016, at 9:19 PM, Ricardo Polo notifications@github.com wrote:

We have a plugin with multiple events. In the first events it saves the attachments in the hard disk and store their path in transaction.notes.

We would like to delete the attachments before the transaction ends, even if the transaction was ok, or if other hook returns return next(DENY)

I tried to use hook disconnect but transaction is null .

So in which is the last event that runs in a transaction, even if its ok or DENY? (To always delete the files)

Thanks

cc @baudehlo @smfreegard @msimerson

— Reply to this email directly or view it on GitHub.

ricardopolo commented 8 years ago

@baudehlo

I based my code in the example that is in https://github.com/haraka/Haraka/blob/master/docs/Transaction.md

My actual code for save the attachments path is

exports.save_attachments = function(next, connection){
  connection.transaction.notes.attachments = [];
  connection.transaction.attachment_hooks(
    function (ct, fn, body, stream) {

    stream.connection = connection; // Allow backpressure
    stream.pause();

    tmp.file({ keep: true }, function (err, path, fd, cleanupCallback) {
        var ws = fs.createWriteStream(path);
        stream.pipe(ws);
        stream.resume();
        ws.on('close', function () {
          //End of stream reached
          fs.close(fd, function(){
            connection.transaction.notes.attachments.push({ content_type: ct, filename: fn, path: path, body : body, cleanupCallback : cleanupCallback});
          });
        });
    });
    }
  );
  return next();
};

And for delete is

exports.clean_attachments_from_disk = function(next, connection){
  if(connection.transaction.notes.attachments){
    connection.transaction.notes.attachments.forEach(function(entry) {
      entry.cleanupCallback();
    });
  }
};

So far it works. (The delete method not if put in the disconnect hook). Am I doing something wrong related about the pause and resume? Thanks for your answer!

baudehlo commented 8 years ago

At stream.on('end') you need to pause the connection. And when you've deleted your tmp file (or done with the contents of it) you need to resume it.

On Jan 14, 2016, at 10:24 PM, Ricardo Polo notifications@github.com wrote:

@baudehlo

I based my code in the example that is in https://github.com/haraka/Haraka/blob/master/docs/Transaction.md

My actual code for save the attachments path is

exports.save_attachments = function(next, connection){ connection.transaction.notes.attachments = []; connection.transaction.attachment_hooks( function (ct, fn, body, stream) {

stream.connection = connection; // Allow backpressure
stream.pause();

tmp.file({ keep: true }, function (err, path, fd, cleanupCallback) {
    var ws = fs.createWriteStream(path);
    stream.pipe(ws);
    stream.resume();
    ws.on('close', function () {
      //End of stream reached
      fs.close(fd, function(){
        connection.transaction.notes.attachments.push({ content_type: ct, filename: fn, path: path, body : body, cleanupCallback : cleanupCallback});
      });
    });
});
}

); return next(); }; And for delete is

exports.clean_attachments_from_disk = function(next, connection){ if(connection.transaction.notes.attachments){ connection.transaction.notes.attachments.forEach(function(entry) { entry.cleanupCallback(); }); } }; Am I doing something wrong related about the pause and resume? Thanks for your answer!

— Reply to this email directly or view it on GitHub.

ricardopolo commented 8 years ago

@baudehlo I tried it.

In the data event I pause the connection.

//Saves the attachments in notes
exports.save_attachments = function(next, connection){
  connection.transaction.notes.attachments = [];
  connection.transaction.attachment_hooks(
    function (ct, fn, body, stream) {

    stream.connection = connection; // Allow backpressure
    stream.pause();

    tmp.file({ keep: true }, function (err, path, fd, cleanupCallback) {
        var ws = fs.createWriteStream(path);
        stream.pipe(ws);
        stream.resume();
        ws.on('close', function () {
          //End of stream reached
          connection.pause();
          fs.close(fd, function(){
            connection.transaction.notes.attachments.push({ content_type: ct, filename: fn, path: path, body : body, cleanupCallback : cleanupCallback});
          });
        });
    });
    }
  );
  return next();
};

Then in the data_post event I delete the files and resume the connection

     connection.transaction.notes.attachments.forEach(function(entry) {
       entry.cleanupCallback();
     });

     connection.resume();

It work good when I send emails with just one attachment, if I send emails with two or more attachment it doesn't :disappointed:

baudehlo commented 8 years ago

Where you have connection.pause() isn't the end of the stream. It's when writing has completed, which is some time after that.

You have to pause at stream.on('end'), or Haraka will see the final dot (end of data marker) and run data_post hooks before your code has finished.

You also have to do some nastiness with attachment counting to know when you've got your last attachment processed, and keep track of whether you actually landed in data_post or not.

I can probably privately share some code with you from emailitin.com that shows all these ugly steps. Processing attachments in Haraka is harder than it should be because we chose to use streams.

On Thu, Jan 14, 2016 at 11:15 PM, Ricardo Polo notifications@github.com wrote:

@baudehlo https://github.com/baudehlo I tried it.

In the data event I pause the connection.

//Saves the attachments in notesexports.save_attachments = function(next, connection){ connection.transaction.notes.attachments = []; connection.transaction.attachment_hooks( function (ct, fn, body, stream) {

stream.connection = connection; // Allow backpressure
stream.pause();

tmp.file({ keep: true }, function (err, path, fd, cleanupCallback) {
    var ws = fs.createWriteStream(path);
    stream.pipe(ws);
    stream.resume();
    ws.on('close', function () {
      //End of stream reached
      connection.pause();
      fs.close(fd, function(){
        connection.transaction.notes.attachments.push({ content_type: ct, filename: fn, path: path, body : body, cleanupCallback : cleanupCallback});
      });
    });
});
}

); return next(); };

Then in the data_post event I delete the files and resume the connection

 connection.transaction.notes.attachments.forEach(function(entry) {
   entry.cleanupCallback();
 });

 connection.resume();

It work good when I send emails with just one attachment, if I send emails with two or more attachment it doesn't [image: :disappointed:]

— Reply to this email directly or view it on GitHub https://github.com/haraka/Haraka/issues/1297#issuecomment-171867994.

smfreegard commented 8 years ago

@baudehlo - have a look at the attachment plugin, I'm pretty sure I based my wait_for_attachment_hooks() function on your emailitin code.

ricardopolo commented 8 years ago

@baudehlo this is the emailitin ricardo24hyacinth@emailitin.com

I am a little bit confused... are the docs wrong?

baudehlo commented 8 years ago

The docs are just confusing. Real code trumps them.

I didn't mean signup for emailitin - I meant I could send you some of the code behind it. But as smf says, check the attachment plugin - it does the right things.

On Fri, Jan 15, 2016 at 5:06 PM, Ricardo Polo notifications@github.com wrote:

@baudehlo https://github.com/baudehlo this is the emailitin ricardo24hyacinth@emailitin.com

I am a little bit confused... is the docs wrong?

— Reply to this email directly or view it on GitHub https://github.com/haraka/Haraka/issues/1297#issuecomment-172107409.