I will point out 4 basic but common issues I faced over years of working on Express, and how simply following some best practices can produce "bang for the buck" results. I use my own library to solve the problems, but you can prefer your way to address the concepts.
1. Dealing with numbers - Check for EXACTLY what you are expecting
For example:
router.get('/user/:id', (req, res) => {
let id = parseInt(req.params.id)
// some code to retrieve a user.
})
The problem here is with #parseInt used wrongly, or interchangeably with #Number. Using #parseInt often accepts inaccuracy and causes problems.
Not ok to accept inaccuracy
The code may look fine at first, because id is almost universally an integer. But #parseInt can inaccurately transform wrong information into something usable.
To illustrate, say someone intends to access the route user/1/photos but accidentally left out a slash:
GET /user/1photos
Familiar error isn't it? This results in #parseInt falsely evaluating the inputs to something usable but misleading:
parseInt('1photos') // gives 1
The code is misled to continue downstream, thinking that you want user #1's information, providing a response with status 200, tripping developers to wonder why the response did not have any photo data.
Using #Number, and being accurate
The right way is to use #Number, which will produce NaN:
router.get('/user/:id', (req, res) => {
let id = Number(req.params.id)
if (isNaN(id)) return res.status(400).send('Error')
if (!Number.isInteger(id)) return res.status(400).send('Error')
// some code to retrieve a user.
})
Now, you will see that we corrected code to use #Number, and we also have some checking going on ensure it's an integer. But what trips programmers here is that they often forget to complete a full check, or compromise doing so.
Discipline in this aspect is often difficult to instil. And since most of the time the yardstick is that the code works and passes tests, it seemed OCD to be quibbling over cases of others’ carelessness. Or perhaps also it’s not worth the effort to write the code to help other people realise their mistake faster.
Either way, I was left with no choice but to write my own library. Not does away with all these problems.
import Not from 'you-are-not'
const not = Not.create({ willThrowError: false})
router.get('/user/:id', (req, res) => {
let error = not('integer', req.params.id)
if (error) return res.status(400).send(error)
// some code to retrieve a user.
})
2. Provide actionable error messages
Going back to the misfortunate example of missing slashes:
GET /user/1photos
Since we are already returning a status 400 to say that the inputs are wrong, we should also say what's wrong.
This is not easily done because you need to manually write some messages, or developers -- who are almost always -- on tight schedule will be skimpy on error handling/messaging. Again, no amount of discipline will solve this problem. The solution is just to have a library that comprehensively handles type checking/validation, error handling, and error messaging.
Referring to the previous example, we expand the code to use some features of Not to help us out:
// GET /user/1photos
import { format } from 'utils'
router.get('/user/:id', (req, res) => {
let error = not(
'integer',
Number(req.params.id),
'id',
`received ${format(req.params.id)}`
)
if (error) return res.status(400).send(error)
// some code to retrieve a user.
})
Not produces a string, this string assigned to error holds the message:
Wrong Type: Expect type `custom:integer` but got `nan`. Note: received "1photos".
Now that clearly tells the API user that there are missing slashes for "1/photos" because the payload is "1photos". This is what I mean by having a very actionable error message to help out the API requestor.
3. Add a timestamp to error messages
This is perhaps the single most "bang for the buck" thing to do that can quickly squash all your bugs. To illustrate: User gets an error, provides the timestamp, you search your logs to locate the error, and the problem gets fixed. Simple, fast, and productive.
The Not way of doing this is automatically taken cared of. Suppose you need to add an array:
let not = Not.create({
willThrowError: false,
timestamp: true // switch on timestamp
})
router.put('/array', (req, res) => {
let error = not(
'array',
req.body.array)
)
if (error) {
console.error(error)
return res.status(400).send(error)
}
// some code to add the array.
})
So suppose someone erroneously post a string with commas (a very very common error):
let payload = "item 1, item 2"
post('/array', payload)
This will produce an error message to the user, as well as in your logs:
Wrong Type: Expect type `array` but got `string`: "item 1, item 2". (TS: XXXXXX)
You can then search your logs and quickly identify the problem (if the user somehow still cannot figure it out with the error message provided).
4. Always sanitise the payload
I cannot emphasise enough the importance of sanitising request payloads. This means regardless of what was sent to your API, your API should always filter out all other information that it is not intended to receive.
Accessing hidden information
I once had a client who wanted to keep deregistered user data in the database. Never mind data security regulations and all, the method was to use a field hidden, which is set to true to "delete" the records.
To illustrate the vulnerability, the simplified request payload and route syntax is like this:
User sends in a request like this:
{
gender: 'male',
location: 'Singapore'
}
User information lookup is like this:
router.get('/users', (req, res) => {
let options = {
hidden: false
// and a lot more options
}
Object.assign(options, req.body)
DB.find(options).then(results => { res.send(results) })
})
Again things look fine, except not sanitising payloads can easily open up a security vulnerability for the requestor to get "deleted" users:
{
gender: 'male',
location: 'Singapore',
hidden: true
}
#Object.assign will overwrite the default option of hidden: false to hidden: true, allowing requestor to gain unauthorised access to "deleted" users. You may point out a quick fix, which is to flip the object assignment:
Object.assign(req.body, options)
But that's not the point. One way or another, security vulnerabilities are bound to surface if no sanitisation is done. And it is a pain with custom code. So Not does it like this, on top of all the error messaging functionality it provides:
router.get('/users', (req, res) => {
// define a schema
let schema = {
"gender?": 'string', // you can define your own validation, but we keep it simple as string
"location?": 'string' // "?" denotes optional
}
// payload will only contain what is in the schema
// so `hidden: true` will be removed
let payload = Not.checkObject(
'request',
schema,
req.body, //payload
{ returnPayload: true }
)
// at this point, payload will become an array if there are errors
// you may simply send the actionable errors back to the requestor
if (Array.isArray(payload)) return res.status(400).send(payload)
// otherwise it is an object, ready to be used:
let options = {
hidden: false
// and a lot more options
}
Object.assign(payload, options)
DB.find(options).then(results => { res.send(results) })
})
And there you have it, type checking, validation and sanitisation all nicely settled.
Conclusion
So building an API is really not that simple. Not only that we need it to work, but also to let everyone figure out what's wrong when something breaks. Moreover, we are most likely to create errors more than be successful, so error messaging is really important to quicken up the development process. A small library that can allow neat validation/checking code will also aid that process of (1) vastly improve code maintainability, and (2) reducing the barrier to do error handling/messaging.
I hope my thoughts can contribute to better codes. Let me know what your comments are!
About Not.JS
If you like what you see, do drop me a star here.
Top comments (0)