fix(streaming): harden Redis ACK handling and document topics#7
Conversation
Moves xack outside the callback error handler so a failed callback never skips the ACK and causes messages to accumulate in the PEL. safeAck swallows transient Redis ACK errors to keep the loop alive.
Replaces the Default setup which was incorrectly configured to scan the 'actions' language (GitHub Actions YAML) — this repo has no custom action source code, causing the scan to fail with exit code 32.
|
@JC5467 Go ahead and take care of reviewing this when you can, thank you! |
jrb00013
left a comment
jrb00013
left a comment
There was a problem hiding this comment.
Why is this moved in the catch error exception block now?
jrb00013
changed the title
|
Hey Joe, the safeAck actually moved after the try catch block, not into the catch. The structure is try callback, catch and log any error then always ACK. The reason is that the old code only ACKed inside the try, so if the callback threw an exception the message was never acknowledged, it'd pile up in the Redis Pending Entry List and be redelivered to the consumer in an infinite loop. Moving it outside the catch makes the ACK unconditional. safeAck is just a thin wrapper that silences transient Redis xack errors so they don't crash the consumer loop. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Should we fix the underlying Redis xack errors to begin with instead of just silencing them? |
|
Hey Joe, I updated the shared StreamingClient ACK handling so we are no longer silently swallowing Redis xack failures. What I changed: Kept ACK outside the callback try/catch so callback failures don’t leave messages stuck in PEL. |
|
But what about figurting out the underlying Redis xack errors to begin with? Or were they hypothetical |
1 similar comment
BaoT1301
changed the title
|
@BaoT1301 But what about figurting out the underlying Redis xack errors to begin with? Or were they hypothetical |
4 participants
IMPORTANT:
Description
Hardens the shared Redis Streams client ACK behavior and exposes typed document stream topic constants for the current Deepiri data-streaming contract.
Include:
@deepiri/shared-utilsstreaming client and topic registryChanges
XACKhandling outside callback failure paths so callback errors do not leave messages stuck in the pending entries list.document.vectorize,document.training,document.structured, anddocument.artifactsconstants toStreamTopics.document.*topic constant values.npm install, build, and tests; removed the broken custom CodeQL workflow conflict with GitHub Default Setup.ts-jestdev dependency used by Jest config.Related
Testing
Verified locally:
npm testnpm run buildRemote GitHub checks are passing:
Additional testing details:
Important Notes (Optional)
Workflow Checklist (Required)
Review Requests
@Team-Deepiri/support-team