Skip to content

fix(stream): resend the whole combined Firehose record on partial failure - #229

Open
mecham-lynn wants to merge 3 commits into
feat/TASK-002-p1-testsfrom
fix/TASK-002-firehose-retry
Open

mecham-lynn wants to merge 3 commits into
feat/TASK-002-p1-testsfrom
fix/TASK-002-firehose-retry

Conversation

@mecham-lynn

@mecham-lynn mecham-lynn commented Oct 16, 2026 •

Copy link
Copy Markdown

Summary

Fixes a dropped-data bug in the Firehose write path and bumps to 7.1.17. Stacked on #228; review that first.

toLeo on Firehose sends one combined record per putRecordBatch, so RequestResponses has one entry. On a partial failure the retry kept only records[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 leaves records unchanged and backs off, so the whole record is resent.

rstreams-rs carries the same fix (PARITY N13) in its P3 PR, Chub-Engineering/rstreams-rs#51.

Testing

npm ci
npx tsc --watch false
npm run coverage-all

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: each putRecordBatch sends a single combined record, so retrying by filtering RequestResponses indices could resend only the first chunk and drop the rest. On FailedPutCount, the buffer is left intact and backoff retries the full payload; logging now includes the first response’s ErrorCode / ErrorMessage.

Unit tests were updated for gzip and raw Firehose modes to assert the second attempt resends the same Data with 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.

mecham-lynn and others added 2 commits October 15, 2026 12:29
…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>
@cursor

cursor Bot commented Oct 16, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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)

@ch-snyk-sa

ch-snyk-sa commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues
✅ Code Security 0 0 0 0 0 issues

💻 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>
@cursor

cursor Bot commented Oct 19, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants