Good code has good error handling. Error handling involves the use of try
/catch
. Therefore, anytime you suspect an error might be thrown, you should catch
it. And once you've caught an error, you... um... well, I guess you log it out?
My goal is to convince you to be ok with letting errors go uncaught, regardless of what kind of environment you are coding in. We'll discuss a small handful of environments, but the general idea extends to any environment.
Browsers
What is the problem with the error handling in this code?
document.querySelector('.refresh-button')
.addEventListener(async () => {
try {
redrawRecords(await fetchRecords());
} catch (error) {
console.error('Failed to fetch records:', error);
}
});
Lets start by asking - do you know what the browser will do if you let an error go uncaught? It'll log it out. So why are we bothering to catch the error and log it when that's exactly what the browser was going to do anyways? You can toss the try
/catch
, it's just extra noise.
You might argue that the console.error()
is adding some valuable context - it's informing you that the error happened while we were trying to fetch records. But that too is redundant, you already have easy access to that same context by looking at the stack trace. If an error really did happen while attempting to fetch records, you will see the function name fetchRecords()
in the stack trace, which gives you the exact same context.
In this specific scenario, an even better solution would be to notify the user when an error happens, but when that's done, don't feel ashamed to rethrow that error and let the browser handle logging it out for you.
document.querySelector('.refresh-button')
.addEventListener(async () => {
try {
redrawRecords(await fetchRecords());
} catch (error) {
informUserOfError(error);
throw error;
}
});
Node Servers
What's wrong with the error handling in this Node server code?
sendEmailToUser(emailOpts)
.catch(console.error);
To give some context, when an error goes uncaught in Node, the entire process will crash. But remember, this was an intentional design decision.
The idea is that once an unknown error happens, your server will be left in an unknown state, and the best thing you can do is to let it crash, then automatically spin up a new server in a clean state. This philosophy, however, only works if all of these conditions apply to you:
- Your code is crash-ready. You never have a scenario where you're blindly doing
await writeThing1(); await writeThing2()
, because the server might crash between those two async steps and leave your saved data in an invalid state. In the ideal world, you would be writing robust code anyways that accounts for the fact that a crash can happen at any point in time anyways, but the reality is that you, like many people, might not be writing code to that kind of standard. - Any dependencies you might use should also be crash ready in the same fashion.
- You should have auto-restarting set up, so when your server does crash, it'll automatically bring itself back up. Ideally, you would have a cluster of instances running at the same time, so one of them crashing wouldn't cause any real downtime.
- Say someone find a particular set of inputs they can give to one of your endpoints, to cause an uncaught error to happen and your server to crash. Is there anything stopping this person from sending a request, every few seconds, to this endpoint, and causing your servers to repeatedly crash, preventing anyone else from using your services? You will need to have protection in place against these kinds of exploits.
If you agree with Node's default error-handling philosophy and are writing the kind of enterprise code that can support it, then stop trying to fight the philosophy with your code by trying to catch and log every error imaginable. When something unexpected happens, let your process crash.
If you're not running a server that can support this kind of unexpected crashing, then Node's default behavior will actually do more harm than good for you. There's a simple solution. Just throw the below code snippet somewhere in your codebase - it'll catch all uncaught errors and log them out, effectively making the default uncaught error behavior match that of a browser.
process.on('uncaughtException', error => {
console.error(error);
});
If you are in the habit of catching and logging every error in Node to prevent Node from crashing, then you are effectively hand-implementing the behavior of the above code snippet, in a tedious, error-prone fashion.
Libraries
If you're a library author, you can't necessarily globally configure Node to log uncaught errors instead of crashing - it's not your place to make that kind of decision. But that's ok. When things go wrong in the internals of your library, it's your responsibility to make sure those errors become uncaught errors, and it's the consumer of your library's responsibility to decide what to do when unexpected issues happen (whether that's crashing Node, logging the issues to stderr, or something else entirely). Don't try and take that choice away from the consumers. Let your unexpected errors go uncaught.
Concluding thoughts
As always, take everything being said with a grain of salt. While, in general, it's ok to let errors go uncaught, there are specific scenarios where preventing errors from bubbling up is a good thing. In some cases, catching errors just to log them is the best option available. I won't go over the nuances on when these scenarios may or may not be appropriate, instead, I just ask that you use your judgement, and to not be afraid to consider uncaught errors as a valid option as you weigh your options.
Top comments (0)