fix: register application actors in every process - #53
Merged
Conversation
An actor registers itself as a side effect of its class loading, and only the solid_objects start process loaded the host application's app/actors. A lazily loading web process therefore booted with an empty registry. ActorChannel#subscribed resolves the actor by name, so UnknownActorType reached the rescue that rejects the subscription. A Passenger worker that had not yet rendered that actor rejected a valid subscription for an actor it could serve, and the page kept a card that never updated. ComponentsController resolves the same way through ActorSnapshot. Transmission.receive already carried a registry-miss retry, added in 0.14.0 because "a lazy-loading web process has no other reason to have loaded the target class"; the same miss went unhandled on the Cable path. The engine now loads app/actors from a to_prepare hook, which is what a host would otherwise have to write in an initializer. The dummy app, which sets eager_load = false, is the harness: a boot that names no actor class reports the actor as registered. The reject was also silent. Five conditions collapsed into one bare reject, which closes the socket without a log line, so an operator cannot tell an unregistered actor type from a tampered token. Every path now emits solid_objects.subscription.rejected with a reason, the actor identity, and the error class where an exception caused it. Exception messages stay out, because a component or payload failure can carry actor state. The reporting helper is reject_and_report, not reject_subscription: ActionCable::Channel::Base already defines reject_subscription and calls it with no arguments from subscribe_to_channel, which the test suite caught as ArgumentError across seven cases. Bumps the version to 0.14.1 with the changelog and the lockfile.
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.
The defect
An actor registers itself as a side effect of its class loading. Only the
solid_objects startprocess loaded the host application'sapp/actors:ApplicationActorLoaderhad exactly two callers,cli.rbandtransmission.rb. The engine registered four initializers (configuration,database, helpers, assets) and none of them installed the loader.
So a lazily loading web process boots with an empty registry.
ActorChannel#subscribedresolves the actor by name:and
UnknownActorTypeis one of the five exceptions in the rescue at thebottom of the method, which calls a bare
reject. A Passenger worker that hadnot yet rendered that actor rejected a valid subscription for an actor it was
able to serve, and the page kept a card that never updated.
ComponentsControllerresolves the same way throughActorSnapshot.The 0.14.0 changelog names the same hazard on the ingest path: a registry-miss
retry was added to
Transmission.receive"because a lazy-loading web processhas no other reason to have loaded the target class." The Cable path never got
the equivalent.
The failing test
The dummy application sets
config.eager_load = false, which makes it theright harness.
test/dummy/actor_registry_check.rbboots it in a separateprocess and names no actor class, the way nothing in a fresh web process names
one until a request does. Before the fix:
The fix
That is the initializer a host would otherwise have to write itself, moved
into the engine.
to_preparealso means a development reload re-registers areplaced actor class, which is why
ApplicationActorLoader#installalreadyused it for the worker.
Transmission.receivekeeps its retry as a guard for a host that reaches thegem without the engine.
The second defect: the reject said nothing
Five conditions collapsed into one bare
reject. A rejection closes thesocket, so the browser cannot report which one fired, and nothing was logged
or instrumented. That is the reason this took so long to find from the
outside.
Every reject path now emits
solid_objects.subscription.rejectedcarrying areason, the actor identity, and theerror_classwhere an exception causedit:
unregistered_actor_typeUnknownActorType, the defect aboveinvalid_stream_tokeninvalid_component_tokenmalformed_component_registrationJSON::ParserErroronparams["components"]missing_subscription_parameterKeyError, no tokenunauthorizedauthorize_subscriptionreturned falseException messages stay out of the payload, matching the existing rule on
payload_broadcast_failed: a component or payload failure can carry actorstate, and a log line is the wrong place for it.
One implementation note worth recording. The helper is named
reject_and_report, notreject_subscription, becauseActionCable::Channel::Basealready definesreject_subscriptionand calls itwith no arguments from
subscribe_to_channel. Shadowing it turned sevenexisting tests into
ArgumentError: wrong number of arguments (given 0, expected 1; required keywords: actor_type, actor_id), which is how thecollision surfaced.
Host-side workaround, no longer needed after this ships
Also in this PR
CHANGELOG.mdandGemfile.lock, matchingsolid-objects-js 0.14.1.
README.mdanddocs/operations.mdnow say the engine loadsapp/actorsin every process that boots the application, not only the CLI, and
docs/operations.mddocuments the new event and its reasons.docs/roadmap.mdrecords the discovery next to the payload-block precedentin the same paragraph: actor registration in a web process was assumed
rather than arranged.
Effects
process is now served.
authorize_subscriptionstill runs on everysubscription and still denies by default; the new event adds actor identity
and an error class to logs, and deliberately no exception messages.
initializer above can drop it, and keeping it is harmless.
Validation
bundle exec rake(test, standard, rubocop, rbs, steep, security): pass,581 runs, 1898 assertions, 0 failures, 0 errors, 15 skips.
origin/mainand passes here.