PPL 2020 — Document Builder: Refactoring Document Builder to Follow Node.js Best Practices

Pande Ketut Cahya Nugraha
pepeel
Published in
7 min readApr 14, 2020

Our Document Builder’s app uses numerous technologies. One of them is Node.js, which we use to build our backend. Our backend is built in the form of a REST API which interacts with the database. While I am familiar with building backend services, I never built it with Node.js, which with its many quirks caught me off-guard and forces me to do a lot of refactoring. In this article, I will explain many quirks of Node.js on our app and how to deal with them.

What is Node.js?

One of the most common misconceptions about Node.js is that it is a programming language. It’s not. It is a runtime environment for JavaScript, which mean it is designed to run JavaScript codes outside of its typical environment, the Web Browser. Thus, using Node.js, we can write JavaScript codes to be run in a server.

One of the main difference between Node.js and other languages typically used for backends such as Java and Python is, that it is by default is asynchronous. Node.js uses an event-driven architecture capable of asynchronous I/O, which mean that it can manage several IO requests at the same time. But what does that mean when you code in Node.js? Let’s have a look below.

Lesson #1: Always, always use async/await

As said above, Node.js is by default asynchronous. Thus, if you call another function, there is a fair chance that it is asynchronous too. On Node.js community nowadays, one of the main points that everyone emphasis is always, always use async/await. Using async/await will always save you headaches, especially for a beginner to Node.js that never code asynchronously before.

This beginner, however, ignorantly ignore this advice, and thus, write a piece of code will never work properly. See below for what I meant.

What is an ordinary code in another language, soon to be revealed as a big mistake in Node.js

To those who are unfamiliar with Node.js, this code might seems fine. But it is far from fine. Due to the asynchronous nature of Node.js, most of the module in Node.js is asynchronous as well. Sequelize, the ORM we use, is no exception. Thus, when we call Template.findOne, Node.js would not wait for the database to successfully fetch the data. It would continue executing code beneath that call. Because of that, more often than not, my code frequently returns undefined, because when that return template is executed, template is still undefined due to Template.findOne not completed yet.

I committed this mistake and went on doing other tasks. After a while, I am baffled because when I tried to retrieve a template, it usually returns undefined. After a bit of Google Search, I discovered that this piece of code needs async/await due to the fact that Sequelize is asynchronous. Thankfully I haven’t pushed this to GitLab yet, so I’m… uh… rebase the commit to fix it :)

Mistake fixed!

Lesson #2: Be careful of Async/Await in loops.

Merely knowing that async/await is needed is not enough. One also must know the quirk in async/await that isn’t obvious to the untrained eye. See the piece of code below.

The code for creating a Document and its Entries. There is a subtle bug here.

The above code describes the process of creating a document and its entries in our Document Builder’s app. After reading it, everything thing seems fine with me. The process is correct, it uses async/await properly, nothing seems wrong. The code should’ve created the entries, await that process, then create the document, and after that is done, await setting the entries so it belongs to the created document, and after all of that is done, the code returns the document with all of its entries information.

But why then the Document foreign key on the Document Entry, which denotes which Document the entry belongs to, be NULL?

Turns our, async/await behave a bit differently in a loop. One of the posts in Node.js community says this.

Oh god.

So essentially this means that we do await for createDocumentEntries to be done, but inside the createDocumentEntries itself, Node.js did not wait until Document Entry is created. If we have a forEach or map, using await inside it doesn’t do anything. And so, some of the entry will not have its Document set, leading to the undefined result in the generated PDF.

Thankfully, this can be easily fixed. Both me and Irene fixed this quickly, and life is good again, where there is no Document PDF with an undefined value.

A subtle bug is the worst. Especially if it is caused by our own lack of knowledge.

Lesson #3: Once you use with async/await, stick with it.

Async/await is not an old thing. Before async/await exist, Node.js programmer has to deal with asynchronous programming the bad way, using the callback. And no, I am not exaggerating, they literally called it “the callback hell”. Why? Because the more callback you need, the more your code gets moved to the right, resulting in a stacked mess like the one you see below. (This one isn’t that bad, actually, only 3-stacked callback.)

To fix this horrendous code, Node.js first introduced Promise. It is nicer, but still have this bad nested call that keeps going to the right. Realizing this, Node.js then introduce async/await, which is far nicer (still confusing for a beginner to Node.js, but after a month using it, you’ll be thankful that async/await exist and callback doesn’t exist (mostly) anymore).

Same code as above, but using async/await. Far nicer!

Notice that I inserted “mostly” there. Turns out, there is still some programmer that uses callback. And one of them is the programmer that makes the module used by Document Builder to convert DOCX to PDF.

Thankfully only one callback.

So, all is fine right? Me, as a beginner Node.js programmer who just recently understand async/await and never actually code using callback just went and think “This is fine. The callback is just an older version of async/await, surely this code is just going to work fine.”

Well, it does. Until it doesn’t.

While my code does its job perfectly fine in converting DOCX to PDF, my mistake is thinking that this is working just like async/await, where it will wait until the conversion process is finished, then continue to the rest of the code. Turns out that it will not wait for the conversion process to be done. As creating and editing Document are hooked up to this function at the end of it, this means that the server responds with “OK, the creating/editing process is done, all is well” while in reality the server is not yet done converting the document. The result? When the user tries to access their document shortly after creating/editing, the server responds with “Nah, not found, there is no file for that document”.

Sure, if the user waits for 1 or 2 more minutes, the document will be available. But it is still not good code, because it tells the user that the document is ready while in reality, it is not ready. So, I went to find a way to make this convert function use async/await, and turns out that Node.js already provide us with a way to convert callback function to async/await function. If only I was not that lazy :/

Just util.promisfy all your callback function. It will save you a lot of headaches.

And so, ends the tale of refactoring Node.js codes. Asynchronous problem is a hard thing to detect. All three of the refactoring I did above only makes itself apparent when tested frequently or on a slow machine, because the asynchronous problem only became apparent when the computer did not deal with the problematic code fast enough. For example, Lesson #3 did not appear until the code is in staging, because in my local environment, the converting process is done in less than 5 seconds. In staging, however, the converting process can take more than 1 minute, thus making the problem itself known. So, always be careful with asynchronous codes.

Also, always consult the community on StackOverflow and other reading sites. Before coding, knows the best convention that exists in the community so you are not trapped in the same beginner mistakes that are easily avoided as I did as shown above.

Thank you for reading, and don’t hesitate to give feedback on the comment section!

--

--