Skip to content

v0.1.21

Choose a tag to compare

@github-actions github-actions released this 09 Sep 16:26
· 9 commits to main since this release

fix: an unreleased subscription is an obligation, retryable and blocking

A client can register the callback and THEN fail, and the transport's own
undo can fail too. Relay forgot its handler on the error and had nothing to
point at, so the next start subscribed beside a listener still live: two
handlers, every message twice, and only the newer nameable at shutdown. The
previous round covered a subscription already established; this covers a
first subscribe that half succeeded.

Both failure paths — a shutdown that could not unsubscribe, and a subscribe
whose undo failed — now record a RETRYABLE release. A settled rejected
promise would refuse every start for the life of the process, and the bus
recovering is exactly the case that has to work: the start retries the
release, clears the obligation on success, and refuses while it stands.

RedisRelayTransport keeps every wrapper per handler rather than one.
Subscribing the same callback twice registers two listeners with the client;
keeping one meant the second overwrote the only way to name the first, and an
unsubscribe removed one of the two. The class is exported, so this was not
limited to relay's own use of it.

And disconnect() removes the listeners it put on the bus whoever owns the
connection. A borrowed one is not relay's to close — but the listeners are
relay's to remove, and returning early left a shutdown that released nothing
at all on a connection the application shares.

Four mutations, each falling on its own test.

fix: a failed teardown stops the next start, and a failed subscribe is undone

Two ways relay could end up with two handlers on one bus.

startTransport() waited for a teardown in progress with
await teardown.catch(() => {}) — so a shutdown whose unsubscribe had FAILED
was treated as done, and the start subscribed beside a handler still
listening. Every message was then delivered twice, and the next shutdown
could only name the newer one. The start now refuses, and says why: a relay
that cannot release its previous subscription cannot know what is still
listening. #transportHandler is kept when the removal was refused, so a
later shutdown still has something to name, and the retry clears the state.

RedisRelayTransport.subscribe recorded the wrapper before calling the
client and forgot it on failure. A client is free to register the callback
and THEN fail — an ambiguous network result, or a duck-typed adapter that
does its bookkeeping first — and the wrapper was then live and unnameable.
The failure path takes it back off; if that fails too the record stays,
because it is the only way a shutdown can name what may be listening.

Two existing tests asserted the client was never reached after a failed
subscribe. They now distinguish the two moments: the failure path DOES reach
it once, to undo what it may have done, and a later unsubscribe still adds
nothing.

Three mutations, each falling on its own test.

fix: a restart must outlive the shutdown it overlapped, and bookkeeping must survive a refusal

Two findings, both about state that outlives the call that owns it.

shutdown() checks the generation BEFORE it awaits disconnect(), so a
restart landing during that await passes a check that has already happened —
and the old shutdown goes on to close the connection the new generation has
just subscribed on. Relay then reports itself started, with a live handler,
on a transport nobody can reach. A start now waits for a teardown still in
progress.

I removed this guard in the previous round because I could not make it fall
a test. It falls one now: hold the transport's disconnect, restart during
it, and the restart's client receives nothing.

RedisRelayTransport recorded the wrapper before calling the client and
forgot it only when quasar REPORTED a failure. A direct rejection left it
recorded — so a later unsubscribe asked the client to remove something it may
never have registered, and a retry was refused by bookkeeping for a
subscription that never happened. The other direction had the mirror defect:
the record was dropped before the client was asked, so a rejecting
unsubscribe left a live subscription nothing could name again, neither to
retry nor to remove at shutdown. Each entry is now forgotten only once the
client has accepted its removal, one at a time.

Three mutations, each falling on its own test.

fix: name the handler, because the bus stacks subscriptions

The generation fix rested on an assumption I wrote down and did not check:
that the transport holds one handler per channel, so a superseded start
attempt had to leave its subscription alone rather than take down the live
one. The bus relay actually runs on does the opposite — @c9up/quasar
stacks handlers in a Set per channel, matching upstream — and my test
hid it by modelling the transport as a Map, where the second subscribe
silently replaced the first.

With the real shape, a superseded attempt left its handler delivering
alongside the live one: every broadcast reached each client twice, for the
life of the process.

RelayTransport.unsubscribe now takes the handler, as quasar's own does.
RedisRelayTransport keeps a wrapper per registration instead of per
channel, so it can remove exactly one. Each start attempt owns its handler
and undoes that one when it turns out to have been superseded.

shutdown() unsubscribes only a handler it has live. Asking for "every
handler on this channel" silenced another Relay sharing the same bus, and
there is nothing left to sweep now that every attempt cleans up after
itself.

RedisRelayTransport.disconnect() forgets the connection before closing
it. Cached, the quit client was handed straight to the next subscribe, so
an application that stopped and started again in one process came up on a
socket that was already shut. And shutdown() skips the disconnect when a
newer generation has started, rather than closing the bus underneath it.

The test bus now stacks handlers like quasar's. Four mutations, each
falling on its own tests. A fifth guard — serialising a restart behind the
teardown — could not be made to fall anything: with the generation check,
the named handler and the invalidated connection, every interleaving is
already covered, so it is not in the patch.

fix: a start in flight must not be undone by, or undo, a shutdown

A subscribe is asynchronous, so a shutdown or a restart can land while one
is still under way. Two things went wrong when it did.

A subscription that completed AFTER a shutdown stayed on the bus. The
shutdown had already looked for a handler to remove and found none, so the
handler that landed a moment later was untracked: it kept re-delivering
every remote broadcast to streams the shutdown was meant to release, and no
later shutdown could name it either.

A failure reported after a restart tore down the attempt that replaced it.
The catch cleared the NEW attempt's ping timer and marked the relay stopped
while its subscription was live, so the next start subscribed a second time
to a channel it was already on and every broadcast arrived twice.

Both are the same missing notion: which attempt owns the transport.
#transportGeneration is incremented by every start and every stop, and an
attempt only touches shared state — or undoes its own subscription — while
it is still the generation that owns it.

shutdown() deliberately does NOT wait for an in-flight start: moving the
generation on is what tells that attempt to take its own subscription back
down, and waiting instead would hang a graceful shutdown on exactly the
unreachable bus that makes shutting down urgent.

Four tests, each falling on its own guard: removing the post-subscribe
generation check fails the two shutdown tests and only those; removing the
guard in the catch fails the two restart tests and only those.

fix: a failed or stopped transport must leave relay startable

#transportStarted was set BEFORE the subscribe, so a failure was permanent:
every later call handed back the same rejected promise without ever trying
again, and the ping timer kept running for a bus this instance was not on. A
Redis that came back could not be rejoined.

Nothing was reset on shutdown either, so an application that stopped and started
again in one process — a hot reload, a test — sat silently off the bus while
believing it had subscribed.

A failed start now undoes itself but KEEPS its rejection: startable again is not
the same as nothing happened, and a provider deciding whether to boot needs the
reason. whenTransportReady() refuses to answer at all before a start, which is
the one answer it must never give — it resolved immediately, contradicting the
contract it documents.

And disconnect() runs in a finally: an unsubscribe that rejected skipped it
and leaked the connection relay owns.

docs: the preloads run AFTER the providers start, not before

Upstream's warm-up is providers.start() -> the 'starting' hooks -> the preloads.
These comments said the opposite and placed advice on it.

fix: open the bus in ready(), and never unsubscribe what was never subscribed

Constructing a Relay opened a Redis socket and started a timer. register,
boot and start all run when an application is assembled to be INSPECTED — a
route listing, a codegen pass — and shutdown does not run on that path. So
ream inspect opened a connection nothing would ever close. Upstream reserves
ready for sockets and background work; startTransport() is called from there,
it is idempotent, and the constructor now does no I/O at all.

unsubscribe reached the client even with nothing to remove. Passing no
handler means "drop every listener on this channel" on a shared connection, so
calling it when relay never subscribed — after a failed ready(), or on a
second shutdown — cut the cache's and the sessions' listeners instead of
relay's. It now returns before touching the client, which also stops a shutdown
opening a connection just to disconnect it.

fix: stop destroying Quasar resources relay does not own, and surface a dead bus

Three defects, all in how relay treats a connection it shares.

It removed every listener on the channel. unsubscribe(channel) with no
handler means "drop everything here" -- on a shared @c9up/quasar connection that
silenced the application's own subscriptions the moment relay stopped. The
wrapper actually registered is now kept per channel and named on the way out.

It closed the whole connection. quit() on a quasar connection closes BOTH
its sockets, so shutting relay down took the cache, the sessions and the queues
with it. Ownership is now explicit and defaults to false: a named connection is
borrowed by construction and the flag cannot override that.

A dead bus was invisible. The subscription was begun in the constructor and
nothing waited on it, so a transient Redis failure at start-up left the instance
serving its own clients while missing every broadcast published elsewhere -- a
split-brain the application reported as healthy. Worse, quasar REPORTS a failed
subscribe through a callback and resolves normally, so the .catch() never even
ran. The transport now turns that report back into a rejection, and the provider
awaits readiness in ready(), the last phase that can still refuse to boot.

And registerRoutes() mounts once. Mounting is not idempotent on the router: a
second call added the three endpoints again and could not change the ones
already built. A repeated call is now a no-op, and a customizer arriving too
late says so instead of being silently dropped.

docs(provider): the ready() note said hub where the guard now says hub or channel

fix(provider): warn for a channel authorizer too, and stop promising automatic routes

Since 0.1.20 the SSE endpoints exist only once the application calls
registerRoutes(), and the warning in ready() is the only thing standing between
that upgrade and a silent E_ROUTE_NOT_FOUND. It looked at mounted hubs alone.

The shape simple use takes -- authorize() then broadcast(), which is what the
documentation shows -- mounts no hub. Such an application went from working
realtime to a 404 with nothing logged anywhere, which is precisely the case the
warning exists to prevent. It now fires on anything that presumes the endpoints,
and names what was actually declared rather than announcing a hub to someone who
never wrote one.

authorizedChannels() is the sibling of mountedHubs(), @internal like it, so
the guard reads the declaration without the provider reaching into a private Map.

The header doc-comment still said the provider "auto-binds the three SSE routes".
That is the first thing read when opening this file to find out why the route is
missing, and it sent the reader the wrong way. It now says the provider installs
a mounter and that the routes come from registerRoutes().


Changes since v0.1.20.