[flags-core] re-wire source events when a source restarts - #477
Draft
dferber90 wants to merge 1 commit into
Draft
Conversation
shutdown() unwires the stream and polling event handlers, but nothing wired them again. A client initialized after a shutdown opened a new connection and then ignored every datafile that connection delivered, so evaluations returned the default value with an error reason. The failure was silent: the socket was healthy and the fetch happened, only the handlers were missing. wireSourceEvents() now runs at the start of tryInitializeStream() and tryInitializePolling(), the only two places that start a source. Repeat calls cannot duplicate a subscription, because the handlers are stable instance properties and TypedEmitter keeps them in a Set. Two tests cover that invariant: one across a shutdown, and one for a source that starts twice within a single lifecycle, which happens when a failed initialization is followed by an evaluation. This also affects the VercelProvider OpenFeature provider, whose onClose() hook shuts the client down, since section 2.5.2 of the provider spec allows a provider to be initialized again after it is closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dferber90
marked this pull request as draft
August 15, 2026 05:48
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
1 participant
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.
The defect
shutdown()removes the event handlers of the stream source and the polling source. But no code adds these handlers again.wireSourceEvents()occurs only in the constructor.Thus a client that you initialize after a shutdown opens a new connection, but it ignores all data from that connection. Evaluations then give the default value, with the reason
error.The failure is difficult to find, because it is silent. The connection is good, and the request to
/v1/streamoccurs. Only the handlers are absent.This condition also applies to the
VercelProviderOpenFeature provider. ItsonClose()hook shuts the client down, and requirement 2.5.2 of the OpenFeature provider specification permits a new initialization after a shutdown.The correction
wireSourceEvents()now occurs at the start oftryInitializeStream()andtryInitializePolling(). These two functions are the only functions that start a source. I examined all 6 calls ofstreamSource.start(),pollingSource.poll(), andpollingSource.startInterval(). All 6 calls occur in these two functions.For the polling source, the new call must be before the first line of the function, because that line polls immediately, and a poll sends an event.
Repeated initialization does not duplicate the handlers
You can start a source two times in one lifecycle, with no shutdown between the two starts: if the initialization fails, a subsequent evaluation runs the fallback chain, and that chain starts the source again. In this condition the handlers must not be added two times.
They are not: the handlers are stable instance properties, and
TypedEmitterkeeps them in aSet. Thuson()with the same handler makes no change.Because of this guarantee, I removed a
sourceEventsWiredflag that I added at first. The flag was not necessary, and no test could show a difference.Other resources that a shutdown releases
I examined the other resources, to find more than one defect. These 3 resources are correct, and I made no change to them:
StreamSource.start()clears its cached promise when the connection stops. Thus it makes a new connection.PollingSource.startInterval()makes a newAbortController.Scheduler.shutdown()clears its pending batch. ThusUsageTrackerbatches again.The event handlers were the only defect. This agrees with the behavior: the connection opens again, but the data has no effect.
Tests
There are 5 new tests:
To count the subscriptions, the tests use the
console.errormessage of a failed poll. This message is the one signal that shows how many times a handler occurs.I did mutation tests on the new tests, to make sure that they can fail:
All 494 tests of the package are successful.
tsc --noEmitis successful.biome checkgives one warning, but that warning is inindex.make.test.ts, a file that this branch does not change.The
CLAUDE.mdfile of the package now gives this information, to prevent a move of the wiring back into the constructor.Specification
https://openfeature.dev/specification/sections/providers#25-shutdown
🤖 Generated with Claude Code