Find a way to detect potential nil values that are not preceded by a conditional
gitlab-org/release/retrospectives#1 (closed) describes an incident with deploy tokens. Part of the issue was that the code in question performed the operation expires_at > X
, and expires_at
was nil. This then results in a NoMethodError
because NilClass
does not respond to >
. We have seen such errors many times before, and they are probably one of the most common Ruby errors.
Unfortunately solving this will be hard. In case of Ruby my personal preference would be the use of null objects (see this one year old issue for more details), but this would require serious refactoring. For example, every existing instance of if foo
or if !foo
would have to be changed to if foo.present?
and if !foo.present?
(or if foo.nil?
) respectively, because if
will treat everything as true, except for nil
and false
.
With that said, I think there is great value in using null objects. In this particular case, we could have returned some sort of NullDate
that does respond to >
, and just returns false
. To the best of my knowledge this would then result in the rest of the code working perfectly fine. However, we need to introduce this carefully, and preferably in an isolated way so we don't have to refactor everything up front.
Another solution we should look into is some form of static analysis. For example, using db/schema.rb
we can determine which columns allow NULL
values. We can then note down these names, and scan for patterns such as that_column_that_can_be_null > something
, and error if it is not preceded by if that_column_that_can_be_null
or something similar. This wouldn't require substantial refactoring, but it does require potentially complex static analysis. A benefit of this approach is that we can catch problems without requiring the developer to do anything extra, such as using null objects or writing better tests.