3

I've started to write unit tests in Node.js (using TypeScript) for the backend, using Mocha+Chai+Sinon libraries

As part of making the function return a correct output for any given input, It makes sense for example, in login method: login(userName: string, password: string) : LoginResult I believe its must to verify that the given username and password are strings and not undefined.

The TypeScript engine will alert when the given input is not string, but it will not alert that the given input is undefined which is invalid input for that unit (the tested method).

Therefore, In the start of the code I am using asserts:

assert(userName !== undefined, 'Given "userName" is "undefined"); assert(password !== undefined, 'Given "password" is "undefined");

And I wonder if this is the correct approuch to use asserts in production code for TDD/defensive programming input validations and validate responses from other methods, or it can be improved somehow.

I am using assert-plus library that can disable the asserts on production based on environment variable which is a plus, but I am not sure if I should since the asserts allows to give much more indicative errors and avoiding running code in unexpected conditions.

Thanks!

4
  • 1
    I will say don't use asserts, use functions like isValidUsername or isValidPassword - but others may say asserts are the way to go. This is an opinion based question due to that, and I'm close-voting as such Commented Jun 15, 2018 at 8:51
  • It's same thing as if (userName !== undefined) throw new Error('Given "userName" is "undefined"'). You just need to have this in your code unless you want to have cryptic exceptions or even worse, invalidated data. Unless you have your assertions covered by tests (and not every of them can be tested because it depends on the input), it's unreasonable to disable them in production. Probably makes sense to have 2 copies of assert-plus, where one improves debugging and can be disabled in production and another is vital and cannot be disabled. Commented Jun 15, 2018 at 9:19
  • See also stackoverflow.com/questions/37603093/… Commented Jun 15, 2018 at 9:24
  • I would not use them. You should be in control of your input validation code. The implementation of assert may change or get deprecated (not likely, but might). Or imagine that you're using your isValidPassword logic multiple places in your code and you have to change how it behaves. Commented Jun 15, 2018 at 13:47

1 Answer 1

5

Input validation is really important, but it depends on where the code is located in your software and in the end is comes down to a judgement call. There are two ways of thinking about it and you generally see these two points of view disagreeing allot.

Always Validate Everywhere

This assumes that your code will be taken out of context and thus you need to validate for invalid user input in the function before it is used. This is commonly backed up with well documented exploits. In this case you want to convert each assertable situation and convert it to if & throw as @Estus Flask commented. The obvious downside of this is that you may spend allot of time validating the same piece of data over and over.

If you adhere to this physiology, don't use assert in production code, convert each assert to an if statement and thow an exception with a reasonable readable error message.

Validate at the System Boundary

This assumes that you have well defined client code and can don't pass invalid data into to critical parts. Although you still do everything to prevent remote code execution, but accessing a member of an undefined value and "crashing" is acceptable behavior for misbehaving client code. The bug is in the client code and that should just not pass invalid data into the function. The input validation then is all done in the most outer layer of your application / library. (e.g. in the REST route handlers) But it still makes sense to document your assertions about the data in the code in the form of asserts. Theses assert should probably be out in production for performance reasons, but are acceptable in staging and test code. (You could keep them on during a blue / green deployment process to check if the new code behaves.)

If you adhere to this physiology, use asserts in production code to document the assertion the code makes about the input data and state of the world. These assertions should be off most of the time.

Sign up to request clarification or add additional context in comments.

1 Comment

Good answer man!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.