fix(tradein): включаем idempotency-key на фронте, чиним assert-crash и честность докстрингов (#3471)
All checks were successful
CI Trade-In / backend-tests (pull_request) Successful in 7m35s
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 11s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m10s
All checks were successful
CI Trade-In / backend-tests (pull_request) Successful in 7m35s
CI Trade-In / changes (pull_request) Successful in 9s
CI / changes (pull_request) Successful in 11s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Successful in 1m10s
Ревью PR #3495 нашло, что механизм был мёртвым кодом: фронт не отправлял Idempotency-Key ни в одном запросе, весь прод-трафик шёл по ненадёжному fallback-отпечатку. Плюс два "assert" в record_inbound после ON CONFLICT давали AssertionError (не ловится except SQLAlchemyError) уже ПОСЛЕ доставки в Telegram — под `python -O` assert и вовсе исчезает. - useSupportChat.ts: useSendSupportMessage генерирует Idempotency-Key (crypto.randomUUID()) на намерение отправить, переиспользует его при повторной отправке ТОГО ЖЕ текста, сбрасывает на успехе. - web_support_storage.record_inbound: assert -> явные ветки с логом; логируем отброшенный topic_message_id проигравшего гонку (не молча). - Докстринги/комментарии переписаны честно: что именно закрывает pre-check (ответ клиенту потерян / двойной клик после успеха), а что НЕ закрывает (сетевую потерю на плече Selectel -> Telegram — там сообщение просто не доставлено, повтор это законная первая попытка). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JY6iWDnGDthdvsMWgK1BMG
This commit is contained in:
parent
1b595a0091
commit
d5c876e3d0
3 changed files with 126 additions and 23 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
"<set>" 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(
|
||||
|
|
|
|||
|
|
@ -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<SupportMessage, Error, string>({
|
||||
mutationFn: (text) =>
|
||||
apiFetch<SupportMessage>(`${scopeBase(scope)}/messages`, {
|
||||
mutationFn: (text) => {
|
||||
if (!intentRef.current || intentRef.current.text !== text) {
|
||||
intentRef.current = { text, key: crypto.randomUUID() };
|
||||
}
|
||||
return apiFetch<SupportMessage>(`${scopeBase(scope)}/messages`, {
|
||||
method: "POST",
|
||||
headers: { "Idempotency-Key": intentRef.current.key },
|
||||
body: JSON.stringify({ text }),
|
||||
}),
|
||||
});
|
||||
},
|
||||
onSuccess: () => {
|
||||
// Успех закрывает намерение — следующая отправка (даже с тем же текстом)
|
||||
// обязана получить НОВЫЙ ключ, иначе она молча схлопнется с этой.
|
||||
intentRef.current = null;
|
||||
queryClient.invalidateQueries({ queryKey: messagesKey(scope) });
|
||||
},
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue