title | slug | createdAt | language | preview |
---|---|---|---|---|
On useless try-catches, being overly defensive, I/O boundaries and variable scope |
on-useless-try-catches-being-overly-defensive |
2018-11-03T10:43:09Z |
en |
Many things can go wrong when try-catch is used, especially in an asynchronous context. As a structure try-catch is quite powerful and should be used sparingly, only when it's really needed. |
Many things can go wrong when try-catch
is used, especially in an asynchronous context. As a structure try-catch
is quite powerful and should be used sparingly, only when it's really needed.
Starting point:
async function getUsers() {
try {
return await db.select('SELECT * FROM app_users')
} catch(err) {
throw err; /* 1. */
}
}
async function main() {
try {
const users = await getUsers()
console.log(`Ya! We have ${users.length} users!`) /* 2. */
} catch(err) {
console.error('Something went wrong..')
}
}
End result:
function getUsers() {
return db.select('SELECT * FROM app_users')
}
async function main() {
let users
try {
users = await getUsers()
} catch(err) {
console.error('Something went wrong..')
return
}
console.log(`Ya! We have ${users.length} users!`) /* 2. */
}
Why would you increase indirection and the amount of code like this?
1. If the catch block only rethrows the error, the whole try-catch structure is useless
- async function getUsers() {
- try {
- return await db.select('SELECT * FROM app_users')
- } catch(err) {
- throw err; /* 1. */
- }
+ function getUsers() {
+ return db.select('SELECT * FROM app_users')
}
Ok, this is one of the classics. It might be that you used to have some special magic inside the catch
block, but you removed it and forgot to clean after yourself. Our goal should be to push exception handling as far up the call stack (as early in execution) as possible.
The theory behind this might be that exception handling is an indicator of side-effects. Because side-effects should be done only at the I/O boundaries of the program, these things would also wrap together quite nicely. Need to clarify my own thinking about this.
1.1 Never await as part of return expression
With JS promises:
return await db.select('SELECT * FROM app_users')
is the same as:
return db.select('SELECT * FROM app_users')
so I guess we're mostly talking about a syntactical error. This discussion could me expanded to other similar wrapper values, especially lazy ones and how pulling out the value without reason gives less control to the calling function. Now you can get rid of the async
keyword as well.
2. The only things allowed in try {}
block are things that can throw
async function main() {
+ let users
try {
- const users = getUsers()
- console.log(`Ya! We have ${users.length} users!`) /* 2. */
+ users = getUsers()
} catch(err) {
console.error('Something went wrong..')
+ return
}
+ console.log(`Ya! We have ${users.length} users!`)
}
Don't put anything else there. console.log
can't throw so it needs to be outside. The reason for this is that the reader of your code cannot know which line of code can actually cause an exception. And yeah, of course they can go into the function definition and have a look, but we don't want to force the reader to do that. Quite the opposite actually: our aim is to write such code that the reader could understand it only by looking at the directory structure.
Of course by doing this we have to declare the variable outside try {}
s scope, which is admittedly ugly and I do not like it either. It's a small cosmetic sacrifice we do for better readability.