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

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

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

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

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

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

О проекте

Приложение - часть большого проекта. В то же время и само приложение не одна большая "программулина", а состоит из нескольких отдельных "сервисов". В сумме это больше 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.