Repository navigation
Conversation
| scheduledDate: { | ||
| lte: dayjs().toISOString(), | ||
| }, | ||
| OR: [ |
There was a problem hiding this comment.
🔴 Ветвь OR в deleteMany не ограничена method: WorkflowMethods.SMS — удаляются напоминания любых типов
Вторая ветка OR-условия { retryCount: { gt: 1 } } в deleteMany не содержит фильтра по method: WorkflowMethods.SMS. Поскольку ветви OR независимы, эта ветка совпадает с любым workflowReminder (email, push и т.д.), у которого retryCount > 1, независимо от метода и даты. Это приводит к тихой потере данных не-SMS напоминаний при каждом вызове эндпоинта.
| scheduledDate: { | ||
| lte: dayjs().toISOString(), | ||
| }, | ||
| OR: [ |
There was a problem hiding this comment.
🟠 Удаление по retryCount игнорирует scheduledDate — удаляются запланированные на будущее напоминания
Ветвь { retryCount: { gt: 1 } } удаляет напоминания только по количеству повторов без ограничения по scheduledDate. Напоминание, запланированное на будущую дату, но достигшее порога повторов, будет удалено до наступления срока. В сочетании с отсутствием фильтра по method это расширяет радиус поражения.
| } catch (error) { | ||
| await prisma.workflowReminder.update({ | ||
| where: { | ||
| id: reminder.id, |
There was a problem hiding this comment.
🟡 Обновление в catch-блоке может выбросить исключение и скрыть исходную ошибку
В catch-блоке выполняется await prisma.workflowReminder.update(...) без собственного try-catch. Если обновление не удастся (запись удалена, проблема с БД), выброшенная ошибка заменит исходную ошибку планирования, а последующий console.log с исходной ошибкой никогда не выполнится. Клиент получит необработанную 500 без полезной диагностики.
| })) as (PartialWorkflowReminder & { retryCount: number })[]; | ||
|
|
||
| if (!unscheduledReminders.length) { | ||
| res.json({ ok: true }); |
There was a problem hiding this comment.
🟡 Ошибка обновления в else-ветке перехватывается внешним catch и вызывает повторное обновление тем же значением
Если prisma.workflowReminder.update в else-ветке выбрасывает исключение, внешний catch перехватывает его и пытается выполнить то же обновление с тем же reminder.retryCount + 1. Двойного инкремента не происходит (базовое значение то же), но тратится лишний запрос к БД, а реальная причина сбоя в else-ветке маскируется. Catch-блок не различает ошибку планирования и ошибку обновления.
| }, | ||
| }); | ||
| } else { | ||
| await prisma.workflowReminder.update({ |
There was a problem hiding this comment.
🔵 Дублирование логики обновления retryCount в else и catch-ветках
Блок prisma.workflowReminder.update({ where: { id: reminder.id }, data: { retryCount: reminder.retryCount + 1 } }) дословно повторяется в else-ветке и в catch-блоке. Дублирование повышает риск рассинхронизации при изменении логики и увеличивает нагрузку на поддержку.
🔍 Автоматическое ревью кодаКраткий итогСинтез ревью: 9 уникальных групп замечаний по 2 исходн. выполнениям; 6 для публикации (критических: 1, высоких: 1, средних: 2, низких: 2, мелких: 0). Замечания🔴 Критично
🟠 Важно
💡 Дополнительно
Автоматическое ревью от Review Engine v2.0 |
| scheduledDate: { | ||
| lte: dayjs().toISOString(), | ||
| }, | ||
| OR: [ |
There was a problem hiding this comment.
🔵 Семантика порога повторов off-by-one относительно предполагаемого намерения
Очистка удаляет напоминания с retryCount > 1 (т.е. >= 2), а инкремент retryCount устанавливает 1 при первой неудаче и 2 при второй. Поскольку deleteMany выполняется в самом начале функции, напоминание, достигшее retryCount=2 в предыдущем запуске, удаляется до повторной попытки в текущем. Эффективный максимум попыток — 2 предыдущих сбоя. Возможно, это задумано, но не задокументировано.
1 participant
Benchmark fixture for cal_dot_com PR #14943.\n\nGolden comments:
benchmark/golden_comments/cal_dot_com.json— PR14943.