fix(node): prevent closeAndFlush from dropping in-flight events#1365
Merged
Conversation
closeAndFlush had two issues that caused data loss under rate limiting: 1. The default timeout (flushInterval * 1.25 = 12.5s) was shorter than a single Retry-After: 60 cycle. Raised the floor to max(60s, flushInterval) * 1.25. 2. On timeout, .catch(() => undefined) silently swallowed the error and returned, but retry loops continued running. The process would then exit killing in-flight retries. Now calls publisher.abort() to cancel pending sleeps via AbortController, and marks affected batches as failed.
Adds an http_response event to the NodeEmitter contract and emits it from the publisher after each API response. Enables consumers to observe status codes, headers, and timing without monkey-patching fetch.
Node CLI: use http_request/http_response listeners for verbose logging. Browser CLI: track Retry-After from 429 responses and use it as the settle time, so the CLI doesn't exit while retries are still pending. Both CLIs now respect config.timeout for their wait durations.
🦋 Changeset detectedLatest commit: 1e85dea The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1365 +/- ##
==========================================
- Coverage 91.72% 91.57% -0.15%
==========================================
Files 163 163
Lines 4674 4699 +25
Branches 1155 1157 +2
==========================================
+ Hits 4287 4303 +16
- Misses 387 396 +9
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The previous logic forced a 60s minimum when Retry-After was 0 or missing, causing e2e tests with short/zero Retry-After to time out. Now uses the actual header value (default 10s when absent), so Retry-After: 0 settles quickly while Retry-After: 60 waits properly.
With the new 75s closeAndFlush timeout floor, the "handles a bad key" test would exceed the 30s jest timeout. Pass an explicit 20s timeout and register resolveCtx before closeAndFlush so the error listener catches the delivery failure event when abort fires.
bsneed
approved these changes
Apr 27, 2026
didiergarcia
approved these changes
Apr 27, 2026
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
.catch(() => undefined)) and returned without cancelling in-flight retry loops. On process exit, pending retries were killed mid-flight. Now callspublisher.abort()via AbortController to cancel pending sleeps and mark affected batches as failed.flushInterval * 1.25(12.5s default) toMath.max(60s, flushInterval) * 1.25(75s minimum), so the default timeout survives at least one Retry-After: 60 cycle.Test plan