Skip to content

fix: anchor retry backoff reset to the first event of each connection - #38

Merged
tanderson-ld merged 2 commits into
mainfrom
ta/SDK-2846/good-since-once-per-stream
Sep 23, 2026
Merged

tanderson-ld merged 2 commits into
mainfrom
ta/SDK-2846/good-since-once-per-stream

Conversation

@tanderson-ld

@tanderson-ld tanderson-ld commented Sep 23, 2026 •

Copy link
Copy Markdown

Summary

retryResetIntervalMillis is documented as resetting the backoff once the stream "has remained active for some amount of time," but the implementation re-anchored the internal "good since" timestamp on every received event. That measured idle-time-before-failure rather than connection health: a chatty stream that had been active for hours never qualified for a reset (its last event was always moments before the failure), while a stream that went silent for the interval and then failed did. Tracked as SDK-2846.

Fix

The "good since" timestamp is now anchored once per connection, at the first event received on that connection. A connection that has been delivering data for at least retryResetIntervalMillis resets the backoff to the initial delay on its next failure. A connection that fails before delivering any event — including one that only sent comment heartbeats — does not count as healthy and leaves the backoff progression intact.

The README's two descriptions of this option contradicted each other (line 93 described connection-anchored semantics, the example comment described last-event semantics); both now describe the fixed behavior.

Behavior change

This changes released retry-reset timing for all consumers: long-lived active connections now correctly reset the backoff (previously they never did), and briefly-idle-then-failed connections no longer do. No API changes.

Testing

  • New test: a connection that delivers events continuously past the reset interval and then drops must reset the delay to the initial value. Verified to fail against the previous implementation.
  • New test: a connection that stays open past the reset interval sending only comment heartbeats must not reset the backoff.
  • Full suite: 90 passing, standard clean. New tests stable across repeated runs.

Note

Overview
Fixes retryResetIntervalMillis so backoff reset matches the documented “connection has been active” semantics instead of measuring idle time since the last event.

receivedEvent now calls setGoodSince only once per successful stream (first parsed event), with goodSinceAnchored cleared when a new HTTP 200 connection starts. Long-lived streams that keep delivering events can reset exponential backoff after the interval; connections that only send comment heartbeats or fail before any event still do not count as healthy.

README wording for this option is aligned with the implementation. Tests add verifyDelaysWithHandler plus scenarios for eventful long connections vs comment-only heartbeats.

Reviewed by Cursor Bugbot for commit bad4092. Bugbot is set up for automated code reviews on this repo. Configure here.

tanderson-ld added a commit that referenced this pull request Sep 23, 2026
…#39)

## Summary

Current Node releases raised the default `Buffer.poolSize` from 8192 to
65536. `lib/capacity.js` floors every allocation to `Buffer.poolSize`,
and three capacity-test expectations hardcoded values derived from the
old 8192 default, so `build-test (latest)` now fails on any branch of
this repo (first observed on #38, whose own changes pass; the failing
assertions are untouched by it).

## Fix

Test-only: the doubling and exceeds-doubling cases now express their
inputs and expectations relative to `Buffer.poolSize`, the same way the
existing minimum-capacity test already does. No production code changes.

## Testing

- `test/capacity_test.js`: 5 passing with the platform default pool size
(8192 locally) and with `Buffer.poolSize` forced to 65536 to reproduce
the current-Node condition.
- Full suite: 88 passing, `standard` clean.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> Fixes CI failures on newer Node where default **`Buffer.poolSize`** is
65536 instead of 8192. **`CalculateCapacity`** already floors small
allocations to **`Buffer.poolSize`**, but two tests in
**`capacity_test.js`** still asserted fixed doubling sizes (8192→16384,
etc.), so they broke when the pool size changed.
> 
> The **exponential doubling** and **required capacity exceeds
doubling** cases now use **`Buffer.poolSize`** for inputs and expected
outputs, matching the existing minimum-capacity test. A short comment
explains why those cases start at pool size. **No production code
changes.**
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
01adb7c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@tanderson-ld
tanderson-ld merged commit 9d46860 into main Sep 23, 2026
4 checks passed
@tanderson-ld
tanderson-ld deleted the ta/SDK-2846/good-since-once-per-stream branch September 23, 2026 17:42
tanderson-ld added a commit that referenced this pull request Sep 29, 2026
…ions (#43)

## What

Adds an explicit `permissions:` block (`contents: write`,
`pull-requests: write`) to the `release-please` job in
`.github/workflows/release-please.yml`.

## Why

release-please has failed on every push to `main` since 2026-09-23 (the
runs for #38, #40, #42), all with the same error:

```
POST /repos/launchdarkly/js-eventsource/git/refs
→ 403 "Resource not accessible by integration"   (x-accepted-github-permissions: contents=write)
✖ Error when creating branch
release-please failed: Error creating Pull Request: Resource not accessible by integration
```

The `release-please` job passes `token: ${{secrets.GITHUB_TOKEN}}` but
had **no `permissions:` block**, so it inherited the repository's
*default* `GITHUB_TOKEN` permissions. It worked for every release
through 2.2.0 (Apr 2025) because that default was read-write; it broke
once the default was tightened to read-only, which removed the write
access the job needs to create the release branch/PR. The workflow file
itself has not changed since 2024-04-26 — only the environment did.

(The lone "success" on 2026-09-23 was a `chore:` push, which
release-please treats as non-release-worthy, so it never reached the
branch-creation step. The next release-worthy commits — #38, #40, #42 —
all hit the 403.)

## Fix

Grant the two write scopes explicitly on the job that needs them, so the
workflow no longer depends on the repo default. This brings the job to
parity with the js-core monorepo's `release-please` job, which already
sets `contents: write` + `pull-requests: write` explicitly and continues
to run successfully under the same `GITHUB_TOKEN`.

## Effect

Once merged, the push to `main` re-runs release-please with the
corrected permissions and it will open the pending release PR (covering
the currently-unreleased #38, #40, #42), which on merge publishes the
next version.

## Notes / possible follow-ups (intentionally not in this PR)

- If the org has *also* disabled "Allow GitHub Actions to create and
approve pull requests," a later run could 403 on the PR-create step (a
separate setting from token scope). That setting was clearly enabled
through the 2.2.0 release, so it is likely still fine.
- The action reference here
(`google-github-actions/release-please-action@v4`) is the old org name
on a floating tag; js-core uses the renamed
`googleapis/release-please-action` pinned to a SHA. Modernizing it is
optional and left out to keep this fix focused.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> **Fixes release-please 403 failures** by giving the `release-please`
job explicit `GITHUB_TOKEN` scopes instead of relying on the repo
default (which was tightened to read-only).
> 
> The job now declares `contents: write` and `pull-requests: write` so
it can create the release branch/tag and open or update the release PR.
The downstream `release-package` job already had these permissions; only
`release-please` was missing them, which caused `Resource not accessible
by integration` on recent pushes to `main`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
97f01e5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
tanderson-ld pushed a commit that referenced this pull request Sep 29, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.3.0](2.2.0...2.3.0)
(2026-09-29)


### Features

* allow a custom retry delay strategy to be provided
([#40](#40))
([dd081f0](dd081f0))


### Bug Fixes

* anchor retry backoff reset to the first event of each connection
([#38](#38))
([9d46860](9d46860))
* destroy non-200 responses so their sockets close
([c0ba67c](c0ba67c))
* destroy non-200 responses so their sockets close
([#42](#42))
([988a7b3](988a7b3))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> This PR **bumps the package to 2.3.0** via Release Please:
`package.json`, `.release-please-manifest.json`, and a new **2.3.0**
section in `CHANGELOG.md`.
> 
> There are **no runtime or library code changes** in the diff itself.
The changelog records what ships in this tag: **custom
`retryDelayStrategy`** for reconnect timing, **retry backoff reset tied
to the first event on each connection**, and **destroying non-200 HTTP
responses** so underlying sockets close cleanly.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
ebf94e0. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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