Repository navigation
fix(stream): resend the whole combined Firehose record on partial failure - #229
Open
mecham-lynn wants to merge 3 commits into
Open
mecham-lynn wants to merge 3 commits into
mecham-lynn wants to merge 3 commits into
Conversation
…lure The Firehose path sends one combined record, so RequestResponses has a single entry and the index-based retry filter dropped every chunk after the first. Leave records unchanged and back off so the whole record is resent. Bump to 7.1.17. Task: TASK-002 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e record Use one-event chunks with gzipFirehose so the first putRecordBatch carries several chunk lines; the retry must resend them byte for byte. Fails against the old index-based retry. Task: TASK-002 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: ecb652fe-0fca-4c4f-9d89-6d517f7b545d) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
The partial-failure log now labels its two numbers (failed Firehose records, buffered chunks) and includes the first response's ErrorCode and ErrorMessage. The gzip resend test decodes the resent record, and a new test covers the raw (gzip off) resend path. Task: TASK-002 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 8de5df0e-2b26-407c-9b7f-033b4b5ff319) |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a dropped-data bug in the Firehose write path and bumps to 7.1.17. Stacked on #228; review that first.
toLeoon Firehose sends one combined record perputRecordBatch, soRequestResponseshas one entry. On a partial failure the retry kept onlyrecords[i]for each failed index, which resent the first record and dropped the rest of the payload (lib/stream/leo-stream.js, noted in #228). The fix leavesrecordsunchanged and backs off, so the whole record is resent.a13f77e: the fix and the version bump.6a19822: the test: pin leo-sdk-nodejs behavior as the reference for rstreams-rs parity #228 test now uses one-event chunks withgzipFirehose, so the first put carries several chunk lines, and asserts the retry resends them byte for byte. It fails against the old index-based filter.rstreams-rs carries the same fix (PARITY N13) in its P3 PR, Chub-Engineering/rstreams-rs#51.
Testing
685 passing, 0 failing.
Task: TASK-002 (no Jira yet)
🤖 Generated with Claude Code
Note
Medium Risk
Changes Firehose retry behavior on a critical write path; the fix reduces dropped data but retries may resend a larger combined record than before.
Overview
Fixes data loss on Firehose partial failures in
toLeo: eachputRecordBatchsends a single combined record, so retrying by filteringRequestResponsesindices could resend only the first chunk and drop the rest. OnFailedPutCount, the buffer is left intact and backoff retries the full payload; logging now includes the first response’sErrorCode/ErrorMessage.Unit tests were updated for gzip and raw Firehose modes to assert the second attempt resends the same
Datawith all events. Package version is bumped to 7.1.17.Reviewed by Cursor Bugbot for commit 4e8d6d0. Bugbot is set up for automated code reviews on this repo. Configure here.