Put events fix for multiple targets #11880
Conversation
LocalStack Community integration with Pro 2 files 2 suites 1h 2m 57s ⏱️ Results for commit 9698788. ♻️ This comment has been updated with latest results. |
| except Exception as error: | ||
| processed_entries.append( |
There was a problem hiding this comment.
how is this now handled, vaguely remember that AWS handles errors with targets this way - but I see that we don't have a test for this. If testing this I would do it via a missing IAM permission e.g. for EventBrdige to write to an SQS target because this should invoke a failed on send message and result in a similar error.
There was a problem hiding this comment.
Thanks for the feedback Max! After running the test against AWS, I discovered that AWS actually returns a successful EventId even when there are target permission failures. I've updated the code and tests to match this behavior.
Specifically:
- PutEvents returns a successful EventId and FailedEntryCount=0
- The failure to deliver to targets (due to missing permissions) doesn't affect the API response
- Added a snapshot test that verifies this behavior
There was a problem hiding this comment.
Ok nice - yes it is a bit strange why they do it like this - they also silently fail if you put an event to a non-existent event bus (see line 1357)
| events_put_rule( | ||
| Name=rule_name, | ||
| EventPattern=json.dumps( | ||
| {"source": ["test.source"], "detail-type": ["test.detail.type"]} |
There was a problem hiding this comment.
it would make sense to use the constant pattern here too: TEST_EVENT_PATTERN_NO_DETAIL from test_events.py and similarly use TEST_EVENT_PATTERN as test event in line 647
50578d0 to
9698788
Compare
| """Verify the message content matches what we sent.""" | ||
| body = json.loads(message["Body"]) | ||
|
|
||
| assert ( |
There was a problem hiding this comment.
are individual asserts easier than snapshotting the body? It can make the initial failure more explicit 🤔
| ): | ||
| """Test that put_events returns successful EventId even when target delivery fails due to non-existent queue.""" | ||
| # Create a queue and get its ARN | ||
| queue_url = sqs_create_queue() |
There was a problem hiding this comment.
minor: we probably don't even need to create a queue in the first place, just makes it a bit easier to get an ARN :)
Motivation
Fixes EventBridge PutEvents response so it correctly returns event IDs only once per event, regardless of how many rules or targets match. Currently, the response includes duplicate EventIds for each matching target, which doesn't match AWS behavior and can cause issues when clients try to track events by their IDs.
Related issue: #11597
Changes
_process_entriesand related methods in events provider to track and return event IDs only onceTesting