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

Об ошибке

У нас на работе долгоживущее SPA приложение, которое пользователь может не закрывать очень долгое время. И в какой-то момент мы заметили, что некоторые Observable HTTP запросы никогда не завершались.

После небольшого исследования выяснилось, что Angular никак не реагирует на внешний XMLHttpRequest.abort(), если браузер сам отменяет HTTP запросы. Такая ситуация возникает, когда пользователь переводит устройство в спящий режим (конкретно наш случай) или нажимает Ctrl+S с целью сохранить веб-страницу. Мы даже нашли соответствующий issue. Более того, кто-то уже сделал пулл-реквест (ПР) на фикс этого бага. Мы написали обходное решение с использованием XhrFactory внутри нашего проекта, пока этот фикс попадает в какой-то из релизов Angular, и успешно забыли об этом.

Каково же было мое удивление, когда я зашел на страницу issue спустя полгода, а он по-прежнему открыт. Плюс ПР отменили из-за того, что автор не правил ревью комменты. Тогда я решил, что это мой шанс попробовать в open source разработку.

Контрибьют

Прежде всего я обратился к гайдлайнам для контрибьютеров, которые лежат в CONTRIBUTING.md файле. Вот список того, что нужно сделать, чтобы ваш ПР приняли в Angular:

1. Убедиться, что на баг или фичу еще нет ПР на github.

2. Форкнуть проект.

3. Ответвиться от master ветки.

4. Написать свой код и написать как минимум один тест на него, следуя Code Style.

5. Запустить тесты Angular.

6. Сделать коммит с правильным сообщением.

7. Запушить свою ветку.

8. Подписать Contributor License Agreement (CLA).

9. Сделать ПР в Angular в master ветку.

Первые три пункта, думаю, можно не комментировать. Интересное начинается с четвертого. Мне было легче, так как уже был отмененный пулл-реквест, который нужно было только улучшить. Но давайте пройдемся по флоу от начала до конца. Как бы я теоретически правил ошибку, не имея никаких наработок.

Проблема возникает в запросах к серверу. Значит, нам нужно в http модуль. Быстро пробежав по репозиторию, находим нужный пакет. Знаем, что ошибка связана с xhr и его событием abort, ищем, где внутри http модуля создается XMLHttpRequest и как обрабатывается abort событие. Находим файл xhr.ts, немного изучив, что там происходит, понимаем, что он-то нам и нужен.

Ищем в этом файле слушателя на событие abort. Видим, что событие никак не обрабатывается. Бинго!

Получается, что нужно просто назначить обработчик события abort, чтобы по нему Observable HTTP запроса завершался с ошибкой. И не забыть написать на это дело тест. В xhr.ts уже есть функция onError, можем использовать ее. Делов на две строчки кода.

Были ещё мысли, что можно обрабатывать внешний аборт, как HttpEvent вместо ошибки, тогда все немного сложнее. Но, на мой взгляд, ситуация, когда твой запрос неожиданно прервали внешние силы, является скорее ошибкой.

Код, который правит баг, написан. Дальше необходимо написать юнит-тест на этот код, дабы при изменении функциональности http модуля ничего не сломалось. У каждого файла в папке src есть свой файл с расширением .spec.ts для тестов в папке test. Алгоритм очень простой — находим максимально похожий тест и немного изменяем. Мне ещё нужно было расширить один mock.

Тесты написаны. Теперь нужно запустить и прогнать все тесты на проекте. Это может занять много времени. Будьте готовы. 

При написании тестов также можно проверять отдельный модуль, это будет намного быстрее. Например так:

yarn bazel test packages/common/http/test

Все тесты прошли. Пора коммитить наши изменения. Перед этим советую сделать yarn lint, если вы до этого не настраивали автоматическое форматирование в виде clang. Code style — наше все.

Переходим к самому коммиту. В Angular строгое отношение к наименованию коммитов. Поэтому внимательно читаем секцию про commit message в гайдлайнах контрибьютеров и следуем содержимому.

Теперь публикуем нашу ветку. Осталось совсем немного. Необходимо подписать CLA с компанией Google. Это позволяет контрибьютеру сохранить свое право собственности на код, предоставляя Google необходимые юридические права на использование этого кода. CLA нужно подписать только один раз, и он охватывает все проекты Google.

Дальше все легко. Делаем ПР и ждем, когда его посмотрят разработчики Angular.

Код ревью

После прочтения данной публикации на Хабре я был готов к длительному ожиданию. Но мой пулл-реквест посмотрели спустя 13 минут! Уж не знаю, это мне очень сильно повезло или команда Angular стала уделять больше времени сторонним разработчикам, но было очень круто так быстро получить фидбек по своему коду.

Andrew Kushnir аппрувнул мой ПР и сказал, что мои изменения кажутся ему адекватными. Он также добавил в обсуждение Alex Rickabaugh, который имеет больше контекста по этой ошибке. Alex поставил аппрув спустя 30 минут и написал, что изначально у него были некоторые опасения по поводу моего решения, но немного поразмыслив, он с ним согласился.

Однако на этом все не закончилось. Оказалось, что есть похожий пулл-реквест, который правит ошибку, связанную с xhr timeout, и код там почти идентичен моему, только вместо abort события — timeout. И у нас возникнут merge конфликты, когда этот ПР вольется. Меня попросили сделать rebase и поправить конфликты, когда настанет время. Я вежливо согласился. Спустя 15 минут мне написали о появлении ожидаемых конфликтов. Я разрешил их и сделал push с  --force-with-lease флагом.

На следующий день Andrew Kushnir поставил метку “merge” на пулл-реквест и поблагодарил меня за вклад. А буквально через два часа мои изменения влили в проект.

Спустя еще пару дней обновили CHANGE_LOG.md, что баг поправлен и войдет в 12 версию Angular.

Итоги

Это был очень интересный опыт. Было занимательно посмотреть структуру фреймворка, на котором пишешь долгое время, запустить тесты и узнать какие процессы в Angular команде. Как оказалось, вносить свои изменения в open source фреймворк совсем не сложно.

UPD

Про git push --force-with-lease. Если еще кто-то коммитил в вашу удаленную ветку и ваша локальная ветка не содержит этих коммитов, вы не сможете сделать push с этим флагом, пока не обновите свою локальную ветку. Данный флаг гарантирует, что вы не перепишете чужую историю коммитов. Его рекомендуют использовать в гайдлайнах контрибьютеров, если вам необходимо обновить git историю удаленной ветки.