Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions aw_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,19 @@ def get_events(
events = self._get(endpoint, params=params).json()
return [Event(**event) for event in events]

def insert_event(self, bucket_id: str, event: Event) -> None:
def insert_event(self, bucket_id: str, event: Event) -> Event:
endpoint = f"buckets/{bucket_id}/events"
data = [event.to_json_dict()]
self._post(endpoint, data)
response = self._post(endpoint, data)
created = response.json()
return Event(**created[0]) if created else event
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent fallback returns event without server-assigned ID

When the server returns an empty list, insert_event silently falls back to returning the original event object (which has id=None if not pre-assigned). This breaks the stated contract of the function — callers expect an Event with a server-assigned ID, but they silently receive the input event instead.

Any caller that does client.delete_event(bucket_id, client.insert_event(bucket_id, e).id) would then call delete_event(bucket_id, None), causing a confusing error downstream rather than a clear failure at the insertion step.

The insert_events sibling doesn't have this fallback at all — it just returns [] on an empty response — creating an inconsistency in error handling between the two methods.

A more defensive approach would be to raise a clear error if the server unexpectedly returns an empty list, rather than silently swallowing the failure:

def insert_event(self, bucket_id: str, event: Event) -> Event:
    endpoint = f"buckets/{bucket_id}/events"
    data = [event.to_json_dict()]
    response = self._post(endpoint, data)
    created = response.json()
    if not created:
        raise Exception("Server returned empty response when inserting event")
    return Event(**created[0])


def insert_events(self, bucket_id: str, events: List[Event]) -> None:
def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]:
endpoint = f"buckets/{bucket_id}/events"
data = [event.to_json_dict() for event in events]
self._post(endpoint, data)
response = self._post(endpoint, data)
created = response.json()
return [Event(**e) for e in created]
Comment on lines +187 to +192
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert_events silently returns fewer events than inserted

If the server returns fewer events than were passed in (e.g., partial insertion, empty response), the function silently returns a shorter list — including [] for a completely empty response — with no warning or error. The caller has no way to detect that some events were not created.

Consider validating that the returned count matches the input count:

def insert_events(self, bucket_id: str, events: List[Event]) -> List[Event]:
    endpoint = f"buckets/{bucket_id}/events"
    data = [event.to_json_dict() for event in events]
    response = self._post(endpoint, data)
    created = response.json()
    if len(created) != len(events):
        raise Exception(
            f"Server returned {len(created)} events, expected {len(events)}"
        )
    return [Event(**e) for e in created]


def delete_event(self, bucket_id: str, event_id: int) -> None:
endpoint = f"buckets/{bucket_id}/events/{event_id}"
Expand Down
15 changes: 11 additions & 4 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ def test_full():
e2 = create_unique_event()
e3 = create_unique_event()
events = [e1, e2, e3]
client.insert_events(bucket_name, events)
inserted_events = client.insert_events(bucket_name, events)

# Check that insert_events returns events with server-assigned IDs
assert len(inserted_events) == len(events)
assert all(e.id is not None for e in inserted_events)

# Get events
fetched_events = client.get_events(bucket_name, limit=len(events))
Expand Down Expand Up @@ -90,15 +94,18 @@ def test_full():
)

# Create and delete an event: check that it no longer exists
# Also verify that insert_event returns the event with a server-assigned ID
e_del = create_unique_event()
client.insert_event(bucket_name, e_del)
e_del_inserted = client.insert_event(bucket_name, e_del)
assert e_del_inserted.id is not None

fetched_events = client.get_events(bucket_name)
assert (e_del.timestamp, e_del.duration, e_del.data) in [
(e.timestamp, e.duration, e.data) for e in fetched_events
]

e_del_fetched = [e for e in fetched_events if e.data == e_del.data][0]
client.delete_event(bucket_name, e_del_fetched)
# Use the ID returned from insert_event directly (no need to search for it)
client.delete_event(bucket_name, e_del_inserted.id)
fetched_events = client.get_events(bucket_name)
assert (e_del.timestamp, e_del.duration, e_del.data) not in [
(e.timestamp, e.duration, e.data) for e in fetched_events
Expand Down
Loading