Let it crash or not?

In some situations I’m not sure whether it’s better to let it crash or not. I can identify three main types of errors:

  1. Programmer error => let it crash
  2. User error (invalid form input) => handle the error through changeset validators and display an error message for user to correct
  3. Programmer error OR abnormal/malicious user input => ?

For the latter I’m not sure how to handle these errors. Say a form that contains a <select> element, allowing you to specify a relation; e.g. a user fills a doctor’s appointment details, and he has to select one of the possible doctor’s offices. These office IDs can’t be changed by a normal user as the <select> offers a limited set of options, but some malicious user could go into Developer Tools and change the <option> value, which will result into a DB foreign key contraint error.

For these cases, where an error might be the programmer’s fault, but also might be the user causing it (we should remember this is external untrusted data), what is best to be done?

Should I systematically use Ecto foreign_key_constraint/3 to avoid crashing the process? If so, what would you display to the user?
The disadvantage with this is that if it actually was a bug, I will miss that bug (no error log).

Should I just let it crash? Crashing has some overhead though, e.g. we want to log it. Now imagine a malicious user sending thousands of requests quickly triggering these errors. As this error is related to external data, is it really wise to let it crash? Most oftenly will be a programmer error, but what if it’s not.

It depends, but in general malicious user is still user, so this gives you to which bucket such errors should go.

For your specific example, I would the following:

Render only the possible valid options in the form and…

If the invalid state is most likely to happen (for whatever reason), I’d check if the user can in fact do this operation in the context and avoid the constraint error altogether by deflecting the operation to another flow.

However, If it’s something a little unpredictable (or I don’t want to enforce through workflow), I’d just “let it crash” and then treat the specific constraint error when the exception happens by redirecting the user back to the form and showing a nice message explaining the reason (this can easily be achieved using changesets).

The whole point of just letting things crash is to avoid too much defensive programming when you have more unknowns in the workflow that can lead to bad software state. I’d argue that you could avoid the bad state altogether by controlling the program flow or letting it crash (if you don’t know exactly what or how to handle the state), but this always depends on other factors like the software architecture and the workflow.

If you are using SPA, you might want to do validations in Javascript to avoid hitting the server.
If you have a REST API that is consumed by an integration, you might want to validate the inputs before actually doing something to avoid heavy lifting.
If you only have a form posting to an endpoint that goes to a context and calls a changeset, it might be light enough to process all the validations and return to the user.

IMHO, the amount of “defensive programming” you’ll apply drastically depends on constraints other than the one you are working with at the moment.

2 Likes

You should always validate the input, so you will know in that case that the doctor office ID is incorrect. There is no reason to crash the process here because you know how to handle that. You should return to the browser with a 400 error code, and display a generic error page, or redirect to the form with an error message.

And you can still call the Logger if you want to log errors.