Сия заметка не столь для программистов, многие из которых уже сталкивались с подобным, и только улыбнутся «ну, открыл Америку» — а больше для разного рода менеджеров, заказчиков и всех кто считает что достаточно лишь нанять умных разработчиков, и дело в шляпе. Вот шляпой дело нередко и оборачивается.

Готовится релиз. Сроки подходят. Мне скидывают странный баг: Наше приложение вдруг стало жаловаться на невозможность соединиться с соседним.

А почему не может? Защищённое соединение не устанавливается.

А почему не устанавливается? Файлы сертификатов для этого соединения не удаётся загрузить.

А почему файлы не грузятся? А потому что путь к файлам «отсутствует в конфигурации».

А если руками залезть и глазами посмотреть — присутствует. Чудеса! Эффект Шрёдингера!

О проекте

Приложение — часть большого проекта. В то же время и само приложение не одна большая «программулина», а состоит из нескольких отдельных «сервисов». В сумме это больше 4500 исходных файлов кода, из которых, правда, 2500 это файлы тестов. Из файлов с тестами, правда, примерно половина — пустые, потому что специальная умная утилита создаёт их по шаблону в любой новой директории с исходниками.

Сервисы приложения используют некую «базу данных», её я беру в кавычки т.к. это довольно специфическое и притом небольшое хранилище — но для нашего рассказа это неважно. Знатоки сразу подметят «а хорошо ли что все сервисы вместе ходят в базу?» Действительно, этого часто стараются избегать — но тут сложилось что, в частности, некоторые параметры конфигурации хранятся в этой самой «базе» — вот как раз чтобы все сервисы могли удобно сходить и их прочитать.

И, как видим, один из параметров (один ли?) вдруг не читается.

Расследование показало

Смотрю в код сервиса, который не смог прочесть злосчастный «путь к сертификатам», а там что-то в духе такого (язык и названия изменены):

В одном из файлов инициализирующих сервис вызывается функция для определения пути к сертификатам:

function initSecureConnection() {
    // ...
    certificatesPath = certificatesParametersReader.ReadCertificatesPath()
    if (certificatesPath == ERROR_NOT_FOUND) {
        // кидаем ошибку
    }
    // ...
}

Не хухры-мухры, у нас целый вспомогательный класс для чтения параметров связанных с сертификатами — и пути к ним в частности. Как-то так:

class CertificatesParametersReader {
    // ...

    function ReadCertificatesPath() {
        // тут строк 30
        // лезем в базу...
        // устанавливаем сессию...
        // проверяем разные ошибки...
    }

    // ...
}

В целом написано всё солидно. Этот класс для чтения параметров густо покрыт «юнит-тестами», в частности только на метод ReadCertificatesPath который нас интересует — тестов штук 7 написано — проверяют разные случаи проблем при обращении к базе и так далее.

В общем, прихожу к выводу что тут проблемы нет. Если нужный параметр в базу записан — то сервис его прочтёт.

Значит его нет, когда его читают.

Но я же его вижу…

Хм… Посмотрим, откуда он в базе появляется. К сожалению эта часть про управление защищёнными соединениями и их сертификатами — это целый отдельный сервис — в общем, функционал мне незнакомый до сих пор — такое случается если проект довольно большой и ты много работаешь в основном с какими-то другими его аспектами.

Найти в проекте из тысяч файлов кто записывает строчку в базу — это всё же не иголку в стоге сена искать. По имени под которым данный параметр в базе находится, отыскиваю нужный файл. Тут всё не так развесисто, просто есть метод для записи:

function fillConfig() {
    // ...

    if (fillCertificatesPath(session) == ERROR) {
        return new Error("error while filling certificates path")
    }

    // ...
}

// ...

function fillCertificatesPath(session) {
    session.WriteData(certificatesPathParamName, certificatesPathParamValue)
}

Ну хорошо, включим уровень логгирования на котором видно, когда происходит запись в базу. Перезапустим сервис… Вот, опять ошибка… Смотрим, анализируем… Ну конечно:

• чтение параметра certificatesPath произошло в 17:38:24 (и обломалось)

• запись параметра certificatesPath произошла в 17:38:26 (на 2 секунды позже)

Очевидно, наша архитектура кривовата — сервисы вынуждены стартовать в строго определенной последовательности и не умеют дождаться пока один другому параметры в базе проставит. В общем много способов как это можно было сделать проще и лучше, но мы на них останавливаться не будем.

Потому что проблема даже не в этом!!!

Стал я размышлять как тут лучше поступить — то есть, перебирать в голове упомянутые «много способов». Поскольку речь идёт о релизе, тут надо действовать минимальными и наиболее безобидными изменениями, чтобы не задеть чего-нибудь ещё, чтобы не пришлось много перетестировать.

Самым очевидным кажется подсунуть куда-то вокруг (или внутри) функции чтения повтор с задержкой, ну в духе:

function ReadCertificatesPath() {
    // вынесем тело в отдельную функцию ниже, а здесь будет цикл
    delay = 100 * millisecond
    startTime = currentTime()
    while (currentTime() - startTime < 5000 * millisecond) {
        path = readCertificatesPathAttempt()
        if (path != ERROR) {
            return path
        }
        sleep(delay)
        delay = delay * 3 / 2
    }
}

function readCertificatesPathAttempt() {
    // прежние 30 строк тут
}

То есть пробуем вычитывать параметр в течение 5 секунд, например, и если это хоть раз удастся — возвращаем то что прочлось. Иначе делаем паузу (с постепенно увеличивающейся задержкой, чтобы поменьше «бомбить» базу) и повторяем попытку.

Но прежде чем делать это…

Дай-ка, пожалуй, посмотрю, а откуда изначально вообще этот путь к сертификатам приходит, откуда он передаётся в функцию сохранения. Помните вот это:

    session.WriteData(certificatesPathParamName, certificatesPathParamValue)

Вот что это за certificatesPathParamValue — при первом чтении я не обратил внимания — а сейчас глянул на определение (оно, как модно, оказывается в другом файле):

const certificatesPathParamValue = "/etc/trusted/common.crts"

То есть это константа. Вся эта свистопляска с методами для записи значения в базу, с мудрёным классом для его вычитывания, с тестами на этот класс — ради передачи «захардкоженного» значения, которому не судьба в жизни никогда поменяться.

Я ещё подумал — может, чего-то не догоняю? Может этот параметр какими-то силами можно проставить извне, поменять путь к сертификатам (зачем? даже звучит странно — но мало ли…)

Ан нет, ведь вычитывается он только на старте, а при старте наших сервисов этот «конфигурационный» раздел базы очищается и перезаписывается — ну, сами видите, на старте вызывается fillCertificatesPath — и затрёт любые изменения.

Анализ

В «релизной» ветке допустимы только минимальные изменения — я просто меняю код так чтобы в случае ошибки чтения параметра сервис не падал с ошибкой, а просто возвращал такую же константу (это очень глупо звучит, но код релиза требует бережного обращения).

В основной ветке, конечно, я выпиливаю код относящийся к чтению и записи этого параметра — в сумме уходит несколько сотен строк, вместе с «тестами» (которые проверяют класс, который вычитывает из базы записанную туда константу) — поскольку этот же код ещё и раздублирован с небольшими изменениями в другие сервисы которым тоже нужен этот путь к сертификатам (они тоже могут упасть, поэтому в релизной ветке делаю поправку и для них).

Как же это получилось? Откуда такой сложный код для выдачи константы? Можно подумать что это результат исторический — быть может когда-то давно это не было константой… И только после в результате рефакторинга сложилось так, как сложилось.

Но нет, код проекта ещё пока в состоянии «стартапа», и хотя он пишется уже не первый год, у него нет исторической коньюнктуры. Более того — я уже видел (и выпиливал) подобные штуки в нём пару месяцев назад — просто тогда они не доводили до багов. Просто увидел, удивился, удалил.

Итак, как же это получилось?

Это не от глупости ведь. Наоборот видно, код развесистый, сложный, мудрёный — настолько что вот и тестами его солидно покрыть пришлось. Глупец так не сделает, так только умный сделает. Может быть, слишком умный. В разработке было некое поветрие, увлечение «паттернами проектирования» — то есть, «шаблонами», якобы, полезными — и вот здесь угадываются черты этого «шаблонного мышления» (в обоих смыслах).

Нельзя просто так взять и вычитать параметр.

Нужен класс Reader к параметру.

Нужен интерфейс на этот класс.

Нужны тесты на этот класс.

Нужны «моки» на этот интерфейс.

И только одна маленькая деталь упущена — параметр вообще не требовалось вычитывать. Но поскольку проект такой здоровый, поскольку запись в другом сервисе, поскольку над кодом работают человек 20 программистов — это прошло незамеченным.

Незамеченным? Да нет, ведь, как я сказал, это не случайность — подобных константных параметров в коде не так уж мало…

Это образ мысли.

Со стороны (менеджерам, заказчикам) кажется что проект такой большой потому что в нём много функционала. Я же на глазок оцениваю что тут фактической работы на 3 человек на полгода — всё остальное — это вот подобная «развесистая клюква».

И это не в какой-то «сомнительной шараге» — контора солидная, в топах разных рейтингов (и на хабре / хабр-карьере в том числе). Тысячи человек народу. Они пишут миллионы файлов исходного кода. Представьте процент этой «клюквы» в этих миллионах файлов. Пересчитайте это во время и деньги.

Хуже того — как видим, временами эта ересь ведёт к сбоям — хорошо что поймали до релиза, а не после. Было бы сложнее, больнее, дольше и дороже разбираться.

И ведь это не диверсанты делают, это не со зла. Из лучших побуждений — а вдруг когда-то понадобится нарастить этот функционал — а у нас уже все готово.

А вдруг — не понадобится? Почему бы не добавлять код тогда, когда он нужен. Это же код, его можно редактировать!

Не зря говорится: Don’t be too clever.