This is a story of how wanting to catch a small error in JavaScript lead to destroying the user experience of a signup process. How the duck did that happen? Here's a small recap, a little primer on how to catch errors in JavaScript, and a cautionary tale about how choosing the StackOverflow answer with the most upvotes might not always get you what you wish for.
Catching Errors with JavaScript
Before I can tell you the entire story, first a little primer on JavaScript error Catching. It's not the hardest thing you can do in JavaScript. In fact, it is pretty simple:
try {
// ...
// Do whatever. Here we use fancy "await" syntax to post
// something to the server.
await postData(url, processedData)
} catch (err) {
console.error("Woah something went wrong.", exc)
}
Great! Whatever goes wrong, we log that to the console (and in production also to Sentry). Now we could, or rather should, also communicate this to the user:
try {
// ...
await postData(url, processedData)
} catch (err) {
console.error("Uh oh.", err)
// https://www.youtube.com/watch?v=6iCPIUGnHQ8
showErrorModal(
"Oh oh. Something went wrong. Don't panic. " +
"Our technicians have been informed and will solve this shortly. " +
"Meanwhile, please contact our support at…")
}
Catching a specific error type in JavaScript
In our case, our postData
function raises specific exceptions, depending on what happened:
SubmissionError
when (server side) form validation failed (if you are using redux-forms, this will sound familiar)HttpError
when something else happened. Request took to long, server did not respond or maybe even the server failed.
Now, depending on that, you want to act very differently. The SubmissionError
should show up right next to the regarding field, while the HttpError
should show the modal we established earlier.
How do you do that? (kudos for this to David Walsh)
try {
await postData(url, processedData)
} catch (err) {
if (err.constructor === SubmissionError) {
showFieldValidationError(err)
} else if (err.constructor === HttpError) {
showErrorModal("Oh oh. Something went wrong. Don't panic…")
} else {
console.error("Ok what is going on?", exc)
}
}
You've stuck with me this far, so it's time to reward you with our cautionary tale: on the internet, you will find code that uses the .name
property instead of .constructor
. I am especially thinking about a rather popular answer on StackOverflow. I do consider it a valid way of doing this, but there is one situation where it fails miserably: minification.
Since you are a good web citizen, you are probably minifying your JavaScript, right?
When minification lets you question reality
Here's what happened to us: We just introduced the above exception handling, however we used the .name
property.
try {
await postData(url, processedData)
} catch (err) {
// Reminder: Don't do this. This is the "bad" example.
if (err.name === "SubmissionError") {
// …
}
}
Everything worked splendidly. We were able to provide better user feedback in the UI, we had cleaner code and a standardized way of doing things™ (we love that a lot here at Codista). Also, it worked. At least locally. During development, we never thought it might not work. So, we shipped it.
And that my friends, is where things went awry.
See, the thing is… In production we run with a minified build. So, some nice program (in our case webpack), makes your JavaScript smaller by cutting away unnecessary characters. This also means that it renames certain things. And renaming it did. Our SubmissionError
ended up being renamed to t
. Yup. Just t
.
If you'd let me illustrate this for a second:
try {
await postData(url, processedData)
} catch (err) {
if ("t" === "SubmissionError") {
// …
}
}
See our dilemma? t
can never be equal to SubmissionError
. That's why you should always check for exception types using David Walsh's method with the .constructor
property.
Lessons learned?
I'm glad to say we rarely miss glaring issues like these. Thankfully, there are people in this world who do contact support. Without them, we would have probably not noticed this within 2 hours and fixed it in one.
In any case, a "post mortem" is important. So here's our's:
- End to end tests: when projects reach a certain level of maturity, we have to check very critical paths via end to end tests that run in an environment that mirrors production.
- Frontend tests: could we have caught this via a frontend test? I don't think so. At least our's only ever run against a development build, so no minification. Maybe it would make sense to run them again against a production build.
- Improved QA process: as previously teased, this happened in a signup flow. Signup is usually one of the most important aspects of anything you put on the internet these days. When we change something there, we should always take the time to take it for a spin manually in the staging environment… before it is shipped onto production.
- Improved QA process #2: Again, with critical flows like these in a project of a certain maturity, we basically owe it to our own engineering pride to keep a testing checklist at hand that should also include these critical flows.
- Also: We come from the internet and yet we constantly have to be reminded to not trust everything on it. Even a high scoring answer on StackOverflow can fail you.
And now, let's get back to work.