Skip to content

Message remove in loop #16

@antidodo

Description

@antidodo

⚠️ Potential issue | 🟠 Major

Modifying a list while iterating over it causes skipped elements.

When you call remove() on a list while iterating over it with a for loop, Python may skip elements because the indices shift. If multiple messages match the message_id, some may not be deleted.

🐛 Proposed fix using list comprehension or iteration over a copy
     def delete_message_by_id(self, message_id: str, type: Literal["outgoing", "incoming"]) -> int:
         """
         Delete a message from the outgoing messages list.
         :param type:
         :param message_id:
         :return:
         """
         number_of_deleted_messages = 0
         if type in ["outgoing", "incoming"]:
-            for message in self.list_of_outgoing_messages if type == "outgoing" else self.list_of_incoming_messages:
+            target_list = self.list_of_outgoing_messages if type == "outgoing" else self.list_of_incoming_messages
+            for message in target_list[:]:  # Iterate over a copy
                 if message.body["meta"]["id"] == message_id:
-                    self.list_of_outgoing_messages.remove(message) \
-                        if type == "outgoing" else self.list_of_incoming_messages.remove(message)
+                    target_list.remove(message)
                     number_of_deleted_messages += 1
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

        if type in ["outgoing", "incoming"]:
            target_list = self.list_of_outgoing_messages if type == "outgoing" else self.list_of_incoming_messages
            for message in target_list[:]:  # Iterate over a copy
                if message.body["meta"]["id"] == message_id:
                    target_list.remove(message)
                    number_of_deleted_messages += 1
🤖 Prompt for AI Agents
In `@flamesdk/resources/client_apis/clients/message_broker_client.py` around lines
207 - 212, The loop removes items from list_of_outgoing_messages or
list_of_incoming_messages while iterating, which can skip elements; update the
code in the MessageBrokerClient method that currently checks `if type in
["outgoing", "incoming"]` to avoid in-loop removals by either iterating over a
copy (e.g., list(self.list_of_outgoing_messages) /
list(self.list_of_incoming_messages)) or replacing the list with a filtered
comprehension that excludes messages whose body["meta"]["id"] equals message_id;
ensure number_of_deleted_messages is computed correctly based on the difference
in lengths or by counting matches, and keep the logic that selects between
outgoing vs incoming based on `type`.

Originally posted by @coderabbitai[bot] in #14 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions