<!--IssueSummary start--> <details> <summary> Everyone can contribute. [Help move this issue forward](https://handbook.gitlab.com/handbook/marketing/developer-relations/contributor-success/community-contributors-workflows/#contributor-links) while earning points, leveling up and collecting rewards. </summary> - [Close this issue](https://contributors.gitlab.com/manage-issue?action=close&projectId=278964&issueIid=24974) </details> <!--IssueSummary end--> https://gitlab.com/gitlab-org/release/retrospectives/issues/1 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](https://gitlab.com/gitlab-org/gitlab-ce/issues/37763) 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.