diff --git a/tradein-mvp/backend/app/api/v1/support.py b/tradein-mvp/backend/app/api/v1/support.py index 07406a23..5183efbe 100644 --- a/tradein-mvp/backend/app/api/v1/support.py +++ b/tradein-mvp/backend/app/api/v1/support.py @@ -358,15 +358,25 @@ def _resolve_idempotency_key(request: Request, *, identity_key: str, text: str) """Ключ идемпотентности inbound-отправки (#3471, миграция 301). Почему заголовок + fallback, а не что-то одно. Явный `Idempotency-Key` — - предпочтительный путь: клиент генерирует ключ ОДИН раз на "намерение + ЕДИНСТВЕННЫЙ надёжный путь: клиент генерирует ключ ОДИН раз на "намерение отправить" и переиспользует его на любом ретрае (fetch retry / переотправка после таймаута) независимо от того, что именно менялось в UI между - попытками. Без заголовка (старые клиенты, п.5 требования — они не должны - сломаться) используем детерминированный отпечаток sha256(identity, текст, - минутное окно): типичный повтор (двойной клик, ретрай браузера) укладывается - в секунды, минутное окно с запасом это покрывает, а любые два РАЗНЫХ по - смыслу сообщения с одинаковым текстом, отправленные намеренно с разницей в - пару минут, в одно не схлопнутся. + попытками (см. `useSendSupportMessage` во фронтенде — реальный трафик + обязан идти этим путём). Fallback без заголовка — детерминированный + отпечаток sha256(identity, текст, минутное окно) — существует ТОЛЬКО для + клиентов, которые заголовок не прислали (п.5 требования — старое поведение + не должно сломаться), и у него два честных изъяна, оба снимаются самим + фактом использования заголовка: + 1. два РАЗНЫХ по смыслу сообщения с одинаковым текстом от одного и того + же человека в течение одной минуты ("да" и ещё раз "да", "+", "ок") + СХЛОПНУТСЯ в одно — второе будет молча проглочено, клиент получит id + первого, оператор не увидит второе сообщение вообще; + 2. окно — не скользящие 60 секунд, а округление `time.time() // 60` вниз: + фактический срок дедупликации случаен от 0 до 60с в зависимости от + момента внутри минуты, и часть настоящих повторов (ретрай ровно на + границе окна) эту защиту не получит. + Оба пункта перестают быть важны, когда клиент реально шлёт заголовок + (см. `useSendSupportMessage`). Хранит и потенциально логирует (см. вызовы в этом файле) только САМ ключ — он либо непрозрачный клиентский токен, либо хэш. Текст сообщения сюда @@ -422,12 +432,21 @@ async def send_support_message( if cooldown is not None: raise _too_many_failures_error(cooldown) - # Идемпотентность (#3471): pre-check ДО похода в Telegram — повтор с тем же - # ключом не должен слать второе зеркало в топик, не только не писать вторую - # строку в БД. Тред может ещё не существовать (это первая отправка этого - # ключа) — тогда сравнивать не с чем, идём в Telegram как обычно; сам факт - # "нет треда" здесь безопасен, т.к. тред создаётся ТОЛЬКО в этой же ручке - # (см. H1) — если бы он уже был записан под этим ключом, тред уже был бы. + # Идемпотентность (#3471). Что именно закрывает этот pre-check — важно не + # переоценить: строка в БД (и её idempotency_key) появляется ТОЛЬКО ПОСЛЕ + # успешной доставки в Telegram (H1 ниже), поэтому потерю самого запроса на + # плече Selectel -> api.telegram.org этот механизм НЕ дедуплицирует — если + # `send_message` не удался, ключ нигде не записан, и повтор клиента после + # неудачи это законная первая попытка. Реально закрывается другой, тоже + # частый случай: доставка УЖЕ состоялась (Telegram принял, строка + # закоммичена), но ответ до клиента не дошёл (обрыв на обратном пути, + # клиентский таймаут) или пользователь кликнул "отправить" второй раз по + # той же ещё не отрисовавшейся отправке — тогда pre-check находит уже + # записанное сообщение по ключу и отдаёт его же, не уходя в Telegram снова. + # Тред может ещё не существовать (это первая попытка этого ключа) — тогда + # сравнивать не с чем, идём в Telegram как обычно; сам факт "нет треда" + # здесь безопасен, т.к. тред создаётся ТОЛЬКО в этой же ручке (см. H1) — + # если бы сообщение под этим ключом уже было записано, тред уже был бы. idempotency_key = _resolve_idempotency_key(request, identity_key=username, text=payload.text) existing_thread_id = storage.find_thread_id(db, username) if existing_thread_id is not None: diff --git a/tradein-mvp/backend/app/services/tgbot/web_support_storage.py b/tradein-mvp/backend/app/services/tgbot/web_support_storage.py index feb9ec2b..8d80e1f1 100644 --- a/tradein-mvp/backend/app/services/tgbot/web_support_storage.py +++ b/tradein-mvp/backend/app/services/tgbot/web_support_storage.py @@ -149,14 +149,74 @@ def record_inbound( ) if row is not None: return dict(row) - # Конфликт пойман индексом (см. докстринг выше) — idempotency_key точно не - # NULL здесь: при NULL partial-индекс в конфликт не участвует вообще. - assert idempotency_key is not None - existing = find_inbound_by_idempotency_key( - db, thread_id=thread_id, idempotency_key=idempotency_key + + # Конфликт пойман индексом — idempotency_key почти наверняка не NULL здесь + # (при NULL partial-индекс в конфликт не участвует), но НЕ `assert`: это + # прод-путь ПОСЛЕ уже доставленного в Telegram сообщения (см. H1 в + # app.api.v1.support), а `except SQLAlchemyError` в вызывающей стороне + # `AssertionError` не ловит — под `python -O` assert вдобавок исчезает + # молча и `existing` осталось бы `None`, что развалилось бы чуть ниже при + # сборке ответа. Вместо падения — явная ветка с логом и WORKING откатом. + existing = ( + find_inbound_by_idempotency_key(db, thread_id=thread_id, idempotency_key=idempotency_key) + if idempotency_key is not None + else None ) - assert existing is not None # конфликт по индексу гарантирует наличие строки - return existing + if existing is not None: + # Ожидаемый случай гонки (#3471): проигравший запрос уже отправил своё + # СОБСТВЕННОЕ зеркало в Telegram (свой topic_message_id) до того, как + # обнаружил конфликт здесь — эта копия зеркала теперь осиротела + # (реплай оператора на неё никуда не смаршрутизируется, т.к. строки + # для неё в БД нет). Осознанно не чиним это в этом PR (см. коммент + # выше по коду), но фиксируем в логе, а не молчим — ровно то же самое, + # для чего уже есть `_warn_operator_message_not_recorded` в другом месте. + logger.warning( + "web support: гонка по idempotency_key — зеркало topic_message_id=%s " + "(thread_id=%d) отправлено, но НЕ записано, победила строка id=%d", + topic_message_id, + thread_id, + existing["id"], + ) + return existing + + # Крайний случай (в норме недостижим при READ COMMITTED, которую использует + # этот сервис): индекс сообщил о конфликте, но повторное чтение строку не + # нашло. Логируем и пишем БЕЗ идемпотентности — NULL-ключ никогда не + # конфликтует сам с собой (partial-индекс его не видит), INSERT гарантированно + # пройдёт; для этого одного сообщения дедупликация отключается, но клиент + # получает корректный ответ вместо 500 после уже состоявшейся доставки. + logger.warning( + "web support: ON CONFLICT сообщил о конфликте (thread_id=%d, " + "idempotency_key=%s), но повторное чтение строки её не нашло — " + "записываем без идемпотентности", + thread_id, + "" if idempotency_key is not None else None, + ) + row = ( + db.execute( + text( + """ + INSERT INTO web_support_messages + (thread_id, direction, text_body, topic_message_id, + support_chat_id, operator_tg_id, idempotency_key, created_at) + VALUES + (CAST(:thread_id AS bigint), 'in', :text_body, + CAST(:topic_message_id AS bigint), + CAST(:support_chat_id AS bigint), NULL, NULL, NOW()) + RETURNING id, direction, text_body, operator_tg_id, created_at + """ + ), + { + "thread_id": thread_id, + "text_body": text_body, + "topic_message_id": topic_message_id, + "support_chat_id": support_chat_id, + }, + ) + .mappings() + .one() + ) + return dict(row) def find_thread_by_topic_message( diff --git a/tradein-mvp/frontend/src/lib/useSupportChat.ts b/tradein-mvp/frontend/src/lib/useSupportChat.ts index 209b2984..cdf712b5 100644 --- a/tradein-mvp/frontend/src/lib/useSupportChat.ts +++ b/tradein-mvp/frontend/src/lib/useSupportChat.ts @@ -42,6 +42,7 @@ * setQueryData-merge path that would be pure incidental complexity here. */ +import { useRef } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { apiFetch, HTTPError } from "@/lib/api"; @@ -129,13 +130,36 @@ export function useSupportUnread(enabled: boolean, scope: SupportScope = "auth") export function useSendSupportMessage(scope: SupportScope = "auth") { const queryClient = useQueryClient(); + // Идемпотентность (#3471): ОДИН `Idempotency-Key` на все повторы ОДНОГО + // намерения отправить. Бэкенд пишет строку/ключ ТОЛЬКО ПОСЛЕ успешной + // доставки в Telegram, поэтому это не про потери на плече Selectel -> + // api.telegram.org (неудачный send там — законная первая попытка) — это + // про то, что доставка УЖЕ состоялась, а ответ до клиента не дошёл (обрыв + // на обратном пути / клиентский таймаут) или пользователь дважды нажал + // "отправить" по одной и той же ещё не отрисовавшейся отправке. Без + // заголовка единственная защита — backend-fallback (минутный + // hash-отпечаток, `_resolve_idempotency_key` в app/api/v1/support.py), а + // он ненадёжен для осмысленно повторяющихся коротких сообщений ("+", "ок", + // два "да" подряд). Намерение отождествляем с текстом: тот же текст подряд — + // это ретрай ЭТОГО намерения, переиспользуем ключ; новый текст — новое + // намерение, новый UUID. + const intentRef = useRef<{ text: string; key: string } | null>(null); + return useMutation({ - mutationFn: (text) => - apiFetch(`${scopeBase(scope)}/messages`, { + mutationFn: (text) => { + if (!intentRef.current || intentRef.current.text !== text) { + intentRef.current = { text, key: crypto.randomUUID() }; + } + return apiFetch(`${scopeBase(scope)}/messages`, { method: "POST", + headers: { "Idempotency-Key": intentRef.current.key }, body: JSON.stringify({ text }), - }), + }); + }, onSuccess: () => { + // Успех закрывает намерение — следующая отправка (даже с тем же текстом) + // обязана получить НОВЫЙ ключ, иначе она молча схлопнется с этой. + intentRef.current = null; queryClient.invalidateQueries({ queryKey: messagesKey(scope) }); }, });