Should I use the file system asynchronously when responding http requests?-CodePudding

I have a simple backend that takes a js object from a POST request and saves it to a JSON file.

app.post('/', (req, res) => {
    const form= {name, surname, email, message}; //some object
    try {
        let forms;
        fs.readFile('./files/forms.json', {encoding: 'utf8'}, (err, str) => {
            if (err) {
                //Handle ENOENT: no such file or directory.
                if (error.code !== 'ENOENT') throw error; //If not ENOENT, rethrow.
                fs.appendFile('./files/forms.json', '', 'utf8', (err) => {if (err) throw err;});
                forms= [];
            } else {
                forms= JSON.parse(str);
            }
            forms.push(form);
        });
        fs.writeFile('./files/forms.json', JSON.stringify(forms), (err) => {if (err) throw err;});
        res.end();
    } catch (error) {
        console.log(error);
        res.status(500).send({err: "The message was received but was not saved."});
    }
});

I was wondering if the calls to the file system could block node's event loop, and therefore prevent it from responding to other incoming requests. If this is the case, what should I do to prevent it?

(Btw any suggestions to improve my code are very appreciated)

CodePudding user response：

You are actually using fs asynchronously which mean that the event loop won't be blocked

It would block the event loop if you used `fs.readFileSync``

Note that your exemple will not do what you expect

Since fs.readFile is asynchronous, the callback you provided will be called at a later time, and the next instructions of the current function will be processed immediately without waiting for your callback to be called

// this will proceed immediately before the fs.readFile callback is called which mean that in your case forms will be undefined
        forms.push(form);

// here again writeFile is asynchronous, the callback will be called at a later time
        fs.writeFile('./files/forms.json', JSON.stringify(forms), (err) => {if (err) throw err;});
// which means that in any case you will end you response even if there is an error that might be thrown later
        res.end();

What you probably want to do here is to put this part inside the callback

app.post('/', (req, res) => {
    const form= {name, surname, email, message}; //some object
    try {
        let forms;
        fs.readFile('./files/forms.json', {encoding: 'utf8'}, (err, str) => {
            if (err) {
                //Handle ENOENT: no such file or directory.
                if (error.code !== 'ENOENT') {
                  res.status(500).send();
                  return;
                }
                fs.appendFile('./files/forms.json', JSON.stringify([form]), 'utf8', (err) => {
                  if (err) {
                    res.status(500).send();
                    return;
                  };
                  res.status(200).send();
                });
                return;
            }
          forms= JSON.parse(str);
          forms.push(form);
          fs.writeFile('./files/forms.json', JSON.stringify(forms), (err) => {
            if (err) {
              res.status(500).send(...);
              return;
            }
            res.status(200).send()
          });
        });
    } catch (error) {
        console.log(error);
        res.status(500).send({err: "The message was received but was not saved."});
    }
});

As you can see all those nested callback is quite cumbersome, you can have a look at async/await synthax which may bee more readable

Also note that the errors you threw in your callback won't be propagated as you expect since the callback won't be running in the same stack call