DEV Community

Cover image for Refactoring node.js (Part 1)
Paula Santamaría
Paula Santamaría

Posted on • Edited on

Refactoring node.js (Part 1)

This is the first part of a series of articles where I'll share tips to write cleaner and more effective node.js code.

1. Use async/await

So there are 3 ways of writing asynchronous code in Javascript: callbacks, promises and async/await.

(If you haven't escaped callback hell yet, I encourage you to check out another dev.to article: How to Escape Callback Hell with JavaScipt Promises by @amberjones)

Async/await allows us to build asynchronous non-blocking code with a cleaner and more readable syntax than promises 👍.

Let's see an example, the following code executes myFunction(), returns the result and handles any errors that may be thrown by the function:

// Promises
myFunction()
    .then(data => {
        doStuff(data);
    })
    .catch(err => {
        handle(err);
    });
Enter fullscreen mode Exit fullscreen mode
// async/await
try {
    const data = await myFunction();
    doStuff(data);
}
catch (err) {
    handle(err);
}
Enter fullscreen mode Exit fullscreen mode

Isn't it cleaner and easier to read with async/await?

A few of extra tips regarding async/await:

  • Any function that returns a Promise can be awaited.
  • The await keyword can only be used within async functions.
  • You can execute async functions in parallel using await Promise.all([asyncFunction1, asyncFunction2]).

2. Avoid await in loops

Since async/await is so clean and readable we may be tempted to do something like this:

const productsToUpdate = await productModel.find({ outdated: true });

for (const key in productsToUpdate) {
    const product = productsToUpdate[key];

    product.outdated = false;
    await product.save();
}
Enter fullscreen mode Exit fullscreen mode

The above code retrieves a list of products using find and then iterates through them and updates them one by one. It will probably work, but we should be able to do better 🤔. Consider the following alternatives:

Option A: Write a single query

We could easily write a query that finds the products and updates them all in one, thus delegating the responsibility to the database and reducing N operations to just 1. Here's how:

await productModel.update({ outdated: true }, {
    $set: {
        outdated: false
    }
 });
Enter fullscreen mode Exit fullscreen mode

Option B: Promise.all

To be clear, in this example the Option A would definitely be the way to go, but in case the async operations cannot be merged into one (maybe they're not database operations, but requests to an external REST API instead), you should consider running all the operations in parallel using Promise.all:

const firstOperation = myAsyncFunction();
const secondOperation = myAsyncFunction2('test');
const thirdOperation = myAsyncFunction3(5);

await Promise.all([ firstOperation, secondOperation, thirdOperation ]);
Enter fullscreen mode Exit fullscreen mode

This approach will execute all the async functions and wait until all of them have resolved. It only works if the operations have no dependencies with one another.

3. Use async fs modules

Node's fs module allows us to interact with the file system. Every operation in the fs module contains a synchronous and an asynchronous option.

Here's an example of async and sync code to read a file 👇

// Async
fs.readFile(path, (err, data) => {
    if (err)
        throw err;

    callback(data);
});

// Sync 
return fs.readFileSync(path);

Enter fullscreen mode Exit fullscreen mode

The synchronous option (usually ends with Sync, like readFileSync) looks cleaner, because it doesn't require a callback, but it could actually harm your application performance. Why? Because Sync operations are blocking, so while the app is reading a file synchronously its blocking the execution of any other code.

However, it will be nice to find a way we could use the fs module asynchronously and avoid callbacks too, right? Checkout the next tip to find out how.

4. Convert callbacks to promises with util.promisify

promisify is a function from the node.js util module. It takes a function that follows the standard callback structure and transforms it to a promise. This also allows to use await on callback-style functions.

Let's see an example. The function readFile and access, from node's fs module, both follow the callback-style structure, so we'll promisify them to use them in an async function with await.

Here's the callback version:

const fs = require('fs');

const readFile = (path, callback) => {
    // Check if the path exists.
    fs.stat(path, (err, stats) => {
        if (err)
            throw err;

        // Check if the path belongs to a file.
        if (!stats.isFile())
            throw new Error('The path does not belong to a file');

        // Read file.
        fs.readFile(path, (err, data) => {
            if (err)
                throw err;

            callback(data);
        });
    });
}
Enter fullscreen mode Exit fullscreen mode

And here's the "promisified" + async version 👌:

const util = require('util');
const fs = require('fs');

const readFilePromise = util.promisify(fs.readFile);
const statPromise = util.promisify(fs.stat);

const readFile = async (path) => {
    // Check if the path exists.
    const stats = await statPromise(path);

    // Check if the path belongs to a file.
    if (!stats.isFile())
        throw new Error('The path does not belong to a file');

    // Read file.
    return await readFilePromise(path);
}
Enter fullscreen mode Exit fullscreen mode

5. Use descriptive Error types

Let's say we're building an endpoint for a REST API that returns a product by id. A service will handle the logic and the controller will handle the request, call the service and build the response:

/* --- product.service.js --- */

const getById = async (id) => {
    const product = await productModel.findById(id);

    if (!product)
        throw new Error('Product not found');

    return product;
}

/* --- product.controller.js --- */

const getById = async (req, res) => {
    try {
        const product = await productService.getById(req.params.id);

        return product;
    }
    catch (err) {
        res.status(500).json({ error: err.message });
    }
}
Enter fullscreen mode Exit fullscreen mode

So, what's the problem here? Imagine that the first line of our service (productModel.findById(id)) throws a database or network related error, in the previous code the error will be handled exactly the same as a "not found" error. This will make the handling of the error more complicated for our client.

Also, an even bigger problem: We don't want just any error to be returned to the client for security reasons (we may be exposing sensitive information).

How do we fix this?

The best way to handle this is to use different implementations of the Error class accordingly for each case. This can be achieved by building our own custom implementations or installing a library that already contains all the implementations of Error we need.

For REST APIs I like to use throw.js. It's a really simple module that contains Errors matching the most common HTTP status codes. Each error defined by this module also includes the status code as a property.

Let's see how the previous example will look like using throw.js:

/* --- product.service.js --- */
const error = require('throw.js');

const getById = async (id) => {
    const product = await productModel.findById(id);

    if (!product)
        throw new error.NotFound('Product not found');

    return product;
}

/* --- product.controller.js --- */
const error = require('throw.js');

const getById = async (req, res) => {
    try {
        const product = await productService.getById(req.params.id);

        return product;
    }
    catch (err) {
        if (err instanceof error.NotFound)
            res.status(err.statusCode).json({ error: err.message });
        else
            res.status(500).json({ error: 'Unexpected error' });
    }
}
Enter fullscreen mode Exit fullscreen mode

In this second approach we've achieved two things:

  • Our controller now has enough information to understand the error and act accordingly.
  • The REST API client will now also receive a status code that will also help them handle the error.

And we can even take this further by building a global error handler or middleware that handles all errors, so that we can clear that code from the controller. But that's a thing for another article.

Here's another module that implements the most common error types: node-common-errors.

Thoughts? 💬

Were this tips useful?

Would you like me to write about any other node.js related topics on the next article of the series?

What are your tips to write effective/clean node.js code?

I'll like to hear your feedback!

Top comments (43)

Collapse
 
avalander profile image
Avalander • Edited

Async/await allows us to build asynchronous non-blocking code with a cleaner and more readable syntax than promises

I wish people would stop saying that. async/await is a nice abstraction if you want to write asynchronous imperative code, but promises end up in nicer and more readable code if you use function composition and higher order functions.

To make a fair comparision, your example could be rewritten like this:

myFunction()
    .then(doStuff)
    .catch(handle)
Enter fullscreen mode Exit fullscreen mode

Which is not only shorter than the async/await example, but avoids an unnecessary intermediate variable* polluting the local scope.

* I'm not saying that locally scoped variables are a bad thing, but each variable in the scope is one more thing you need to keep in mind when reasoning about that piece of code, because it can be accessed and modified elsewhere in that scope. Long functions using async/await tend to result in lots of intermediate variables that is unclear at a first glance whether they are only passed as argument to the next function or are used further below.

Collapse
 
paulasantamaria profile image
Paula Santamaría • Edited

I actually did a little research before writing that sentence, because I thought "maybe I'm just more comfortable with async/await because of my C# background", and I wanted to be objective. Here's what I found with links to the sources (so you don't have to take my word for it):

First I went straight to the source, ECMA-262, 8th edition, were I found async functions to be defined as follows:

Async functions improve the asynchronous programming experience by providing syntax for promise-returning functions.

But I wanted some benchmarks and in-depth analysis, so I kept going. And that's when I found this article Faster async functions and promises, by the V8 team, that definitely convinced me. Here are the key points made in the article:

With async functions, the code becomes more succinct, and the control and data flow are a lot easier to follow, despite the fact that the execution is still asynchronous.

Async functions are intended to make asynchronous code look like synchronous code, hiding some of the complexity of the asynchronous processing from the developer.

async/await outperforms hand-written promise code now. The key takeaway here is that we significantly reduced the overhead of async functions — not just in V8, but across all JavaScript engines, by patching the spec.

So, after this research I felt comfortable enough to recommend async/await over promises. But still, I understand devs that are used to promises and new to async/await may not find this syntax more readable.

Collapse
 
avalander profile image
Avalander • Edited

We could discuss all day long the merits of the second article you linked. For instance, I find it interesting that they conclude that async/await outperforms hand-written code promise when the graph right above that sentence shows that async/await runs faster only in one of the two benchmarks, or how relevant it is for real applications that some arbitrary code runs about 5ms faster than some other arbitrary code.

However, I'm not interested in discussing performance because it's not the point you made in the article, it's not the point I was debating, and promises vs async/await is rarely where the performance bottleneck will be in any real world project.

Going back to the topic of readability, I think we should acknowledge that it is a somewhat subjective matter, and some decisions are going to be more a matter of personal style than one option being clearly superior to the other. I think async/await vs promises is one of those instances.

I've written a lot of asynchronous code in many different styles and contexts. Among others, I've written Javascript code with callbacks, promises and async/await; Python both with its async/await and before that was added to the language; and Java 1.7 with its blocking thread execution until asynchronous operations finish. And I don't see Javascript's async/await being a substantial improvement over promises in the same way that promises and async/await are an improvement over callbacks.

As I said, promises fit the declarative functional paradigm quite good. If your mental model is a pipe of transformations through which the data flows from a given input to the desired output, a promise chain matches that model quite well, and you'll find easier to reason about code written in that style.

On the other hand, async/await is better if you are writing imperative code. If your mental model is a series of steps that need to be performed, then it will be easier to reason about steps that need to be awaited than functions that will be executed at an arbitrary point in the future.

Thus, I think async/await is a horizontal evolution over promises that fills an entirely different niche, for which Javascript didn't have any good solution yet.

Thread Thread
 
paulasantamaria profile image
Paula Santamaría • Edited

I agree, we would discuss all day. If you won't even consider an article written by the people behind the javascript engine inside node.js, I don't think I can come up with a better/more reliable resource.

It's true that readability is subjective, and that's why I try to ignore my own biases and follow recommendations from the leaders of the industry that I also take into account when writing my articles.

The idea behind this article is merely to provide the readers with tips and tools that worked for me to write clean and efficient node.js code, and that are based on official documentation and reliable resources.

Like I said, it's perfectly ok if you still prefer promises.

Thread Thread
 
savagepixie profile image
SavagePixie • Edited

If you won't even consider an article written by the people behind the javascript engine inside node.js, I don't think I can come up with a better/more reliable resource.

You do realise that Avalander has considered said article, right? They are saying two things about it: 1) its conclusions are dubious and 2) it doesn't really address your point or their objection to it. If this doesn't show that they've considered it, I don't know what does.

The point Avalander is contesting is this claim:

Isn't it cleaner and easier to read with async/await?

To which I am afraid I must side with Avalander and say that neither you nor your sources have provided good support for it. async/await reads differently than promises. But that's just it. Not cleaner, nor better, just different.

And I am going to discuss the second source you mention. Particularly the sentence you quoted:

With async functions, the code becomes more succinct, and the control and data flow are a lot easier to follow, despite the fact that the execution is still asynchronous.

I can't help but notice that they write that just below an example where they've rewritten promise syntax to async/await using two fewer lines of code (one of which is merely a )};) and making two other lines twice as long. Specifically, while the promise syntax uses 168 characters, the async/await syntax uses 181. That's 13 extra characters for the supposedly more succint code of async/await. Even if it were 5 characters less, that'd be dubious evidence on which to ground the claim that async/await is more succinct, simply because it's only one example with too small a difference to draw any conclusions.

By the way, I'm not saying that promise syntax is better than async/await. All I'm saying is that I find the claim that async/await is better or clearer wanting.

Thread Thread
 
paulasantamaria profile image
Paula Santamaría

We concluded earlier than readability is subjective, so I don't think it makes any sense to keep arguing about what you find best.

My point is simply this: We can all have our opinions and preferences but, in the interest of objectiveness, I believe the best we can do is follow reliable sources. Unless you have the time to explore the matter in depth and write your own article with your own conclusions.

Both links I've shared on this thread I consider to be reliable and present async/await as an improvement to writing asynchronous code in javascript. And there are many other sources that explore this statement in depth and conclude the same thing. This article was not meant to list the pros and cons of async/await but merely to share tools that worked for me and that are supported by documentation written by people far more experienced than me in the subject.

Here's another article from this very platform that explores the subject more in depth.

Collapse
 
misterwhat profile image
Jonas Winzen

I agree. I'm experiencing async/await as an invite to handle side effects and pure functionality intermixed in one big spaghetti function.

Collapse
 
hugo__df profile image
Hugo Di Francesco

Node 10+ has a promisified version of fs under fs.promises.

const fs = require ('fs').promises
Collapse
 
paulasantamaria profile image
Paula Santamaría

That's awesome, thanks! I'll probably include this in the next article.

Collapse
 
jsardev profile image
Jakub Sarnowski
const readFilePromise = util.promisify(fs.readFile);

const readFile = async (path) => {
    return await readFilePromise(path);
}

Here, the readFilePromise is exactly the same as readFile. You could just do: const readFile = util.promisify(fs.readFile);

Collapse
 
paulasantamaria profile image
Paula Santamaría

Thanks for the suggestion! I was actually aware of that, but I decided to wrap readFilePromise in a function anyway to illustrate how, after using promisify, we can await the "promisified" function within an async function. I couldn't come up with a better example, but I'll try to think of one to avoid any confusion.

Collapse
 
jsardev profile image
Jakub Sarnowski

I was thinking that this might be the case, but I've decided to comment it out, just in case 😄 Great article!

Collapse
 
paulasantamaria profile image
Paula Santamaría

It's 5am, I couldn't sleep and I came up with a better example that made more sense 😂. I just replaced the old one.

Collapse
 
buinauskas profile image
Evaldas Buinauskas

The overall idea to throw exceptions for control flow isn't that great. You could achieve the same with proper method return type which would let higher layers know that a product might not exist. 🙂

Collapse
 
paulasantamaria profile image
Paula Santamaría

I agree. I try to follow the rule of "using exceptions for exceptional circumstances". However, a situation that may be consider exceptional in some use-case, may be regular in another.

My example wasn't really written with any particular use-case in mind. It's just an example of how we can give more information to other layers about what went wrong. For the sake of the example, we could define that calling the getById method with a non-existent id is an exceptional circumstance.

Collapse
 
buinauskas profile image
Evaldas Buinauskas

Yep. The overall idea to throw a more specific exception is good, perhaps the use case isn't as it might confuse more junior developers.

I tend to think that exceptional is something that can happen without end user's interference, e.g. you can expect user to supply an incorrect parameter to an API :)

Thread Thread
 
paulasantamaria profile image
Paula Santamaría • Edited

And that's why use cases matter. I won't expect a user to supply an incorrect id. In most use-cases they won't be writing the id themselves manually, that should be solved by the front-end. And that's why, in this particular case, I find an exception appropriate.

Collapse
 
paulasantamaria profile image
Paula Santamaría

I've been meaning to learn more about good practices related to error handling, but I can't seem to find any in-depth book or resources. If you could recommend a book or any trustable resource, I'll appreciate it :)

Collapse
 
buinauskas profile image
Evaldas Buinauskas

I can't recommend a specific book but I love functional programming as it helps to solve these problems by using appropriate type definitions. e.g. Maybe<Product> which indicates that there might be a product in this wrapper, there might be none, or Result<HttpError, Product> which represents response of a HTTP call to retrieve a product.

Check github.com/rametta/pratica out. A very small lib that has it all :)

Thread Thread
 
paulasantamaria profile image
Paula Santamaría

Thanks! I really want to get more into functional programming, I'll definitely check that out.

Collapse
 
codenutt profile image
Jared

wow, i didn't even know about the different instances of Error. Thank you for sharing!

Collapse
 
paulasantamaria profile image
Paula Santamaría

Thank you, Jared :)
I'm a huge fan of error handling! I'll probably share more stuff about that in this series.

Collapse
 
adisreyaj profile image
Adithya Sreyaj

Really looking forward to this...as I'm pretty bad at handling errors...🙃

Thread Thread
 
paulasantamaria profile image
Paula Santamaría • Edited

In case you haven't had a chance to see it yet, I wrote more about error handling in the second part of this series: Refactoring Node.js (part 2).

Collapse
 
jacqueline profile image
Jacqueline Binya

Great article as always Paula, im a huge fan. From this one I learnt a lot of stuff im hoping to implement in my code, looking forward to the next article in the series.

Collapse
 
paulasantamaria profile image
Paula Santamaría

Thank you so much 🙏 I'll try to keep it interesting!

Collapse
 
mrm8488 profile image
Manuel Romero

Great post, Paula (as usually)
Just a couple of things I would like to highlight:

1) When working with callbacks first argument must be the error(AKA Error first pattern):

//If error
return callbak(error);
//Else
return callback(null, data);

2) If you are using an express like framework you can manage 404 errors in another function just calling next(); or 500 errors with next(error); so that you don't have to replicate the error logic in each controller.

Collapse
 
evanplaice profile image
Evan Plaice

Awesome article 👍

Can you write more content on async + promisify? I feel like these are both super valuable topics that are also not talked about nearly enough.

Collapse
 
paulasantamaria profile image
Paula Santamaría

Thank you Evan! I've only use promisify with the fs module, but I'll try to find more examples so we can understand it in depth together.

Collapse
 
evanplaice profile image
Evan Plaice

Of course. I'm always excited to see someone dig deep into some more advanced topics.

Maybe this'll help

Here's promisify(exec):
github.com/vanillawc/fpm/blob/mast...

And w/ some other file operations:
github.com/vanillawc/fpm/blob/mast...

Thread Thread
 
paulasantamaria profile image
Paula Santamaría

Awesome, thanks!

Collapse
 
petrosvoivontas profile image
petrosvoivontas

I think there's a mistake in this piece of code:

const firstOperation = myAsyncFunction();
const secondOperation = myAsyncFunction2('test');
const thirdOperation = myAsyncFunction3(5);

await Promise.all([ firstOperation, secondOperation, thirdOperation ]);

The variables firstOperation, secondOperation, and thirdOperation include the results of each function NOT the functions themselves. So the array you pass to Promise.all does not have functions as its elements, but the results of those functions.

Collapse
 
paulasantamaria profile image
Paula Santamaría

Hi there! In that example, all functions (myAsyncFunction, myAsyncFunction2 and myAsyncFunction3), are async, which means that they return promises. So what const firtsOperation is storing is the promise returned by myAsyncFunction.

Finally, by awaiting Promise.all we're waiting for all promises to resolve.

If I wanted firstOperation to contain myAsyncFunction final result I should have wrote const firstOperation = await myAsyncFunction();
But the idea of that example was to illustrate how we can execute many async functions in parallel.

More info: MDN - Promise.all()

Collapse
 
orekav profile image
Orekav

There is a very small mistake in the FS code with Promisify

You are repeating "const readFile" twice.

Collapse
 
paulasantamaria profile image
Paula Santamaría

Nice catch, thanks! It's fixed.