Реальные баги в импортерах Zulip: как их найти, почему важно проверять трекер и что делать, когда вашу находку уже исправляют. Практические уроки для разработчиков.
Вы когда-нибудь находили баг, который уже исправлен? Я — да, и это оказалось не разочарованием, а ценным опытом. В этой статье я расскажу, как искал ошибки в импортерах Zulip, наткнулся на два реальных бага, которые оказались уже известны мейнтейнерам, и как в итоге принес пользу проекту, найдя неотмеченные проблемы и предложив исправления.
Zulip — отличная цель для поиска багов: он на Python, входит в список рекомендованных репозиториев для челленджа, и у него репутация кодовой базы с высокими стандартами. Почти полное покрытие тестами, строгая типизация mypy, жестокий линтер и дисциплина коммитов «каждый коммит — минимальная связная идея». Я прогнал расширенный ruff по всему бэкенду, чтобы проверить базовый уровень. Ничего, кроме стилевого шума.
Ошибки уровня линтера в такой кодовой базе не выживают. Значит, нужно искать логические ошибки, а для этого нужна подсистема, где корректность сложна, а входные данные враждебны. Импорт данных — идеальный кандидат: длительные пакетные задания, обрабатывающие экспортные файлы других инструментов, произвольное качество данных и отсутствие культуры повторных попыток, потому что миграция рабочего пространства — разовое событие для администратора. Один необработанный крайний случай на сообщении 31 000 из 50 000 — и все ломается.
Сообщения Slack обрабатываются конвертером чанками по 1000. Карта, которая направляет ответы в треды на соответствующие темы Zulip, была создана внутри функции, обрабатывающей каждый чанк. Корень треда в чанке N, ответы в чанке N+1: ответы попадают в тему-сироту с буквальным названием «... No channel message». Деталь, которая делает этот баг искусством: соседний кэш в том же файле был намеренно сделан глобальным для модуля, с комментарием, объясняющим, что он должен переживать вызовы. Один кэш получил межчанковую обработку. Его собрат, добавленный позже, — нет.
Идентичность треда вычислялась как strftime("%Y/%m/%d %H:%M:%S") плюс идентификатор родительского пользователя. Без компонента канала. Без микросекунд, хотя исходный ts в Slack содержит их как строку прямо в сообщении. Любой бот, который публикует дважды в течение одной секунды и получает ответы на оба сообщения: два разных треда сливаются в одну тему, диалоги перемешиваются.
Оба бага молчаливые. Никаких исключений, никаких предупреждений — просто тихо поврежденный архив.
Я был доволен собой примерно столько времени, сколько потребовалось, чтобы поискать в трекере.
Находка #39650 уколола, а затем стала поучительной: PieterCK, мейнтейнер, владеющий импортерами, уже имел открытый PR #39757 «slack_importer: Fix Slack thread conversion bugs». Я открыл вкладку Files changed с упавшим сердцем. Оба моих бага с тредами исправляются этим человеком, с большим контекстом, чем у меня когда-либо будет.
В этом челлендже есть раздел «Smash Bugs, Respectfully» о том, чтобы не увеличивать нагрузку на мейнтейнеров. Гонка с открытым PR мейнтейнера по его же пунктам аудита — классический способ провалить это. Поэтому: отступаю по багам с тредами. Это не обсуждается и, честно говоря, не разочаровывает, если правильно сформулировать: независимое повторное открытие двух пунктов из аудита мейнтейнера — не потерянная работа. Это калибровка. Мой нос указывал на реальные баги; просто он указывал на них вторым.
Если аудированный файл вычищен, остаются два хода: найти то, что аудит пропустил в другом месте, и найти то, что аудит нашел, но никто не занял.
Аудиты покрывают файлы, но классы багов путешествуют между файлами, переносимые копипастой и общими привычками. Я взял классы багов из аудита Slack и проверил соседние импортеры. Mattermost: чисто по этим классам, другой вспомогательный механизм батчинга. Microsoft Teams, новейший импортер в семействе: джекпот.
Его генератор батчей возвращал список, а затем вызывал .clear() на том же объекте для построения следующего батча. Проверьте минимальную версию сами:
def batched(iterable, size):
batch = []
for item in iterable:
batch.append(item)
if len(batch) == size:
yield batch
batch.clear() # Ошибка: очищаем тот же список!
print(list(batched(range(12), 5)))
# Ожидалось: [[0, 1, 2, 3, 4], [5, 6, 7, 8, 9], [10, 11]]
# Фактически: [[10, 11], [10, 11], [10, 11]]Каждый выданный батч — один общий список. Десять из двенадцати сообщений потеряны, ноль исключений. Баг латентный: текущий потребитель в Zulip обрабатывает каждый батч до перехода к следующему, поэтому сегодня ничего не ломается. Но это нарушенный контракт: потребитель владеет выданным значением, и любой будущий потребитель наследует ловушку, которая молча уничтожает данные во время разовой миграции. Никто об этом не сообщал. Это стало PR #39814.
В #39650 один пункт с высоким влиянием сидел подтвержденным и незанятым: незащищенный ключ сортировки по временной метке. float(message["ts"]) используется для сортировки всех сообщений экспорта и как date_sent. Я проверил открытый PR мейнтейнера, Ctrl+F во Files changed: не найдено. Свободно.
Пункт, как он был подан, имел два режима отказа: отсутствующий ts вызывает KeyError, мусорный ts вызывает ValueError; любой из них прерывает весь импорт из-за одного сообщения. Написание защиты выявило третий режим, который никто не перечислил, и это лучший сувенир всей охоты:
"ts": "NaN"float("NaN") парсится без жалоб. NaN сравнивается как False со всем, что тихо нарушает полный порядок, предполагаемый Timsort, поэтому sorted() возвращает несогласованный порядок. Ни краха, ни предупреждения, только перепутанная хронология.
Крах — это удачный режим отказа.
Именно поэтому отправленная защита требует math.isfinite, а не просто успешного парсинга. Это стало PR #39813.
Оба исправления прошли через одну и ту же процедуру. Сначала пишем регрессионный тест, откатываем исходник до upstream/main и наблюдаем, как тест падает с точно предсказанной ошибкой: KeyError: 'ts' для защиты Slack, AssertionError: 24 != 29 для алиасинга Teams (батчи измеримо съедают сообщения). Затем восстанавливаем исправление и запускаем все: 56/56 на модуле импортера Slack, 10/10 на Teams, линт и mypy чисты, покрытие показывает отсутствие непокрытых строк в затронутом модуле Slack. Регрессионный тест, который никогда не падал на старом коде, — это тест, подогнанный под исправление, а не тест исправления.
Полное раскрытие процесса: механическая часть охоты (клонирование, grep, запуск наборов тестов) прошла через AI-агентные инструменты; каждое решение, о котором вы прочитали — от того, какой репозиторий выбрать, чей баг не обгонять, какую политику отказа отправить, — осталось человеческим. У Zulip есть явная политика использования ИИ для контрибуций, и оба PR следуют ей.
Для сабмишена на другом треке я подключил крах к Sentry и попросил Seer, его ИИ-отладчик, найти корневую причину. Должен отдать должное: Seer поставил диагноз за секунды, вплоть до цитирования точного отравленного сообщения, которое он извлек из локальных переменных фрейма. Затем он предложил исправление:
float(message.get("ts", 0))Этот однострочник — вариант запасной временной метки, от которого я уже отказался в PR. Он закрывает режим отсутствующего ts, оставляет ValueError живым, пропускает "NaN" прямо в сортировку и штампует реальные сообщения датой 1970 года. Один режим отказа из трех, плюс сфабрикованная хронология. Инструмент нашел корневую причину быстрее, чем я бы; решение о политике отказа осталось за мной. Подозреваю, что такое разделение труда будет описывать много отладок в будущем.
Вы когда-нибудь прибывали на две недели позже к своему собственному открытию? И вы записали это как потраченные усилия или как доказательство, что ваш нос работает? Я твердо перешел во вторую колонку.
Хочешь закрепить знания на практике?
Решай задачи на Algolit — интерактивная платформа для обучения
Начать бесплатно →