Сдавшийся Connect() отдаёт OperationCanceledException вместо NotConnectedException: отменённый server_info из SetNetworkId

[Platonenkov]

Тело переписано. Первая редакция описывала механизм неверно — разбор ошибки в комментарии ниже. Здесь — то, что подтверждено по коду.

Сдавшись подключаться, клиент обязан разбудить вызывающего NotConnectedException. Иногда вместо этого прилетает OperationCanceledException — и вызывающий вправе прочитать это как «операцию отменили снаружи», хотя её никто не отменял.

Симптом

Failed TestPermanentlyFailingOnConnectedHandlerStops [438 ms]
  Assert.IsInstanceOfType failed. 'value' expression: 'connectError'.
  Giving up must unblock the waiting caller with NotConnectedException,
  got: OperationCanceledException.

Механизм

XrplClient.Connect — это две операции, а не одна (IXrplClient.cs:787):

await connection.Connect(cancellationToken);
await SetNetworkId();            // шлёт server_info

Путь сдачи OnConnectHandlerFailedAsync зовёт requestManager.RejectAllWithCancellation() (connection.cs:2121), а тот отклоняет все запросы в полёте с OperationCanceledException("Connection was intentionally closed.") (RequestManager.cs).

Дальше исход решает, кто успел:

порядок	что получает вызывающий
цикл опроса в WaitForConnectionAsync заметил сдачу	NotConnectedException — правильно
сокет на мгновение был открыт, connection.Connect() вернулся успешно, управление дошло до SetNetworkId() — и его server_info отклонили отменой	OperationCanceledException — неправильно

То есть спор идёт не между двумя способами разбудить ожидающего, а между «ожидающий заметил сдачу» и «вызывающий проскочил дальше и получил отменённый запрос».

Чего в этом механизме нет

ConnectionManager к этому сценарию отношения не имеет, хотя выглядит подходящим подозреваемым:

• WaitForConnectionAsync (connection.cs:1001) — цикл опроса с интервалом 100 мс, а не ожидание обещания. Он сам бросает NotConnectedException, увидев _permanentlyDisconnected или исчерпанные попытки;
• connectionManager.AwaitConnection() зовётся ровно в одном месте — connection.cs:1116 — и только если попытка уже в полёте (State() == WebSocketState.Connecting). При первом Connect() этого нет;
• ветки отмены в самом цикле опроса завязаны на cancellationToken, а Connect() в тесте вызван без токена, то есть с CancellationToken.None.

Почему это важно за пределами теста

OperationCanceledException означает «операцию отменили». Потребитель, который ловит его, чтобы отличить отмену от отказа — например, не логировать отмену как ошибку и не показывать её пользователю, — при исчерпании попыток подключения молча проглотит настоящий отказ. Два исключения означают разные вещи, и подменять одно другим по стечению обстоятельств нельзя.

Наблюдаемость

Флак виден только на CI:

• PR Models.Path становится Common.PathStep: имя перестаёт спорить с System.IO.Path и с кодеком #121: на одной ветке прогон в 12:15 UTC зелёный, в 12:28 красный, разница между коммитами — один doc-комментарий, кода соединения не касающийся. Перезапуск той же джобы — снова зелёный;
• локально тест проходит 12 из 12 подряд, полный набор TestU (1137 тестов, параллельно по классам) тоже зелёный.

След, за которым стоит проследить

В #114 в OnConnectHandlerFailedAsync добавлен StopMessageProcessor() (connection.cs:2120) — он блокирующе ждёт задачу читателя до двух секунд и стоит прямо перед RejectAllWithCancellation(), то есть внутри окна этой гонки. Гонка существовала и до него; правка могла изменить, кто её выигрывает.

Что делать

Не поднимать таймаут и не перезапускать джобу. Тип исключения должен определяться причиной остановки, а не тем, кто первым добежал:

• сдался цикл подключения → NotConnectedException, независимо от того, на какой стадии Connect() это застало;
• запрос отменён потому, что клиент сдавался, а не потому, что отмену запросили снаружи → вызывающий не должен видеть это как отмену.

Тест верный и трогать его не нужно — он расхождение и обнаружил.

[Platonenkov]

Поправка: разбор в теле задачи неверен. Механизм другой.

Я написал, что гонятся два пути пробуждения через ConnectionManager. Проверил по коду — это не так, и ошибку надо исправить до того, как по ней начнут чинить.

Connect() не использует ConnectionManager вовсе в этом сценарии:

• WaitForConnectionAsync (connection.cs:1001) — это цикл опроса с интервалом 100 мс, а не ожидание обещания. Он сам бросает NotConnectedException, когда видит _permanentlyDisconnected или исчерпанные попытки;
• connectionManager.AwaitConnection() зовётся ровно в одном месте — connection.cs:1116, и только если попытка уже в полёте (State() == WebSocketState.Connecting). При первом Connect() в тесте этого нет;
• ветки отмены в самом цикле опроса завязаны на cancellationToken, а тест зовёт Connect() без токена, то есть CancellationToken.None — сработать они не могут.

Откуда тогда OperationCanceledException. XrplClient.Connect — это две операции:

await connection.Connect(cancellationToken);
await SetNetworkId();

а SetNetworkId() шлёт server_info. Путь сдачи OnConnectHandlerFailedAsync зовёт requestManager.RejectAllWithCancellation(), который отклоняет все запросы в полёте с OperationCanceledException("Connection was intentionally closed.").

Значит гонка вот в чём:

• цикл опроса успел увидеть сдачу → NotConnectedException, тест зелёный;
• сокет на мгновение был открыт, connection.Connect() вернулся успешно, управление дошло до SetNetworkId(), и его запрос отклонили отменой → OperationCanceledException, тест красный.

То есть спор не между двумя способами разбудить ожидающего, а между «ожидающий заметил сдачу» и «вызывающий успел проскочить дальше и получил отменённый запрос».

Подозрение на StopMessageProcessor() из #114 при этом остаётся в силе и даже усиливается: его блокирующее ожидание до двух секунд стоит в OnConnectHandlerFailedAsync перед RejectAllWithCancellation(), то есть прямо в окне этой гонки.

Тело задачи поправлю следующим редактированием; здесь оставляю разбор, чтобы было видно, что именно было неверно.