Explorar el Código

Rule the two lessons the presence crash taught

Both presence crashes read a payload field with an assumption the library does
not hold — first payload.user_id, which no longer exists, then .name on an int
— and both were answered by the event loop with a reconnect backoff that
silently dropped inbound messages. Neither was a rule until now, so nothing
stopped the next one from being written the same way.

Write both as rules: wire enums are ints and are coerced by name and value, and
a handler exception fails the event, not the stream. The first records the
shape, the second the asymmetry that made a cosmetic feature able to cost an
inbound message. AGENTS.md is the document that binds, so the reasoning here
stays out of CONCEPTS.md.
Paul Klumpp hace 6 días
padre
commit
3697aaec95
Se han modificado 1 ficheros con 30 adiciones y 1 borrados
  1. 30 1
      AGENTS.md

+ 30 - 1
AGENTS.md

@@ -737,7 +737,36 @@ untyped source), not for dodging a type.
 
 Tests should exercise real library types too, not mocks shaped like them. The
 upload bug was invisible because the tests replaced `_upload_asset` wholesale
-with an `AsyncMock`, so no test ever touched a real `AssetUpload`.
+with an `AsyncMock`, so no test ever touched a real `AssetUpload`. The presence
+crash repeated the pattern one level down: the tests mocked a payload, and
+every mock they chose agreed on a contract the wire does not have — first a
+`StrEnum` with `.name`, then an `int` sitting on a `MagicMock`. A real
+`PresenceChangedEvent` with `status = 1` inside a real `RealtimeEvent` is what
+pins the wire shape, because the mock can no longer be the thing under test.
+
+### Wire enums arrive as ints
+
+**A realtime payload enum field is a plain `int` on the wire** — the protobuf
+runtime here has no native enum wrappers, so `payload.status` is `1`, not a
+`PresenceStatus`. Read it through `presence_pb2.PresenceStatus.Name(int(...))`
+and map that name onto chattolib's enum *by value*, never by reading `.name`
+directly: `'int' object has no attribute 'name'` is what a `StrEnum` assumption
+costs. `chattolib.bot` applies the same coercion to the same field, so the
+shape is the library's own, not ours. Two payload fields have bitten this way
+(`PresenceChangedEvent.status` and the removed `PresenceChangedEvent.user_id`),
+which is why the coercion is a rule and not an incident.
+
+### A handler bug is not a broken stream
+
+**An exception raised inside an event handler must not be answered with a
+reconnect backoff.** The event loop cannot tell a handler bug from wire damage,
+and both look identical to it — but the costs are not equal. A handler crash
+that the loop treats as a stream break costs a 1s→30s backoff, and every
+`message_posted` that arrives inside such a gap is silently dropped. A presence
+label is cosmetic; an inbound message addressed to the bot is the whole product.
+So a handler that fails on one event fails *that event*: skip it, log the kind
+and the event ID loudly, leave the cursor where it was, and keep streaming.
+Reserve the reconnect for exceptions from `stream_events` itself.
 
 ## Overriding