🥏 Discard failing | all pending messages in message tab

[davoclavo]

Add button to discard all failing or pending messages.

Note:
The current implementation discards all currently loaded/buffered messages, but there might be some messages that did not yet fit in our internal buffer, thus aren't discarded. So in case a Sink is processing thousands of messages and you want to discard them all, you'd need to discard buffered portions multiple times.

Do we want a different mechanism? or is that OK?

Update: I guess that is OK, if the user wants to discard any upcoming messages they could disable the sink.

---

Fixes #1435

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (3)

Skipped posting 3 drafted comments based on your review threshold. Feel free to update them here.

assets/svelte/consumers/ShowMessages.svelte (1)

344-355: handleDiscardMessages does not handle or report errors from the backend, so users will not be notified if discarding messages fails, leading to silent failures.

Scores:

• Production Impact: 3
• Fix Specificity: 5
• Urgency Impact: 2
• Total Score: 10

Reason for filtering: This is a valid comment identifying a real issue. The handleDiscardMessages function at lines 344-355 lacks error handling in its callback, unlike other similar functions in the codebase (e.g., resetMessageVisibility at lines 305-318 which properly handles reply.error). The commitable suggestion is safe and follows the existing error handling pattern used elsewhere in the file.

Analysis: The comment correctly identifies missing error handling in handleDiscardMessages. Production impact is moderate (3) because silent failures can confuse users but won't crash the system. Fix specificity is high (5) as the suggestion provides exact, ready-to-use code that follows existing patterns. Urgency is low-medium (2) since the system functions but user experience suffers from lack of feedback on failures. The suggested fix is consistent with error handling patterns already established in the codebase.

---

lib/sequin/runtime/slot_message_store.ex (2)

1168-1188: discard_messages_and_update_state/2 does not update the payload_size_bytes field in the new state, causing payload_size_bytes to become stale and incorrect after discards.

Scores:

• Production Impact: 0
• Fix Specificity: 0
• Urgency Impact: 0
• Total Score: 0

Reason for filtering: The comment makes unverified assumptions about the State module's internal structure and the behavior of State.pop_messages/2. Without seeing the State module implementation, we cannot confirm that payload_size_bytes becomes stale after discards, or that State.pop_messages/2 doesn't already handle this correctly. The suggested fix assumes specific internal structures (new_state0.messages as a Map, payload_size_bytes field in state) that aren't visible in the provided context.

Analysis: While the comment identifies a potentially valid concern about state consistency, it's based on assumptions about internal State module behavior that cannot be verified from the provided context. The State.pop_messages/2 function may already handle payload_size_bytes updates correctly, making this suggestion unnecessary or potentially harmful by introducing redundant logic.

---

768-792: discard_failing_messages and discard_all_messages use Map.values(state.messages) and Enum.filter to build large lists in memory, which can cause high memory usage and GC pressure for stores with many messages.

Scores:

• Production Impact: 3
• Fix Specificity: 4
• Urgency Impact: 2
• Total Score: 9

Reason for filtering: Valid performance optimization comment. The line numbers correctly identify code that uses Map.values() and Enum.filter() which creates intermediate lists. The suggested :maps.fold/3 approach would reduce memory allocation and GC pressure in message stores with many messages.

Analysis: This identifies a real memory efficiency issue in a message store system. While not immediately critical, it could cause performance degradation under high message loads. The fix is specific and technically sound, using Erlang's :maps.fold/3 to avoid intermediate list creation. Medium production impact as it affects performance but won't cause crashes. Low-medium urgency as it's an optimization rather than a bug fix.

---

[entelligence-ai-pr-reviews]

LGTM 👍