Anton Scotte Anton Scotte - 4 months ago 23
Node.js Question

Passing data between middleware logs correctly but returns undefined inside object

I'm passing data between two express middleware as one of my variables is set in a function (middleware 1) and needs to be accessed outside its function scope (in middleware 2). When I console.log req.invoice in my second middleware it logs correctly so I know that I've passed the data between the middleware correctly but when trying to use my variable to construct a new object in my second middleware the req.invoice is undefined.

var express = require('express');
var app = express();
var Invoice = require('../models/Invoice');
var router = express.Router();
var cookieParser = require('cookie-parser');
var bodyParser = require('body-parser');
var session = require('express-session');
var expressValidator = require('express-validator');
var fs = require('fs');
//Used to create a pdf invoice
var PDFDocument = require('pdfkit');

//Route
router.route('/:item')
.post(generateInvoice, sendMail, function(req, res){

});

//First middleware

var fileName, dest, invoiceNr;
function generateInvoice (req, res, next) {
//Destination for storing the invoice file
dest = __dirname + '/../static/';
//generate invoice nr
Invoice.find(function(err, invoices){
if(err) {
return res.send(err);
} else {
invoiceNr = invoices.length + 1;
fileName = 'invoice' + invoiceNr + '.pdf';
req.invoicePath = path.resolve(dest + fileName);
generate();
}
});
//Create the invoice and store in static directory
function write() {
doc = new PDFDocument();
doc.pipe(fs.createWriteStream(dest + fileName));
doc.text(invoice, 100, 100);
console.log('File written > ' + fileName + '\n Destination: ' + dest);
doc.end();
}
function generate (err){
if (err)
throw err;
if (invoiceNr !== undefined) {
write();
}
}

next();

}

//Second middleware
//I'm using mailgun-js to send the invoice via email
function sendMail(req, res, next){
//Mailgun implementation
var api_key = 'MY_KEY';
var domain = 'MY_DOMAIN';
var mailgun = require('mailgun-js')({apiKey: api_key, domain: domain});
var data = {
from: 'APP_MAIL',
to: 'example@mail.com',
subject: 'Hello',
text: 'Should include attachment!',
//req.invoicePath is undefined when it should be a filepath
attachment: req.invoicePath
//when invoicePath was set as a static string, the attachment was included in the email
//attachment: '/Users/anton/Desktop/app/src/server/static/invoice27.pdf'
};

//again I'm using mailgun-js for sending the emails
mailgun.messages().send(data, function (error, body) {
console.log('Message body: ' + body);
//This works and I get the above: '/Users/anton/Desktop...' in the console
console.log('The path to the invoice: ' + req.invoicePath);
//Works properly as well
console.log('The path is of type: ' + typeof(req.invoicePath));
});
res.end();
}


I've set req.invoicePath like this is my first Middleware.

req.invoicePath = path.resolve(dest + fileName);


A brief explanation of how to send emails with mailgun can be found on the mailgun blog here
Any help at all is much appreciated, thanks!

Answer

You have asynchronous timing issues. In your first middleware, you are calling next() BEFORE your Invoice.find() function has finished, thus executing the 2nd middleware before you set req.invoicePath.

To fix, call next() only when you are done with the asynchronous operations in the first middleware. You also need to move your variables inside of generateInvoice() so they are protected local variables and will not be trounced by another request that is in flight at the same time:

function generateInvoice (req, res, next) {
  var fileName, dest, invoiceNr;
  //Destination for storing the invoice file
  dest = __dirname + '/../static/';
  //generate invoice nr
   Invoice.find(function(err, invoices){
    if(err) {
     return res.send(err);
    } else {
      invoiceNr = invoices.length + 1;
      fileName = 'invoice' + invoiceNr + '.pdf';
      req.invoicePath = path.resolve(dest + fileName);
      generate();
      // move next() here so it is not called until after req.invoicePath is set
      next();
    }
  });
  //Create the invoice and store in static directory 
  function write() {
    var doc = new PDFDocument();
    doc.pipe(fs.createWriteStream(dest + fileName));
    doc.text(invoice, 100, 100);
    console.log('File written > ' + fileName + '\n Destination: ' + dest);
    doc.end();
  }
  function generate (err){
    if (err)
      throw err;
    if (invoiceNr !== undefined) {
      write();
    }
  }
}

There are potentially other asynchronous issues here too since I'm assuming that write() may have some async parts. And, you show an argument to generate(), but you don't pass an argument. And, if generate() executed the throw err, you don't have any handler for that to do something intelligent.

Changes I made:

  1. Moved next() to inside the Invoice.find() callback so it isn't called until after req.invoicePath is set.
  2. Moved variable declarations for fileName, dest, invoiceNr inside the generateInvoice() function so they are unique for each invocation of the function and other requests in flight at the same time will not trounce their values.

Other potential issues:

  1. You declared generate() to accept an err parameter, yet you don't pass one to it.
  2. The throw err in generate() would not be caught and do anything useful if it was ever hit.
  3. You don't have any error handling on the construction of the PDF document.
  4. If any of the construction of the PDF file is asynchronous, you are not waiting for it to finish before letting the next middleware attempt to use it so there could be a race condition where it isn't finished being written yet before you're trying to use it.
Comments