Skip to content

fix(stream): notify all callers after a join callback unsubscribes - #2030

Open
Osraka wants to merge 2 commits into
ProjectOpenSea:mainfrom
Osraka:fix/stream-join-callback-mutation
Open

Osraka wants to merge 2 commits into
ProjectOpenSea:mainfrom
Osraka:fix/stream-join-callback-mutation

Conversation

@Osraka

@Osraka Osraka commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Related to #2029. callerHandle() can remove a caller from subscription.callbackList while sendJoin() is notifying that list. Iterating the live array then skips the next caller after a self-unsubscribe. This affects successful joins, rejected joins, and join timeouts.

Solution

Iterate a snapshot in those three join-result paths, but notify only callers that are still registered. This lets a caller remove itself without starving a sibling while preserving the behavior where a caller removed by another callback receives no notification. No public API changes.

Offline regression tests cover success, rejection, timeout, and removal of a different caller. Validation: 56 transport tests, 1,304 SDK unit tests, npm run check-types, npm run build, Biome on the two changed files, and git diff --check. The self-unsubscribe tests failed on main; the cross-unsubscribe test caught an issue in the first patch before the follow-up fix. No live OpenSea request was made.

@ryanio

ryanio commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Osraka, this is a clean fix, and the tests made it quick to verify. We've recreated it in our internal monorepo with you credited as co-author, and it will ship in the next @opensea/sdk patch release.

Two notes from review:

  1. On impact: OpenSeaStreamClient is the only caller of transport.subscribe, and its onSubscribed and onSubscribeError callbacks only log. PhoenixChannelsTransport isn't exported, so no public API path reaches this today. It's still the right fix for the transport's own contract, and the changelog entry says so.
  2. Walking a copy also fixes a case your tests didn't cover. A caller that subscribed from inside an onSubscribed callback used to be told twice: once immediately by subscribe(), since the topic is already marked joined, and once more by the live loop. Now it's told once, and we added a test for it.

This repo is a read-only mirror, so we can't merge PRs here, but we read every one. Appreciate the careful work.

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