Skip to content

Refactor#27

Merged
AMEST merged 10 commits into
masterfrom
refactor
May 8, 2026
Merged

Refactor#27
AMEST merged 10 commits into
masterfrom
refactor

Conversation

@AMEST
Copy link
Copy Markdown
Owner

@AMEST AMEST commented May 7, 2026

Refactoring:

  • Migrate to .net 10
  • Update nodejs to 20
  • Remove callback hall in frontend
  • Replace old "bad" grid and make more pretty grid for participants
  • Minor refactoring (vars, async, code style (front), refactor store, and other)
  • Add /api/health with redis and coturn healthchecks

AMEST added 4 commits May 7, 2026 21:56
WebHost minor refactoring with .net10 migration
Add healthchecks
@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

/start_review

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • Dockerfile
  • src/PeerMeeting.Domain/PeerMeeting.Domain.csproj
  • .vscode/launch.json
  • src/PeerMeeting.Host/ClientApp/package.json
  • src/PeerMeeting.Host/ClientApp/src/App.vue
  • src/PeerMeeting.Host/ClientApp/.eslintrc.js

Плюсы

  • Переход с .NET Core 3.1 на .NET 10.0 и с Node.js 14 на 18 — поддержка актуальных версий, улучшение производительности и безопасности.
  • В App.vue устранён классический баг с потерей контекста this в mounted за счёт замены var self = this на стрелочную функцию.
  • Использование const и let вместо var, современный синтаксис (стрелочные функции, async/await) — код стал чище и читаемее.
  • Выравнивание и форматирование кода (пробелы, точки с запятой, кавычки) улучшено по стандартам Vue/JavaScript.
  • Добавлена поддержка HTTPS и HTTP через ASPNETCORE_URLS — гибкость при локальной разработке.
  • Обновлён Dockerfile: установка Node.js через официальный скрипт для версии 18 и использование актуальных образов dotnet/sdk и dotnet/aspnet.

⚠️ Проблемы

  1. Безопасность: отключён no-console в production.
    В .eslintrc.js правило 'no-console' установлено в 'off', а не process.env.NODE_ENV === 'production' ? 'error' : 'off'. В production логи могут раскрыть чувствительные данные (например, структуру запросов, ключи).
    Рекомендация: Вернуть оригинальное закомментированное правило или использовать условное отключение.

  2. Отсутствие обработки ошибок в created().
    В App.vue два асинхронных запроса (/api/version и /api/credentials) без try/catch. Если сервер вернёт ошибку (например, 500), loaded останется false, и приложение зависнет в состоянии загрузки.
    Рекомендация: Обернуть блок в try/catch, в catch хотя бы выводить ошибку в консоль и устанавливать loaded = true для дальнейшей работы.

  3. Потенциальное некорректное получение версии в Dockerfile.
    Команда sed -i -e "s/<Version>0-develop<\/Version>/<Version>$(cat version | cut -c2-)/" предполагает файл version в текущем рабочем каталоге /build. Файл скопирован как /build/version, поэтому cat version сработает. Но если путь изменится (например, если WORKDIR будет другим), возникнет ошибка.
    Рекомендация: Указать абсолютный путь: cat /build/version.

  4. Небезопасное экранирование username в POST-запросе.
    В created(): axios.post('/api/credentials', 'username='+username). Если profile.name содержит символы вроде & или =, строка может быть интерпретирована неверно, хотя сервер, вероятно, парсит тело запроса как URL-кодированное. Лучше использовать объект: axios.post('/api/credentials', { username }) и установить правильные заголовки.
    Рекомендация: Передавать данные как объект JSON или FormData с правильной сериализацией.

  5. Проверка на null для profile неполная.
    this.$store.state.application.profile != null ? this.$store.state.application.profile.name : uuidv4() — если profile существует, но name равен undefined, то uuidv4() не будет вызван, и username станет undefined.
    Рекомендация: Более строгая проверка: profile?.name ? profile.name : uuidv4().

  6. Уязвимость: ASPNETCORE_URLS привязан к 0.0.0.0.
    В launch.json указано https://0.0.0.0:5001;http://0.0.0.0:5000. В локальной разработке это нормально, но может позволить другим устройствам в сети обращаться к приложению без авторизации.
    Рекомендация: Использовать localhost или 127.0.0.1, если внешний доступ не нужен.

💡 Советы

  • Рефакторинг App.vue: Вынести логику получения версии и TURN-кредов в отдельные методы (например, fetchVersion(), fetchTurnCredentials()) и вызывать их последовательно в created. Это улучшит читаемость и тестируемость.
  • Упростить Dockerfile: Выделить шаг получения версии в отдельный слой, а не объединять в одной длинной команде RUN echo... ;. Например, использовать RUN git describe --tags --always > /version и затем копировать.
  • Добавить try/catch в created(): Обрабатывать ошибки запросов, логировать их и, возможно, показывать пользователю сообщение об ошибке (через this.$store.commit или вызов алерта).
  • В .eslintrc.js заменить 'no-console': 'off' на 'no-console': process.env.NODE_ENV === 'production' ? 'error' : 'off', чтобы в production ошибка сборки не проходила с console.log.
  • В package.json: Рассмотреть использование npx с fallback'ом на локальную установку, но лучше установить @vue/cli-service локально как devDependency и использовать его напрямую, чтобы не зависеть от глобального npx. Аналогично для --openssl-legacy-provider — это временное решение, лучше обновить зависимости или сам Vue CLI.
  • В Dockerfile: Указать конкретную версию Node.js 18 (например, setup_18.x), но лучше использовать официальный образ Node.js в multistage build, а не добавлять репозиторий в сборочный образ.
  • Добавить проверку CSRF? В целом для внутреннего API это не обязательно, но стоит рассмотреть использование антифордж-токенов для POST-запросов, особенно если приложение будет развёрнуто публично.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/CommonUtils.js

Плюсы

  • Современный синтаксис ES6+ (const, сокращённые методы, стрелочные функции, строгие равенства) делает код более читаемым и предсказуемым.
  • Добавлен export default CommonUtils, что позволяет импортировать модуль в других файлах.
  • Убраны лишние параметры в find и неиспользуемые переменные, сокращена избыточность.
  • Замена var на const гарантирует отсутствие случайного переопределения объекта.
  • Использование строгого сравнения === вместо == уменьшает риск неявных приведений типов.

⚠️ Проблемы

  1. Типы идентификаторов в addToHistory
    Сравнение e.id === room.id может не сработать, если id в store.state.application.roomHistory хранится как число, а room.id приходит строкой (например, из JSON). Это приведёт к тому, что существующая комната не обновится, а добавится новая.
  2. Мутация объекта extra в extractExtraData
    Функция напрямую добавляет свойства audioMuted и videoMuted в переданный объект extra. Если этот объект используется повторно в других частях приложения, он будет неожиданно изменён. Кроме того, если connection.getExtraData(userid) вернёт null или undefined, код упадёт с ошибкой при попытке обратиться к extra.audioMuted.
  3. Некорректный возврат в getUserNameFromEvent
    При event.userid.split('|') может получиться массив из одного элемента (если нет символа |). Тогда event.userid.split('|')[1] будет undefined, и функция вернёт undefined. Блок catch сработает только при исключении, а не при этой ситуации.
  4. Избыточный try/catch в getAvatarFromEvent
    Ошибка может возникнуть, только если event или его вложенные свойства равны null/undefined. Проще использовать event?.extra?.profile?.avatar ?? null (если допустим optional chaining). Текущий catch просто игнорирует ошибку, что может скрыть настоящие проблемы.

💡 Советы

  • Для addToHistory приведите room.id к тому же типу, что и e.id, или используйте нестрогое сравнение с явным приведением: e.id == room.id.
  • В extractExtraData создавайте новый объект без побочных эффектов:
    const extra = { ...(connection.userid === userid ? connection.extra : connection.getExtraData(userid)) };
    return {
      audioMuted: extra.audioMuted || false,
      videoMuted: extra.videoMuted || false,
      ...extra
    };
  • В getUserNameFromEvent замените try/catch на проверку:
    if (event.extra?.profile?.name) return event.extra.profile.name;
    const parts = event.userid.split('|');
    return parts.length > 1 ? parts[1] : event.userid;
  • В bytesToSize добавьте проверку, что bytes — число, и обработайте случаи с очень большими значениями (например, добавив 'PB' в массив).
  • Для единообразия стиля везде используйте optional chaining вместо try/catch (если целевая среда поддерживает ES2020).
  • Добавьте JSDoc-комментарии для описания параметров и возвращаемых значений — это упростит работу с модулем.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/RTCUtils.js

✅ Плюсы:

  • Современный стиль: замена var на const/let, стрелочные функции, camelCase для методов, добавлены фигурные скобки в if.
  • Убраны лишние комментарии и eslint-disable, код чище.
  • Исправлена опечатка в имени события (speachspeech), устранён потенциальный баг.
  • Замена callback-ов на Promise в addBaseStream и addStream – упрощает асинхронное управление и цепочки вызовов.
  • Удалены дублирующиеся переменные (self = this), код короче и понятнее.

⚠️ Проблемы:

  • addBaseStream теперь асинхронен, но старый вызвающий код может не использовать await/.then → сломается. Необходимо обновить все места вызова.
  • Ветка hasMicrophone && !hasWebcam && dontCaptureUserMedia внутри addBaseStream возвращает Promise, но не обрабатывает ошибки getUserMediareject останется необработанным, если нет внешнего .catch. Ранее ошибка попадала в connection.onMediaError.
  • Ветка без микрофона и веб-камеры вызывает createFakeStream, который возвращает объект, а не Promise. Из-за этого async addBaseStream вернёт не Promise, а объект – нарушает единообразие и может привести к ошибкам времени выполнения.
  • В createFakeStream жёстко используется connection.multiPeersHandler, хотя раньше он передавался параметром. Если в других сценариях этот объект может отсутствовать или быть другим, возникнет баг.
  • addStream всё ещё использует setTimeout(..., 300) внутри Promise. Это может вызвать неожиданные задержки. Если задержка не обязательна, лучше убрать или сделать конфигурируемой.

💡 Советы:

  • В addBaseStream оберните результат createFakeStream в Promise.resolve(), чтобы всегда возвращать Promise.
  • Добавьте .catch в цепочки Promise внутри addBaseStream (или перенаправляйте ошибки в connection.onMediaError).
  • Убедитесь, что все внешние вызовы addBaseStream переписаны на await или .then.
  • Рассмотрите возможность убрать setTimeout в addStream или заменить на requestAnimationFrame, если задержка нужна для рендеринга.
  • Замените alert в configureMediaError на более подходящее уведомление (например, console.warn или всплывающий toast).
  • Для надёжности добавьте проверку connection.multiPeersHandler перед вызовом его методов (в createFakeStream, addStream, configureMediaError).

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/WebRtcHub.js

✅ Плюсы

  • Код написан в современном стиле ES6+: используются const, let, стрелочные функции, шаблонные строки.
  • Улучшена читаемость за счёт правильных отступов, точек с запятой и форматирования.
  • Вынесена вспомогательная функция isData, что уменьшает дублирование.
  • Добавлено условное логирование (if (window.pmDebug)), что полезно для отладки.
  • Обработка ошибок при создании подключения через try/catch.
  • Используется автоматическое переподключение (withAutomaticReconnect).

⚠️ Проблемы

  1. Потенциальная ошибка при неудачном создании signalRConnection.
    В блоке catch ошибка логируется, но signalRConnection остаётся undefined. Все последующие вызовы (signalRConnection.on, signalRConnection.start) вызовут исключение.
  2. Вызов несуществующего метода emit.
    В обработчиках onreconnected и .start().then() используется connection.socket.emit('presence', …), но connection.socket определён только как { send: … } и не имеет метода emit. Это приведёт к падению приложения.
  3. Риск бесконечной рекурсии.
    В onMessagesCallback при connection.waitingForLocalMedia вызывается setTimeout(() => onMessagesCallback(message), 1). Если условие не изменится, функция будет вызывать саму себя бесконечно.
  4. Уязвимость прототипного загрязнения (prototype pollution).
    Код присваивает connection.peers[data.data.remoteUserId].extra = data.data.extra, не проверяя, что data.data.extra может содержать опасные ключи (например, __proto__).
  5. Использование alert() в продакшен-коде.
    В блоке catch после неудачного старта вызывается alert(JSON.stringify(e, …)). Это неприемлемо для пользовательского интерфейса.
  6. Отсутствие проверки на существование connection.peers.
    В onMessagesCallback есть обращения connection.peers[message.sender] без проверки, что connection.peers определён.
  7. Возможный редирект в бесконечный цикл.
    При кике window.location.href = window.location.href + '/ended' может накручивать /ended/ended/…, если страница уже содержит /ended.
  8. Повторяющиеся вычисления sdpConstraints.
    Один и тот же набор localPeerSdpConstraints и remotePeerSdpConstraints вычисляется в нескольких местах, что усложняет поддержку.

💡 Советы

  1. Добавить guard clause после try/catch:
    if (!signalRConnection) {
      console.error('SignalR connection not created');
      return;
    }
  2. Заменить вызов emit на send:
    В коде уже есть метод send, который вызывает signalRConnection.invoke. Использовать его для отправки сообщения «presence».
  3. Устранить возможную бесконечную рекурсию:
    Добавить счётчик попыток или тайм-аут, либо перенести логику ожидания в отдельный обработчик.
  4. Защититься от prototype pollution:
    При присвоении extra использовать Object.assign({}, data.data.extra) или явное копирование свойств.
  5. Заменить alert на более цивилизованное уведомление:
    Например, показать сообщение в UI или отправить ошибку в систему мониторинга.
  6. Вынести повторяемые ограничения в отдельные функции:
    const getSdpConstraints = (connection, isRemote = false) => {  }
  7. Проверить обработку кика:
    Использовать window.location.replace() или обрезать /ended перед добавлением.
  8. Добавить проверку существования connection.peers перед доступом:
    if (connection.peers && connection.peers[message.sender]) { … }.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/components/TopMenu.vue
  • src/PeerMeeting.Host/ClientApp/src/components/room/Chat.vue
  • src/PeerMeeting.Host/ClientApp/src/components/room/participant/ConnectionInfo.vue

Плюсы:

  • Улучшен стиль кода: замена var на const/let в Chat.vue, удаление избыточного this. в шаблонах всех файлов (this.userIduserId, this.statsstats и т.д.).
  • Единообразное использование одинарных кавычек и отказ от точек с запятой (если это соответствует настройкам линтера).
  • Переход на сокращённый синтаксис методов (signOut() {...} вместо signOut: function() {...}) и стрелочные функции в анонимных обработчиках.
  • В Chat.vue исправлено сравнение с пустой строкой (=== вместо ==), что исключает приведение типов.
  • Асинхронный код в Chat.vue переписан с использованием async/await вместо цепочки .then() — улучшена читаемость.
  • Добавлена пустая строка в конце всех файлов (устранён no newline at end of file), что соответствует стандарту.
  • Удалены ненужные комментарии (например, <!-- Right aligned nav items -->) и подавление линтера (// eslint-disable-next-line).

⚠️ Проблемы:

  • Отсутствие обработки ошибок в Chat.vue при инициализации соединения. В async created() вызовы connection.start() и connection.invoke() не обёрнуты в try/catch. Если WebSocket упадёт или сервер недоступен, пользователь не получит уведомления, а консоль будет молчать (ранее ошибки также игнорировались, но сейчас код стал асинхронным и может «упасть» неожиданно). Критичность: высокая (снижает надёжность).
  • Нет отключения SignalR-соединения при уничтожении компонента Chat.vue. Отсутствие хука beforeDestroy с вызовом this.connection.stop() может привести к утечке памяти и подключений при переключении между комнатами. Критичность: средняя (потеря ресурсов).
  • Метод signOut в TopMenu.vue использует полную перезагрузку страницы (window.location.reload()). Это нарушает принцип SPA – можно сбросить состояние Vuex и перенаправить пользователя через роутер, избежав полной перезагрузки. Критичность: низкая (стиль, UX-недостаток).

💡 Советы по рефакторингу и улучшению:

  • Добавить в Chat.vue обработку ошибок: try { await this.connection.start(); … } catch (err) { console.error(err); /* оповестить пользователя */ }. Аналогично для invoke.
  • Реализовать отключение SignalR в Chat.vue через beforeDestroy(): if (this.connection) await this.connection.stop();.
  • В TopMenu.vue заменить window.location.reload() на:
    this.$router.push('/');
    this.$store.commit('resetApp'); // или аналогичный сброс
  • Вынести логику работы с SignalR в отдельный сервис (например, src/services/signalr.js) для переиспользования в других компонентах и упрощения тестирования.
  • В ConnectionInfo.vue метод bytesToSize импортирован как CommonUtils.bytesToSize, что хорошо. Рекомендуется убедиться, что эта утилита устойчива к некорректным данным (если stats.data может быть undefined).
  • В TopMenu.vue и Chat.vue методы getInitials вызывают CommonUtils.getInitials(this.$store.state.... Если profile может быть null в момент вызова (например, при гонке), добавить защиту по типу if (!this.$store.state.application.profile) return ''.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/components/room/participant/ControlMenu.vue

Плюсы

  • Улучшено единообразие стиля: исправлены отступы, заменены двойные кавычки на одинарные (соответствует стандартам JavaScript/Vue), добавлены пробелы внутри фигурных скобок шаблонов и после открывающих скобок.
  • Убраны устаревшие вызовы this. внутри методов (Vue автоматически предоставляет контекст).
  • В setTimeout теперь используется стрелочная функция, что сохраняет корректный контекст this (в оригинальном коде также работало, но новый вариант более читаемый).
  • В watch явно указаны параметры newValue, oldValue, что предпочтительнее неявных n, o.

⚠️ Проблемы

  • Опечатка: метод называется kickPatricipant (вместо kickParticipant). Не исправлена в новом коде — это может вызывать путаницу при чтении и не соответствует именам других методов (muteParticipant, muteAllParticipants).
  • Потенциальная бесконечная рекурсия в getPlayer(): если элемент с streamid никогда не появится в DOM, setTimeout будет вызываться каждые 100 мс бесконечно. Желательно добавить ограничение на количество попыток или использовать MutationObserver.
  • Отсутствие обработки ошибок: методы напрямую обращаются к this.connection.connection.peers[this.userId] без проверки, что this.connection и его вложенные свойства существуют. При отсутствии connection или userId возникнет TypeError.
  • Нарушение принципа единой ответственности: компонент напрямую работает с сокетом и DOM (поиск элемента по id). Лучше вынести логику управления потоком (mute/kick) и получения плеера в отдельные сервисы или использовать Vuex/Pinia.

💡 Советы

  • Исправить опечатку: переименовать kickPatricipant в kickParticipant во всех местах.
  • Добавить защиту от бесконечного цикла в getPlayer(): например, let attempts = 0; const maxAttempts = 50; и прерывать поиск после превышения лимита.
  • Вынести имена сокет-команд в константы для избежания опечаток:
    const EVENTS = {
      MUTE: 'mute-participant',
      KICK: 'kick-participant',
    };
  • Использовать watcher для автоматического обновления громкости без ручного поиска DOM: если плеер доступен через $refs или V-models, лучше связать volume напрямую.
  • Проверить серверную логику: метод muteAllParticipants отправляет то же событие, что и muteParticipant, но с полем all: true. Возможно, на сервере ожидается другое событие (например, 'mute-all-participants'). Согласовать с бэкендом.
  • Рассмотреть миграцию на Composition API (если проект на Vue 3) — это улучшит читаемость и тестируемость компонента.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/components/room/participant/ParticipantBlock.vue

Плюсы

  • Код стал чище: убраны избыточные this. в шаблоне, что соответствует стандартам Vue.
  • Удалена устаревшая и сложная логика половинного экрана (halfScreenMode, switchHalfScreen), что упрощает поддержку.
  • Добавлены явные пропсы isSpotlight, isThumbnail, isFullscreen — компонент теперь более предсказуем и переиспользуем.
  • Улучшена обработка краевых случаев: в clearMediaElements() добавлена проверка if (!card) return, что предотвращает ошибки.
  • Стили переписаны с использованием относительных размеров (width: 100%, height: 100%), что делает компонент адаптивным и избавляет от магических чисел.
  • Классы теперь назначаются динамически через объектный синтаксис { 'class-name': condition } — это читаемее.
  • Убраны лишние методы и данные, не используемые в новой архитектуре.

⚠️ Проблемы

  • В пропсах остался неиспользуемый объект DetectRTC — может вводить в заблуждение и требует удаления.
  • Магические числа 400, 1000, 2000 разбросаны по коду без пояснений, что затрудняет понимание временных задержек.
  • В enablePeerStats() вызов this.peerStats.stop() выполняется без проверки на null — хотя рядом есть условие if (this.peerStats), но в текущей реализации this.peerStats может быть null или undefined при первом вызове, и тогда this.peerStats.stop() вызовет ошибку.
  • Метод streamValidate использует this.connection.connection.socket.emit — если сокет по какой-то причине не инициализирован, возникнет исключение.
  • Не очищается streamValidateTimer при изменении streamEvent в watch — если компонент не пересоздаётся, таймер будет множиться (хотя в mounted он устанавливается один раз, а в watch не сбрасывается).
  • В destroyed больше не сбрасывается state.halfScreenMode, но если другие части системы всё ещё полагаются на это свойство, может возникнуть неконсистентное состояние.
  • В prepare дублируется установка muted = true для локального потока (сразу и через 1 секунду).
  • Потенциальная XSS через :src="profile.avatar" — если аватар может содержать javascript: или data: URL, то Vue не экранирует атрибуты, и это может привести к выполнению скриптов.

💡 Советы

  • Удалите пропс DetectRTC, если он больше не используется.
  • Вынесите временные задержки в константы (например, const RENDER_DELAY = 400;) для ясности.
  • Перед вызовом this.peerStats.stop() убедитесь, что this.peerStats не null:
    if (this.peerStats) this.peerStats.stop();
  • Добавьте защиту от отсутствия сокета в streamValidate:
    if (!this.connection?.connection?.socket) return;
  • Сбрасывайте streamValidateTimer при каждом новом streamEvent в watch, а не только в destroyed:
    watch: {
      streamEvent(newValue) {
        clearInterval(this.streamValidateTimer);
        // ... установка нового через setTimeout
      }
    }
  • Если state.halfScreenMode всё ещё нужен глобально, добавьте его сброс в destroyed обратно, иначе удалите из других частей кода.
  • Устраните дублирование установки muted в prepare — достаточно одной строки.
  • Для :src="profile.avatar" используйте санитизацию URL: либо проверяйте на бэкенде, либо применяйте директиву v-bind:src с фильтром (например, только http/https протоколы).
  • Рассмотрите замену setTimeout на this.$nextTick, если задержка нужна только для ожидания рендера DOM.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/components/settings/DevicesTab.vue
  • src/PeerMeeting.Host/ClientApp/src/components/settings/ProfileTab.vue

Анализ изменений кода Vue компонентов

✅ Плюсы

  • Улучшение стиля кода
    • Убраны устаревшие function и лишние фигурные скобки в методах, что соответствует современному синтаксису ES6.
    • Переход на camelCase для методов (saveProfile, saveDevices, fetchDevices), что является общепринятым стандартом для JavaScript.
    • Удалён пустой тег <style> – неиспользуемый код больше не загромождает файл.
  • Упрощение логики
    • Вместо CommonUtils.pushEventToCallbacks используется this.$store.dispatch('notifyInputDeviceChanged') – это более идиоматичный для Vuex подход.
    • В saveDevices копирование объекта через спред-оператор ({ ...this.$store.state... }) предотвращает прямую мутацию состояния store.
    • Убран this.$forceUpdate() – Vuex и Vue реактивно обрабатывают изменения, принудительное обновление не требуется.
  • Добавлена полезная функциональность
    • В ProfileTab.vue теперь сохраняется язык в localStorage при его изменении (через watch $i18n.locale), что улучшает пользовательский опыт.
  • Форматирование
    • Исправлено разбиение атрибутов на несколько строк, код стал более читаемым.

⚠️ Проблемы

  • Порядок dispatch/commit в saveDevices
    • Сначала вызывается this.$store.dispatch('notifyInputDeviceChanged'), а потом this.$store.commit("changeDeviceSettings", ...). Если обработчики события полагаются на уже обновлённые данные, они увидят старые значения. Риск несогласованности состояния.
    • Рекомендация: сначала выполнить commit, а затем dispatch, чтобы уведомление происходило после применения новых настроек.
  • Пропуск сохранения языка при инициализации
    • В ProfileTab.vue watch на $i18n.locale сохраняет язык, но если язык уже задан (например, при загрузке страницы из localStorage), это событие не сработает. Пользователь сможет изменить язык и только тогда он сохранится, но при повторной загрузке без изменений запись не произойдёт. Это не баг, а неполнота логики – обычно язык синхронизируют при монтировании.
  • Дублирование кода
    • В DevicesTab.vue дважды повторяется код фильтрации и маппинга устройств (для аудио и видео).
    • В ProfileTab.vue в mounted и в watch $store.state.application.profile дублируются присваивания username/email.
  • Прямое изменение нереактивного свойства
    • В saveProfile выполняется this.$themeHelper.theme = this.currentTheme. Если $themeHelper – это обычный объект, Vue не отследит это изменение, и UI не обновится. Хотя store changeTheme уже позаботится о реактивности, эта строка избыточна и может ввести в заблуждение.

💡 Советы

  • Избегайте require внутри data
    • В ProfileTab.vue md5: require('md5') выполняется при каждом создании компонента. Лучше импортировать модуль на уровне скрипта: import md5 from 'md5'.
  • Используйте computed или mapState для повторяющихся выражений
    • Например, в DevicesTab.vue можно создать вычисляемое свойство availableAudioDevices и availableVideoDevices, чтобы не дублировать фильтрацию в fetchDevices.
  • Выделите общую логику в методы
    • Для ProfileTab.vue вынесите заполнение полей из mounted и watch в отдельный метод loadProfileFromStore(), чтобы избежать дублирования.
  • Рассмотрите миграцию на Vue 3 Composition API
    • Если проект новый или планируется обновление, использование setup и reactive упростит отслеживание зависимостей и уменьшит количество обращений к this.$store.state.
  • Проверьте, зарегистрирован ли action notifyInputDeviceChanged
    • Иначе вызов dispatch вызовет ошибку в консоли. В идеале использовать строгие типы для action (константы или символы).
  • Обратите внимание на хэширование
    • Хотя md5 безопасен для Gravatar, для более надёжного хэширования (например, если email будет использоваться как идентификатор) лучше использовать sha256 или bcrypt. Но это не критично для данной функциональности.

Итог: код стал чище и современнее, но содержит потенциальную проблему с порядком обновления состояния и дублирование, которое стоит устранить для улучшения поддерживаемости.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/main.js

Плюсы:

  • Код стал чище: удалены избыточные комментарии, заменены var на const/let, устранён console.log(language).
  • Логика восстановления состояния вынесена из main.js в хранилище (store.dispatch('restoreState')), что улучшает модульность и переиспользуемость.
  • Исправлены мелкие стилистические недочёты: единообразные кавычки, убраны лишние точки с запятой в стрелочных функциях.
  • Убран потенциально опасный код с ручным парсингом localStorage и множественными commit — теперь это скрыто в сторе.

⚠️ Проблемы:

  1. Отсутствует определение language
    В коде используется объект language (строки language['en'], language['ru']), но в видимом фрагменте нет его импорта или объявления. Если language не определён глобально или не импортирован ранее, возникнет ошибка ReferenceError.
  2. Потенциальная потеря логики определения темы
    Раньше, если тема не была сохранена в localStorage, применялась медиа-функция matchMedia. Теперь эта логика отсутствует — themeHelper.init() может не содержать аналогичного fallback. Если themeHelper.init() не читает localStorage и не проверяет prefers-color-scheme, тема может остаться неинициализированной (значение undefined).
  3. Порядок инициализации Sentry
    sentryIntegrationTask выполняется параллельно с themeHelperInitTask, но при этом в процессе инициализации Sentry используется router (передан в routingInstrumentation). Если роутер ещё не полностью готов, это может вызвать некорректную трассировку, хотя обычно router уже создан к этому моменту.

💡 Советы:

  • Добавьте импорт language (например, из ../i18n/language.js) или убедитесь, что он объявлен в глобальной области видимости. Лучше явно импортировать.
  • Проверьте реализацию store.dispatch('restoreState') — она должна полностью заменять удалённый блок (восстановление профиля, настроек устройства, истории комнат, turnOnly). Если какой-то из пунктов упущен — добавьте.
  • Убедитесь, что themeHelper.init() обрабатывает случай отсутствия localStorage['theme']: читает matchMedia('(prefers-color-scheme: dark)') и устанавливает тему по умолчанию.
  • Рассмотрите возможность вынести настройку i18n в отдельный модуль (например, i18n/index.js) для уменьшения размера main.js.
  • Замените navigator.getUserMedia = ... на стандартный вызов navigator.mediaDevices.getUserMedia (если поддерживается) — код устарел и может не работать в современных браузерах.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/services/PeerMeetingRtcMulticonnection.js

Плюсы

  • Улучшено форматирование: единый стиль отступов (2 пробела), одинарные кавычки, удалены лишние точки с запятой – код стал чище и соответствует современным практикам JS.
  • Заменены var self = this на стрелочные функции – исправлена потеря контекста, код стал более читаемым и предсказуемым.
  • Убраны избыточные префиксы _ в названиях методов (например, _generateUserId → generateUserId) – это улучшает инкапсуляцию и семантику.
  • Консистентное использование const и let (вместо var) в объявлении переменных внутри методов.
  • Добавлено явное завершение потоков при setOnStreamEnded – улучшена очистка DOM-элементов после завершения стрима.
  • Исправлена логика в configureMediaError: параметр callback заменён на onMediaStateChange – более понятное имя и соответствие сигнатуре вызова.

⚠️ Проблемы

  • Потенциальный баг в startParticipantFixTimer: обращение к this.connection.peers.getAllParticipants() – метод getAllParticipants() обычно вызывается напрямую у connection, а не у connection.peers. Это может привести к ошибке выполнения, так как peers – объект, а не массив с методом getAllParticipants.
  • Отсутствие проверки существования peers[e] в else-блоке цикла forEach: когда участник уже есть в participants, выполняется ветка else, где идёт обращение this.connection.peers[e].peer.participantCardError = 0. Если this.connection.peers[e] не существует (например, участник отключился), произойдёт TypeError.
  • Вызов originalOnUnmute(e) без проверки на undefined: если this.connection.onunmute не был определён до переопределения, originalOnUnmute будет undefined, и вызов вызовет ошибку.
  • Использование магических чисел (5, 6, 2000, 5000) без вынесения в константы – ухудшает читаемость и усложняет поддержку.
  • Синхронный require('detectrtc') в начале класса – в среде Vue/Webpack предпочтительнее import. Хотя это работает, нарушает единообразие импортов в файле.
  • Отсутствие точек с запятой в конце строк – может привести к проблемам при минификации или склейке файлов, если проект не использует инструменты, защищающие от ASI (Automatic Semicolon Insertion).
  • Несоответствие сигнатуры configureMediaError: в старом коде был callback, теперь onMediaStateChange. Если вызов этого метода в других частях кода не обновлён, возникнет ошибка.

💡 Советы

  • Исправить вызов getAllParticipants(): заменить this.connection.peers.getAllParticipants() на this.connection.getAllParticipants().
  • Добавить защитную проверку перед доступом к peers[e] в else-блоке:
    } else if (this.connection.peers[e]) {
        this.connection.peers[e].peer.participantCardError = 0;
    }
  • Проверить, что originalOnUnmute определена, иначе не вызывать:
    const originalOnUnmute = this.connection.onunmute;
    this.connection.onunmute = (e) => {
        if (originalOnUnmute) originalOnUnmute(e);
        // ...
    };
  • Вынести магические числа в константы класса:
    static PARTICIPANTS_CHECK_INTERVAL = 5000;
    static CARD_RETRY_LIMIT = 6;
    static CARD_CHECK_INTERVAL = 2000;
  • Заменить require('detectrtc') на import detectRTC from 'detectrtc' для единообразия.
  • Добавить точки с запятой в конце инструкций – это стандарт для корпоративных проектов и снижает риск ошибок.
  • Проверить все места вызова configureMediaError – убедиться, что передаётся аргумент onMediaStateChange, а не callback.
  • Рассмотреть возможность использования IntersectionObserver или MutationObserver вместо интервалов для отслеживания карточек участников – это снизит нагрузку и улучшит производительность.
  • Добавить обработку ошибок в методах, работающих с DOM (например, removeChild может выбросить исключение, если элемент уже удалён).

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/services/PeerStats.js

✅ Плюсы

  • Чистый современный синтаксис: использование const, стрелочных функций, ранний return в цикле forEach для улучшения читаемости.
  • Убран /* eslint-disable */ — теперь код проверяется линтером, что повышает качество.
  • Консистентность в кавычках: одинарные кавычки по всему файлу.
  • Удалены лишние точки с запятой — стиль соответствует современному JS (ASI).
  • Использование Math.max для защиты от деления на ноль в расчёте perSecond.
  • Явное присвоение this (переименование PeerConnection в peerConnection, self в this) — код стал более понятным.

⚠️ Проблемы

  1. Выброс строки вместо Error:
    throw 'Callback cannot be null' — плохая практика. Следует использовать throw new Error('Callback cannot be null') для корректной обработки стека вызовов и совместимости с инструментами отладки.
  2. Потенциальное деление на ноль:
    В строке (candidatePair.totalRoundTripTime / candidatePair.responsesReceived).toFixed(5) — если responsesReceived равно 0, результат будет Infinity или NaN. Нужно добавить проверку.
    this.stats.rtt = this.isFirefox ? 0 : candidatePair.responsesReceived 
      ? (candidatePair.totalRoundTripTime / candidatePair.responsesReceived).toFixed(5) 
      : 0;
  3. Неочевидное поведение цикла forEach:
    Если в отчёте несколько объектов типа transport или candidate-pair, каждый из них перезапишет поля stats. Возможно, это ожидаемо, но стоит прокомментировать или пересмотреть логику (например, обработать только первый подходящий элемент).
  4. Утечка таймера при повторном вызове start:
    Если start вызвать дважды без stop, старый timer не очищается. Лучше добавить проверку:
    start(callback, interval) {
      if (this.timer) this.stop();
      // ... остальной код
    }
  5. Не обнуляется this.timer после clearInterval:
    В методе stop после очистки стоит присвоить this.timer = null, чтобы избежать повторного вызова clearInterval с некорректным идентификатором.

💡 Советы

  1. Типизация и проверки:
    • В start заменить if (!callback) на if (typeof callback !== 'function') для точности.
    • Для interval задать значение по умолчанию через параметр (interval = 5000), что уже сделано, хорошо.
  2. Рефакторинг parseReport:
    • Вынести вычисление RTT, IP, STUN/TURN в отдельные методы или использовать деструктуризацию:
      const { bytesSent, bytesReceived, packetsSent, packetsReceived, timestamp } = r;
    • Повторяющиеся операции (поиск relay, преобразование IP) логично вынести во вспомогательные функции.
  3. Избегать магических чисел:
    Вынести 5000 (интервал по умолчанию) в константу класса, например static DEFAULT_INTERVAL = 5000.
  4. Использовать includes вместо indexOf:
    candidateType.includes('relay') более читаемо.
  5. Логирование ошибок:
    В catch блока startесть console.error(e) — достаточно, но можно добавить более контекстное сообщение, например console.error('PeerStats parsing failed', e).
  6. Обработка candidatePair для Firefox:
    В Firefox candidatePair.totalRoundTripTime и responsesReceived не заданы (код устанавливает rtt = 0). Можно было бы попробовать получить эти данные из другого источника, если они необходимы.
  7. Документация:
    Добавить JSDoc-комментарии для публичных методов start, stop, parseReport — это улучшит понимание кода другими разработчиками.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/services/RTCStateService.js

Плюсы

  • Использование современного синтаксиса ES6+ (стрелочные функции, async/await, шаблонные строки), что улучшает читаемость и сокращает шаблонный код.
  • Устранение устаревшего шаблона var self = this : контекст теперь правильно захватывается через стрелочные функции (конструктор, циклы forEach).
  • Чёткое разделение ответственности: класс управляет только состоянием RTC, не смешивая логику UI.
  • Асинхронный метод screenSharing теперь использует async/await вместо вложенных колбэков, что делает поток управления более линейным и понятным.
  • Добавлен вызов onStreamAdded для обратной связи при смене источника видео (экран/камера).

⚠️ Проблемы

  1. Устаревшее API getUserMedia – в методе screenSharing используется navigator.getUserMedia (legacy). В современных браузерах это может привести к ошибке "getUserMedia is not a function". Рекомендуется использовать navigator.mediaDevices.getUserMedia с промисами.
  2. Избыточная обработка ошибок при получении микрофона – после успешного получения экранного потока и неудачного получения микрофона блок catch только логирует ошибку, но затем всё равно вызывается RTCUtils.addStream (без микрофонной дорожки). Это может быть намеренно, но отсутствие явного return или throw может запутать. Лучше вынести вызов addStream после проверки успешности получения микрофона.
  3. Потенциальная потеря контекста в stateCheck – хотя в конструкторе используется стрелочная функция, внутри stateCheck используется this, что корректно, так как метод вызывается как this.stateCheck(). Ошибки нет, но метод не проверяет, что rtcConnection.connection всё ещё существует (если соединение было разрушено, attachStreams.forEach упадёт).
  4. Прямое изменение extra объектаthis.rtcConnection.connection.extra.audioMuted = ... предполагает, что свойство extra уже существует и мутирует глобальное состояние. Лучше инкапсулировать эту логику в отдельный метод updateExtraData.

💡 Советы

  1. Рефакторинг дублирования в switchAudioMute / switchVideoMute – вынести общую логику в приватный метод:
    _toggleTrack(type, state) {
        const enabled = state === null ? !this.state[`${type}Enabled`] : state;
        this.state[`${type}Enabled`] = enabled;
        this.rtcConnection.connection.attachStreams.forEach(s => {
            enabled ? s.unmute(type) : s.mute(type);
        });
        this.rtcConnection.connection.extra[`${type}Muted`] = !enabled;
        this.rtcConnection.connection.updateExtraData();
    }
  2. Унифицировать получение медиапотоков – заменить navigator.getUserMedia на navigator.mediaDevices.getUserMedia, а также создать вспомогательную функцию для безопасного вызова.
  3. Оптимизировать stateCheck – добавить флаг _streamsDirty, который выставляется при изменении состояния (например, в switchAudioMute), и проверять его в stateCheck, чтобы не перебирать треки каждую секунду.
  4. Добавить метод очистки – если класс используется в компоненте, который может быть уничтожен, нужно предусмотреть destroy() для очистки setInterval и подписок.
  5. Обработка ошибок при отсутствии rtcConnection – в методах, обращающихся к this.rtcConnection.connection, стоит добавить проверку существования объекта, чтобы избежать TypeError.
  6. Использовать const для неизменяемых переменных – в коде есть mPeer, audioDevice и другие, которые не переприсваиваются. Использование let допустимо, но const даёт явную гарантию неизменности.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/services/store.js

✅ Плюсы

  • Вынос работы с localStorage в отдельный объект storage повышает читаемость и упрощает возможное переключение на другой механизм хранения.
  • Добавление геттеров (особенно sortedRoomHistory) и action restoreState делает логику восстановления состояния централизованной.
  • Упрощение среза в addRoomToHistory и замена var на const — улучшение стиля и читаемости.
  • Добавлена мутация clearDeviceChangedCallbacks, ранее отсутствовавшая — это устраняет потенциальную утечку памяти.
  • Убранные комментарии // eslint-disable-next-line и лишние параметры в функциях делают код чище.
  • Трейлинг-запятые соответствуют современным стандартам ES.

⚠️ Проблемы

  1. Несовместимость с предыдущими данными localStorage
    Раньше в changeTheme строка сохранялась без JSON.stringify, теперь через storage.set она попадает в хранилище в виде "\"dark\"" (с кавычками). При чтении через storage.get (который делает JSON.parse) старые строки без кавычек вызовут исключение, и восстановление состояния сломается. Та же проблема возможна с turnOnly (булевы значения) и roomHistory (если массив хранился без JSON-сериализации, хотя раньше в коде уже был JSON.stringify — в этом случае проблем нет, но updateRoomHistory использовал JSON.stringify и раньше).

  2. Избыточное сохранение при восстановлении roomHistory
    В restoreState каждый элемент истории добавляется через commit('addRoomToHistory'), что приводит к повторному сохранению всего массива в localStorage после каждого push. Это неоптимально и может вызвать лишние записи; к тому же addRoomToHistory содержит проверку длины и срез (а уже сохранённый массив и так не длиннее 10).

  3. Потенциальная потеря данных deviceSettings при восстановлении
    commit('changeDeviceSettings', deviceSettings) внутри restoreState снова вызывает storage.set, перезаписывая те же данные — логически это корректно, но избыточно.

  4. Отсутствие обработки ошибок в storage.get
    Если в localStorage лежит некорректный JSON (например, повреждённый вручную), JSON.parse бросит исключение, которое нигде не ловится. В restoreState это приведёт к остановке инициализации.

💡 Советы

  • Добавить миграцию или fallback при чтении из localStorage:
    Обернуть JSON.parse в try-catch и, если значение не распарсилось, использовать его как строку напрямую (для полей, которые раньше хранились без сериализации, например, theme). Либо при первом запуске новой версии удалить старые ключи и пересоздать их через новую логику.
  • Оптимизировать восстановление roomHistory:
    Вместо цикла с commit установить значение напрямую:
    state.application.roomHistory = roomHistory;
    storage.set(STORAGE_KEYS.roomHistory, roomHistory);
    Аналогично для deviceSettings и profile — можно назначать напрямую в состоянии (без мутаций, которые триггерят повторное сохранение), либо оставить мутации, но убрать из них вызов storage.set, когда состояние восстанавливается. Альтернатива — использовать одно действие restoreState для установки всех полей в состоянии, а сохранение выполнять только один раз.
  • Избежать двойного сохранения turnOnly и theme — в restoreState после commit также будет storage.set, что избыточно. Можно сначала считать данные, затем вызвать одну мутацию, устанавливающую все поля из localStorage без сохранения.
  • Добавить обработку ошибок в storage.get:
    get(key) {
      try {
        return JSON.parse(window.localStorage[key]);
      } catch (e) {
        // Возможно, вернуть значение как есть, или удалить ключ
        return window.localStorage[key] || null;
      }
    }
  • Рассмотреть использование computed свойств вместо геттеров Vuex для простых производных данных (например, sortedRoomHistory можно реализовать как computed в компоненте), но это опционально.
  • Убрать лишние комментарии (уже сделано) и стандартизировать форматирование (пробелы в скобках — соблюдены).

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/ClientApp/src/views/Room.vue

✅ Плюсы

  • Отказ от $forceUpdate: Замена на реактивную переменную participantVersion (счётчик версий) — правильное и производительное решение для обновления computed-свойств, зависящих от Map.
  • Чистота кода: Убраны лишние eslint-disable и дублирующиеся this., импорты оформлены единообразно. Код стал легче читать и поддерживать.
  • CSS Grid: Вёрстка переведена с b-card-group на CSS Grid — более гибкая, адаптивная и предсказуемая раскладка.
  • Новые режимы отображения: Полноэкранный и spotlight-режимы улучшают пользовательский опыт в конференциях.
  • Обработка ухода участников: В streamEnded сбрасывается selectedParticipantId, что предотвращает ссылки на удалённых пользователей.
  • Очистка ресурсов: В beforeDestroy добавлен вызов clearDeviceChangedCallbacks.

⚠️ Проблемы

  1. Не сбрасывается fullscreenParticipantId при уходе участника
    В streamEnded очищается только selectedParticipantId, а fullscreenParticipantId остаётся. Если участник, который был на весь экран, покинет комнату, приложение может пытаться отобразить null или уже несуществующего участника.
    Исправление: Добавить аналогичную проверку для fullscreenParticipantId.

  2. Отсутствие сброса полноэкранного режима при уменьшении числа участников
    watch на participantCount сбрасывает spotlight, если участников ≤ 2, но не затрагивает fullscreen. Если зритель вышел, а полноэкранный режим остался, интерфейс может выглядеть некорректно.
    Исправление: Дополнить условие сбросом fullscreenParticipantId.

  3. Неявная зависимость от participantVersion в computed
    Трюк с void this.participantVersion работает, но неочевиден для новых разработчиков и может сломаться при обновлении Vue, если изменится механизм отслеживания. Лучше использовать this.participantVersion как свойство, от которого зависят вычисления, или сразу возвращать счётчик.
    Исправление: Например, заменить на return this.participantVersion && this.participants.size (но аккуратно с 0).

  4. Потенциальная утечка памяти при addBaseStream
    Раньше addBaseStream не возвращал Promise, теперь — then(...). Если функция не реализует асинхронность, addParticipantBlock вызовется синхронно, но если внутри есть await, а then не обрабатывает ошибки — возможны неожиданные вызовы.
    Совет: Добавить .catch или использовать async/await с try-catch в inputDeviceChanged.

💡 Советы

  • Выделить состояние UI в отдельный модуль. fullscreenParticipantId, selectedParticipantId и логика их переключения разрастаются. Перенести их в Vuex (или composition API в будущем) упростит тестирование и уменьшит связность Room.vue.
  • Заменить require на import в скрипте (кроме adapterjs, который нужен для side-effect). import detectRTC from 'detectrtc' более идиоматично.
  • Использовать кастомные события для дочерних компонентов — уже хорошо реализовано: @toggleSpotlight, @toggleFullscreen. Для единообразия можно переименовать clearSpotlight в @clearSpotlight и сделать обработчик общим.
  • Добавить обработку граничных случаев для selectedParticipant: если участник с selectedParticipantId покинет комнату, сейчас сброс есть, но если покинет после того, как fullscreenParticipantId установлен, и одновременно сработает payticipantCount — логика может запутаться. Рекомендую централизовать все сбросы в одном методе (например, cleanUpParticipant(id)).
  • Проверить работу addBaseStream — если она возвращает Promise, в inputDeviceChanged лучше использовать async/await:
    async inputDeviceChanged() {
      // ...
      const event = await RTCUtils.addBaseStream(...);
      this.addParticipantBlock(event);
    }
  • Обновить ESLint-конфигурацию — код стал строже, но убрано /* eslint-disable */. Убедиться, что проект использует правила, совместимые с новым стилем (точка с запятой, кавычки).

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/HealthChecks/CoturnHealthCheck.cs
  • src/PeerMeeting.Host/HealthChecks/RedisHealthCheck.cs
  • src/PeerMeeting.Host/ClientApp/src/views/Welcome.vue
  • src/PeerMeeting.Host/PeerMeeting.Host.csproj

✅ Плюсы

  • В Welcome.vue переход от метода getHistory() к вычисляемому свойству sortedHistory — это правильное реактивное решение, повышающее производительность и читаемость.
  • В Welcome.vue удалён пустой блок components и улучшен синтаксис методов (стрелочные функции, устранение лишних return) — код стал аккуратнее.
  • Добавлены HealthChecks для Coturn и Redis — это улучшает наблюдаемость и позволяет мониторить состояние критических зависимостей.
  • В CoturnHealthCheck реализован таймаут соединения (5 секунд) — защита от зависания health check.
  • В .csproj обновлены версии зависимостей (Serilog, prometheus-net, VueCliMiddleware) — это включает исправления и новые возможности.
  • Добавлен пакет Microsoft.Extensions.Diagnostics.HealthChecks — явное указание зависимости делает проект самодостаточным и упрощает будущее обновление.

⚠️ Проблемы

  • TargetFramework net10.0 в .csproj — такой версии .NET на данный момент не существует (по состоянию на 2025 год стабильные — .NET 8/9). Сборка проекта с этим TFM невозможна; скорее всего опечатка, должно быть net9.0.
  • В CoturnHealthCheck парсинг адреса через Split(':') сломается для IPv6 (например, [::1]:3478). Требуется учитывать наличие скобок или использовать System.Uri.TryCreate.
  • В CoturnHealthCheck нет отмены задачи connectTask при таймауте: после Task.WhenAny поток не ожидает connectTask, но она может висеть и потреблять ресурсы (хотя using client вызовет Dispose, но сам TcpClient может не освободить сокет корректно). Лучше передавать CancellationToken с таймаутом в ConnectAsync.
  • В RedisHealthCheck нет таймаута подключения: ConnectionMultiplexer.ConnectAsync по умолчанию может ждать очень долго. Необходимо указать ConnectTimeout в ConfigurationOptions, иначе health check может повиснуть.
  • В RedisHealthCheck каждый раз создаётся новое подключение. Для health check это приемлемо, но при частых запросах может возникнуть перегрузка. Лучше использовать кэшированный мультиплексор с проверкой состояния.
  • В .csproj используются превью-версии (StackExchange.Redis 3.0.2-preview, Sentry 14.12.1-dump1). В production это нестабильно; стоит перейти на финальные стабильные версии.

💡 Советы

  • В Welcome.vue заменить window.location.href = '/' + id на использование Vue Router (router.push) — это обеспечит SPA-навигацию, сохранит состояние приложения и позволит избежать возможных XSS-инъекций (если id будет содержать небезопасные символы, а сейчас он состоит из UUID, но лучше застраховаться).

  • В CoturnHealthCheck вынести парсинг адреса в отдельный метод, поддерживающий IPv6, например: System.Net.IPEndPoint.TryParse() или регулярное выражение.

  • В CoturnHealthCheck использовать CancellationTokenSource с таймаутом для отмены ConnectAsync:

    using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
    try
    {
        await client.ConnectAsync(host, port, cts.Token);
        return client.Connected ? HealthCheckResult.Healthy(...) : HealthCheckResult.Degraded(...);
    }
    catch (OperationCanceledException)
    {
        return HealthCheckResult.Degraded("Coturn connection timed out");
    }
  • В RedisHealthCheck аналогично добавить таймаут:

    var config = new ConfigurationOptions
    {
        EndPoints.Add(_connectionString),
        ConnectTimeout = 5000
    };
    await using var connection = await ConnectionMultiplexer.ConnectAsync(config);
  • В .csproj исправить TargetFramework на актуальную версию, например net9.0 (или net8.0, если необходимо LTS).

  • В .csproj заменить превью-пакеты на стабильные версии, например StackExchange.Redis на 2.8.22 (стабильная), Sentry.AspNetCore на последнюю стабильную (например 4.12.1 — уточнить по дате).

  • Во всех файлах C# добавить закрывающий перевод строки (EOF) — в CoturnHealthCheck и RedisHealthCheck отсутствует символ новой строки в конце. Это рекомендуется для совместимости с инструментами и стандартами.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/Program.cs

Плюсы:

  • Использование современного подхода с WebApplication (minimal hosting model) вместо классического Startup — код стал короче и понятнее.
  • Настройка SignalR с DetailedErrors и ограничением размера сообщения — хорошая практика.
  • Добавлена поддержка Redis для бэкплейна SignalR и DataProtection — это улучшает масштабирование и безопасность (ключи DataProtection сохраняются централизованно).
  • Внедрение middleware для компрессии ответов и Csrf-защиты — повышает производительность и безопасность.
  • Использование Serilog и обработка глобальных исключений (UnhandledException, UnobservedTaskException) — улучшает наблюдаемость.
  • Настройка Swagger/OpenAPI? (нет, но можно добавить) — отсутствие этого не минус, а контекст не указан.

⚠️ Проблемы (потенциальные баги и уязвимости):

  1. Двойной вызов AddSignalR() при включенном Redis: сначала с параметрами, затем AddSignalR().AddStackExchangeRedis(...). Второй вызов может перезаписать или сбросить настройки из первого (например, EnableDetailedErrors, MaximumReceiveMessageSize). Баг: настройки могут быть потеряны.
  2. Регистрация хабов SignalR как Singleton: builder.Services.AddSingleton<WebRtcHub>() и ChatHub — это грубая ошибка. SignalR создаёт отдельный экземпляр хаба для каждого соединения/вызова. Singleton приведёт к состоянию гонки, потере данных и невозможности работы с несколькими пользователями. Серьёзный баг.
  3. EnableDetailedErrors = true без условия: в production-среде детальные ошибки могут раскрыть внутреннюю информацию (стек трейсы, подробности исключений). Рекомендуется включать только в Development.
  4. Отсутствие UseAntiforgery() в pipeline: хотя AddAntiforgery выполнен, middleware UseAntiforgery не вызывается. Кастомный UseCsrf() может не работать корректно без вызова встроенного антифоржери. Потенциальная уязвимость: CSRF-защита может быть неактивна.
  5. Использование устаревшего AddMvc(): рекомендуется AddControllers() или AddControllersWithViews(). AddMvc() добавляет ненужные компоненты (Razor Pages, View Engines) и может замедлить приложение.
  6. Health checks не мапятся в endpoints: зарегистрированы RedisHealthCheck и CoturnHealthCheck, но нет вызова app.UseHealthChecks(...). Эндпоинты /health не будут доступны — функциональность мониторинга работает только на регистрации, но не обслуживается.
  7. TransportMaxBufferSize = 0: безлимитный буфер может привести к исчерпанию памяти под нагрузкой. Лучше установить разумный лимит (например, 64 КБ).
  8. Аутентификация и авторизация вызываются, но не настроены: UseAuthentication() и UseAuthorization() в pipeline присутствуют, но нет AddAuthentication(). Это приведёт к тому, что все запросы будут обрабатываться без проверки прав. Уязвимость: эндпоинты могут быть доступны без аутентификации.
  9. Настройка Redis DataProtection без указания пароля/SSL: если Redis доступен извне или в одной сети без шифрования, ключи могут быть перехвачены. Это угроза безопасности.
  10. Отсутствие обработки HTTPS-редиректа: не используется UseHttpsRedirection(), что может быть проблемой, если приложение ожидает HTTPS.

💡 Советы (возможности рефакторинга):

  • Исправить регистрацию хабов: удалить AddSingleton<WebRtcHub>() — SignalR сам регистрирует хабы как transient. Если нужна дополнительная зависимость, использовать фабрику или Scoped.
  • Объединить вызовы AddSignalR: настраивать Redis внутри первого вызова: builder.Services.AddSignalR(o => { ... }).AddStackExchangeRedis(...); или использовать условную логику без дублирования.
  • Вынести конфигурацию SignalR и Redis в отдельный extension-метод для читаемости.
  • Заменить AddMvc() на AddControllers() (или AddControllersWithViews(), если используется Vue и рендеринг на сервере — но в коде есть только WebApi и SPA).
  • Добавить условное включение детальных ошибок: o.EnableDetailedErrors = app.Environment.IsDevelopment(); (после сборки app).
  • Добавить app.UseAntiforgery() после UseRouting() (или объединить с кастомным UseCsrf).
  • Настроить Health Checks через MapHealthChecks: app.MapHealthChecks("/health");.
  • Установить лимит для TransportMaxBufferSize (например, 0 для разработки, 65536 для прода).
  • Добавить аутентификацию: builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme).AddJwtBearer(...); с реальной проверкой токенов.
  • Использовать Options Pattern для Redis и Coturn конфигураций вместо вызова GetRedisConfiguration() напрямую.
  • Добавить app.UseHttpsRedirection() и app.UseForwardedHeaders() для корректной работы за прокси.
  • Перенести обработчики глобальных исключений из UseSerilog в отдельную конфигурацию логгирования или в middleware для большей гибкости.
  • Убрать AddHostedService<MetricsService> — обычно метрики собираются через middleware, а не фоновый сервис, если он не ведёт периодический сбор.

Резюме: код написан в современном стиле, но содержит несколько критичных багов (регистрация хабов, двойной AddSignalR, отсутствие Antiforgery) и уязвимости (детальные ошибки, не настроенная аутентификация). Требуется доработка перед релизом.

@AMEST
Copy link
Copy Markdown
Owner Author

AMEST commented May 8, 2026

🤖 AI Code Review:

Files:

  • src/PeerMeeting.Host/Startup.cs

Вот анализ кода из удалённого файла Startup.cs. Факт удаления не комментирую — оцениваю только содержимое.


✅ Плюсы

  • Использование Prometheus для метрик с добавлением статических меток (окружение, хост, приложение) — хорошая практика наблюдения.
  • Настройка SignalR с поддержкой Redis backplane для масштабирования и Data Protection поверх Redis для работы в ферме серверов.
  • Включена компрессия ответов (Brotli и Gzip) даже для HTTPS.
  • Антифоржери-защита с кастомным заголовком X-CSRF-TOKEN.
  • Корректный порядок middleware: Routing → Metrics → Csrf → StaticFiles → Authentication → Authorization → Endpoints.
  • Ограничение размера сообщений SignalR (256 КБ) и явное разрешение только WebSocket транспорта.
  • Использование AddSpaStaticFiles и прокси для Vue в разработке — удобно для SPA.

⚠️ Проблемы

  1. Регистрация хабов как синглтонов
    services.AddSingleton<WebRtcHub>() и services.AddSingleton<ChatHub>()это баг. Хабы SignalR должны быть transient или scoped, так как каждый новый сеанс требует новый экземпляр. Если использовать синглтон, все соединения будут разделять одно состояние, что приведёт к гонке данных и неверной работе (например, один клиент может получить данные другого).

  2. Избыточность AddMvc
    Вызов services.AddMvc() вместе с services.AddControllers() — в ASP.NET Core 3+ AddMvc добавляет все компоненты (Views, Pages и т.д.), а AddControllers — только для API. Если не используются Views, следует оставить только AddControllers. Это может замедлить старт приложения и потреблять лишнюю память.

  3. Глобальные статические метки Prometheus
    Metrics.DefaultRegistry.SetStaticLabels изменяет глобальный реестр. Если в одном процессе запущено несколько приложений (например, в тестах), это может привести к конфликту меток. Лучше передавать метки через конфигурацию сборщика метрик.

  4. Отсутствие явной настройки безопасности заголовков
    Нет заголовков X-Content-Type-Options: nosniff, X-Frame-Options: DENY/SAMEORIGIN, Content-Security-Policy, Strict-Transport-Security. Хотя SuppressXFrameOptionsHeader = false включён, сам заголовок не задаётся — это может быть уязвимостью для clickjacking и других атак.

  5. Конфигурация через синглтоны-значения
    services.AddSingleton(Configuration.GetMetricsConfiguration()) — если конфигурация может меняться в рантайме (например, из базы данных), синглтон зафиксирует только начальное значение. Лучше использовать IOptions<T> или IOptionsSnapshot<T>.

  6. Потенциальная проблема с UseAuthentication после UseCsrf
    UseCsrf — кастомный middleware, который, скорее всего, проверяет CSRF-токен. Если он вызывается до UseAuthentication, то может не иметь доступа к identity пользователя, что может сломать логику проверки (например, если токен привязан к сессии). Порядок должен быть: Routing → Authentication → Csrf → Authorization.


💡 Советы

  • Исправить регистрацию хабов на transient:
    services.AddSignalR().AddHubOptions<WebRtcHub>(...) — хабы регистрируются автоматически, не нужно добавлять их вручную в DI как сервисы. Удалить строки AddSingleton<WebRtcHub>() и AddSingleton<ChatHub>().

  • Убрать AddMvc(), оставив только AddControllers() (если не используются Razor Pages или Views).

  • Вынести настройку SignalR и Redis в отдельные методы расширения для читаемости.

  • Добавить middleware безопасности в начало пайплайна:

    app.UseHsts();
    app.UseHttpsRedirection();
    app.UseXXssProtection(...);
    app.UseXContentTypeOptions();
    app.UseXFrameOptions(...);
    app.UseCsp(...);
  • Для конфигураций использовать IOptions<T> или Bind, чтобы избежать синглтонов с неизменяемыми объектами.

  • Рассмотреть использование IPrometheusCollector вместо глобальных статических меток — через Prometheus.Metrics.WithLabels.

  • Разделить метод Configure на несколько helper-методов (например, ConfigureMetrics, ConfigureSignalR, ConfigureSecurity), чтобы улучшить поддерживаемость.

@AMEST AMEST merged commit ff27fee into master May 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant