fix(events): require valid data.timestamp at ingest (HTTP 400)#30
Closed
colombod wants to merge 1 commit into
Closed
fix(events): require valid data.timestamp at ingest (HTTP 400)#30colombod wants to merge 1 commit into
colombod wants to merge 1 commit into
Conversation
…silent dead-letter
The /events endpoint accepted events whose data.timestamp was missing/empty (202,
created_by stamped) but the durable drainer then crashed building the graph node
(datetime.fromisoformat('') -> ValueError), retried, and dead-lettered them -- no node,
and no error surfaced to the caller. Now:
- post_events validates data.timestamp is present, a non-empty string, and valid
ISO-8601 BEFORE queuing; otherwise HTTP 400 with a clear, value-naming message.
- make_node_id wraps the parse and re-raises a NAMED error (event + session in the
message) so any malformed event that bypasses ingest dead-letters legibly, not as a
bare 'Invalid isoformat string'.
Verified safe against real traffic: 224,530 real events across 759 on-disk records all
carry data.timestamp, so the 400 only catches malformed/hand-rolled payloads (the gap
surfaced in the live AC10 run). 11 new tests; 15 pre-existing /events tests that sent
timestamp-less payloads updated to well-formed bodies (assertions unchanged). Suite 1376 green.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem:
/eventsaccepted events missingdata.timestampwith 202, but the durable graph drainer later crashed (datetime.fromisoformat('')), retried, and dead-lettered them — a silent data loss (caller saw success, no node landed). Surfaced during the live Entra AC10 end-to-end run.Fix (Option A — reject at the boundary):
post_eventsvalidatesdata.timestamp(present, non-empty string, valid ISO-8601) BEFORE queuing → HTTP 400 with a clear, value-naming message.make_node_idre-raises a NAMED error (event + session) so anything that ever bypasses ingest dead-letters legibly instead of as a bareInvalid isoformat string.Safe against real traffic: a scan of 224,530 real events across 759 on-disk records found 0 missing
data.timestamp— the 400 only catches malformed/hand-rolled payloads.11 new tests (8 HTTP-level 400 cases + 3 drainer-guard); 15 pre-existing
/eventstests that were sending timestamp-less payloads updated to well-formed bodies (assertions unchanged). Full suite 1376 green. Independent of the Entra auth PR (#29).