Hi!
You might have been linked this blog post in a PR that has a try...catch
statement.
There might be a request for changes linking to this blog post, asking that you avoid a try...catch
, or failing that, reduce its scope.
This blog post is an explainer for why to avoid try...catch
statements when possible, along with examples of alternate strategies you can take.
Avoid try...catch
if possible
I advise using try...catch
statements as little as possible.
They should only ever be used when in exception cases: ones that aren’t expected to happen normally, or when an API outside of your control might throw.
If you know some situation will happen in code, your code should handle it as directly as possible. If something is crashing, figure out the root cause of why it’s crashing, and handle that root cause.
👉 See Checking Before Wrecking below for an example.
Reducing try...catch
scope as much as possible
If you absolutely must use a try...catch
, try to put as little as possible inside it.
try...catch
statements are a very blunt tool.
They catch any exception, including ones that you might not expect.
Instead of wrapping large swathes of code in a try...catch
, wrap only the specific piece of code that is expected to throw.
👉 See Trying Less below for an example.
Examples
These are some contrived examples showcasing the two strategies in order. I separated them out because I know a lot of people wont’t have the time to read them in detail.
Checking Before Wrecking
Suppose you have a emphasize
JavaScript function that’s supposed to turn a string into an uppercase version with an exclamation mark:
// emphasize.js
export function emphasize(text) {
return `${text.toUpperCase()}!`;
}
It might be used in an application to log the value of a --message
argument.
Running the following file like node index.js --message "Hello world"
should print HELLO WORLD!
:
// index.js
import { parseArgs } from "node:util";
import { emphasize } from "./emphasize.js";
const { values } = parseArgs({
options: {
message: { type: "string" },
},
});
console.log(emphasize(values.message));
Let’s say you’re seeing errors from emphasize
complaining that text.toUpperCase()
is not a function:
Uncaught TypeError: text.toUpperCase is not a function
at emphasize (emphasize.js:2:20)
at index.js:6:13
A bit of debugging might show that the message
argument being passed to emphasize
is actually undefined
.
One fix could be to wrap the contents of emphasize
in a try...catch
, so errors attempting to call .toUpperCase()
on the undefined
value are caught and reported:
// 🛑 See later - there's a better way!
export function emphasize(text) {
try {
return `${text.toUpperCase()}!`;
} catch (error) {
console.warn("Oh no:", error);
return text;
}
}
But, that’s a “band-aid” fix: it doesn’t address the root issue of emphasize
being passed the wrong type of data.
Band-aid fixes are almost never the right strategy.
Most of the time, it’s better to address root problems so they don’t present issues elsewhere in code.
Here, the “unhappy” case was when users don’t provide a message
argument.
They might be running node index.js
without --message
.
When that happens, message
’s value is undefined
rather than a string
.
A better fix might be to print a better error if there’s no message
provided:
import { parseArgs } from "node:util";
import { emphasize } from "./emphasize.js";
const { values } = parseArgs({
options: {
message: { type: "string" },
},
});
if (values.message) {
console.log(emphasize(values.message));
} else {
console.error("No --message provided.");
process.exitCode = 1;
}
By refactoring to handle the “unhappy” case separately, the code didn’t have to worry about invalid data being passed to emphasize
.
💡 In TypeScript terms, we made sure the argument passed to
emphasize
matched the expected type of the parameter:string
, notstring | undefined
.
Trying Less
Let’s say you have a fileIncludes
function that determines whether some file includes a string.
It reads the file from disk as text, then checks whether that text includes the search string:
import fs from "node:fs/promises";
async function fileIncludes(filePath: string, search: string) {
const data = await fs.readFile(filePath);
const text = data.toString();
return text.includes(search);
}
fileIncludes
will throw if no file exists under filePath
.
So, it might be tempting to wrap the function’s body in a try...catch
:
import fs from "node:fs/promises";
async function fileIncludes(filePath: string, search: string) {
// 🛑 See later - there's a better way!
try {
const data = await fs.readFile(filePath);
const text = data.toString();
return text.includes(search);
} catch (error) {
console.warn(`Failed to read ${filePath}:`, error);
return false;
}
}
There’s no need to wrap all of the code inside fileIncludes
in the try...catch
.
Only the call to fs.readFile
is expected to throw.
Even if another line of code were to throw, it wouldn’t likely be for the logged reason “Failed to read …”.
A more targeted strategy would be to only wrap the one line in a try...catch
:
import fs from "node:fs/promises";
async function fileIncludes(filePath: string, search: string) {
let data;
try {
data = await fs.readFile(filePath);
} catch (error) {
console.warn(`Failed to read ${filePath}:`, error);
return false;
}
const text = data.toString();
return text.includes(search);
}
This way, only the one line that might throw an error is handled by the try...catch
.
Developers reading the fileIncludes
function can more easily understand what specifically might fail.
Any other unexpected exception that occurs would fall back to however the project handles unexpected exceptions.
Further Reading
If you want to read more about why try...catch
is something to avoid when possible, I recommend:
- Are exceptions as control flow considered a serious antipattern? If so, Why?
- Why TypeScript Doesn’t Include a throws Keyword
Thank you for sending a pull request in, and thanks for iterating on it! 💙