Javascript and Express keep sending a file before it is made, even though I use promises

aSadProgrammer 0
Viewed 28

I have some experience in javascript, but I've never spend much time on Promises. I occasionally (clumsily) manage to implement them, but I just can't get this one working. Can someone who knows more about promises give me some feedback please?

router.post('/fileupload', function (req, res) {
    //Receive xml file

    let fileSend = new Promise ((resolve, reject) => {
        //Use XML2JS to parse the xml file
        //createExcelFile is a callback where the parsed data is exported to an excel file using excel4node
        //The newly generated excel file is saved on the server.
        var parser = new xml2js.Parser();
        parser.parseString(req.files.file.data, createExcelFile)

        console.log('file parsed')
        resolve('success')
    })

    fileSend.then(function(value) {
        //takes the newly generated excel file and sends it to the client
        res.download('./output.xlsx')
        console.log('.then visited')
    })

    fileSend.finally(function() {
        console.log('File send')
    })
    
    console.log('End')
})

And here is the terminal output:

file parsed
End
.then visited
File send

Which I figure adds up, but it only works when there is already a file in the directory. And if so, it sends the old excel file to the client, then proceeds to replace it with the new excel file. If there isn't a file already there, I get this error:

Error: ENOENT: no such file or directory, stat 'C:\Users\foobar\foobar\backend\output.xlsx'

The files themselves are perfect, even if they are consistently send one file later, so I figured I implemented the promise wrong.

Thanks in advance for your time.

EDIT

I don't think it's very relevant, but here's essentially what my createExcelFile() looks like. I'm not posting the actual function, as it is pretty huge, but this is about the same

const createExcelFile = (err, result) => {
    //result is the data gotten from parsing the xml file,

    // Create a new instance of a Workbook class
    var wb = new xl.Workbook();

    // Add Worksheets to the workbook
    var ws = wb.addWorksheet('Sheet 1');

    // Set value of cell A1 to number1 from result
    ws.cell(1, 1)
    .number(result['number1'])

    // Set value of cell B1 to number2 from result
    ws.cell(1, 2)
    .number(result['number2'])

    // Set value of cell C1 to a formula
    ws.cell(1, 3)
    .formula('A1 + B1')

    // Set value of cell A2 to 'name' from result
    ws.cell(2, 1)
    .string(result['name'])

    // Set value of cell A3 to bool from result
    ws.cell(3, 1)
    .bool(result['bool'])

    wb.write('output.xlsx');
    console.log('file written')
}

With this, the terminal would say the following:

file written
file parsed
End
.then visited
File send
express javascript
1 Answers

parseString is an asynchronous operation, but this code isn't awaiting it:

var parser = new xml2js.Parser();
parser.parseString(req.files.file.data, createExcelFile)

console.log('file parsed')
resolve('success')

So the code is incorrectly indicating "file parsed" and "success" even though nothing has been parsed yet. You're already passing a callback function, but that function probably already does something meaningful and you don't want to modify it. You can, however, wrap it in a new callback function which also handles the completion of the operation:

var parser = new xml2js.Parser();
parser.parseString(req.files.file.data, function (err, result) {
  createExcelFile(err, result);
  console.log('file parsed');
  resolve('success');
});

As a further exercise, you can check the contents of err and result to see if the operation was successful or not and conditionally resolve or reject the Promise. I'm not familiar with the library being used so you will want to debug and know more about these values, but that may look something like this:

var parser = new xml2js.Parser();
parser.parseString(req.files.file.data, function (err, result) {
  if (err) {
    reject(err);
  } else {
    createExcelFile(err, result);
    console.log('file parsed');
    resolve('success');
  }
});

Expanding on this, you don't even really need your manual Promise. Essentially you're trying to wrap an asynchronous operation in a Promise and appending callbacks to that Promise, but that operation already provides a callback. Just use it directly and simplify the code:

router.post('/fileupload', function (req, res) {
  var parser = new xml2js.Parser();
  parser.parseString(req.files.file.data, function (err, result) {
    if (err) {
      // the parsing failed, handle there error here.
      // most likely return an HTTP error code and message to the client.
    } else {
      createExcelFile(err, result);
      res.download('./output.xlsx');
    }
  });
});

The linked documentation also provides a Promise based API which you can potentially use as well. Either way, the point is that you just need to await the result of the operation before reporting success.

As an aside, createExcelFile sounds like it may also be an asynchronous operation. If that's the case then you'll want to await the result of that operation as well.

user3pn10
Related