chore: NATS broker for micro services by sampaiodiego · Pull Request #35305 · RocketChat/Rocket.Chat

sampaiodiego · 2025-02-21T22:07:36Z

https://rocketchat.atlassian.net/browse/RDI-278

Proposed changes (including videos or screenshots)

This is a proof of concept. It adds NatsBroker, a second network broker for the microservices deployment. It talks to NATS directly instead of going through Moleculer. It uses the NATS services protocol for calls and discovery, and plain subjects for events. Moleculer stays the default. BROKER=nats selects the new broker in every service and in the monolith running in microservices mode.

Broker selection

startBroker() (ee/packages/network-broker/src/startBroker.ts) picks MoleculerBroker or NatsBroker from BROKER. The NATS factory (nats.ts) reads NATS_URL, falling back to TRANSPORTER, plus REQUEST_TIMEOUT and BROKER_LOCAL_ROUTING.
Api.setBroker() now throws once the API has started, so a broker can no longer be swapped under running services.

What `NatsBroker` implements

Calls: rpc.<service>.<method>, load balanced by a NATS queue group. node.<nodeID>.<service>.<method> pins a call to one instance and backs CallingOptions.nodeID. A service in the same process is called directly, so arguments keep their identity instead of going through EJSON. BROKER_LOCAL_ROUTING=false turns that off.
Events:
- broadcast publishes on event.<name> and reaches every node.
- emitToOne publishes on emit.<name>, where each service subscribes in a queue group named after itself, so one instance of each listening service receives it.
- broadcastLocal stays in process.
- Events and methods live under different prefixes, because some event names collide with <service>.<method>, e.g. accounts.login.
Errors: a throwing handler answers with a service error, and the caller rethrows it. MeteorError keeps its error, reason, details and isClientSafe.
Discovery: $node.list and $node.services are answered from a $SRV.PING round trip, with a short cache. Each service carries its node id as metadata. License enforcement uses that list, the same way it does under Moleculer.
Startup:
- A service is registered only once its dependencies (settings and license by default) are reachable locally or remotely.
- Services whose dependencies are met together are registered together, before their created()/started() hooks run.
- A failing hook is logged instead of rejecting api.start().
Retries: a request that reaches no responder (503) is retried with backoff for about 4s, which covers peers that are still booting or being rolled. Nothing else is retried, because a timed-out request may already have run.
Streams: a Readable in a call's arguments or its result is replaced by an inbox subject. The other side pulls 256KB chunks from that subject, which provides backpressure. An idle timeout cleans up the subject when the consumer never pulls.
Context: hooks, remote handlers and local calls all run inside asyncLocalStorage, so ServiceClass.context is populated. Under NATS, ddp-streamer's login handler depends on this.
Shutdown: on SIGTERM/SIGINT every service runs stopped(), then the connection drains and the process exits. SKIP_PROCESS_EVENT_REGISTRATION=true disables this.

Shared with `MoleculerBroker`

The default-dependencies rule now lives in withDefaultDependencies.ts, used by both brokers.
startBroker from moleculer.ts is renamed to startMoleculerBroker.

Tests and docs

NatsBroker.test.ts runs the broker against an in-memory FakeNatsConnection. It covers calls, events, emitToOne, errors, retries, dependency waiting, lifecycle failures, streams, context and shutdown. nats.test.ts covers the signal handling.
docs/service-brokers.md describes all three brokers, everything that had to be rebuilt for NATS, how events reach other instances in each deployment, what is not implemented yet, and the configuration.

Not done yet

Warning

Tracing, metrics and namespaces are not implemented. Switching to BROKER=nats silently loses all three. See "Not implemented yet" in docs/service-brokers.md.

Tracing: the caller's trace context is not sent with a call, and handlers do not run in a tracer span, so a trace stops at the first remote call.
Metrics: broker.metrics is undefined. ddp-streamer's rocketchat_subscription, users_connected and users_logged are not recorded, and no Prometheus reporter is exported (no MS_METRICS equivalent).
Namespaces: there is no MS_NAMESPACE equivalent. Subjects and service names are global to the NATS server, so two deployments on one server would answer each other's calls, receive each other's events, and discover each other's services. Until this lands, give each deployment its own NATS account.

Also missing:

broadcastToServices is a no-op.
$node.connected / $node.disconnected are never emitted. Presence relies on onNodeDisconnected to drop the sessions of a node that died.
$services.changed and the other broker lifecycle events are never emitted. Nothing in the codebase listens to them today.

Issue(s)

Steps to test or reproduce

Unit tests: yarn testunit in ee/packages/network-broker.
End to end: the CI e2e workflow has a broker input that is passed to docker compose as BROKER. The workflow checks that the requested broker actually started. The input defaults to nats on this branch so CI exercises the new broker. That default has to go before merge.
Locally: start the microservices compose with BROKER=nats.

Further comments

The nats dependency was bumped to ^2.29.1 in every app so the repo keeps a single version.

🤖 Generated with Claude Code

dionisio-bot · 2025-02-21T22:07:40Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is still a draft
This PR is missing the 'stat: QA assured' label
GitHub is still computing mergeability — this will refresh shortly
This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2025-02-21T22:07:40Z

⚠️ No Changeset found

Latest commit: d82927a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

codecov · 2025-02-21T22:15:47Z

Codecov Report

❌ Patch coverage is 93.84615% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.33%. Comparing base (a6ae198) to head (d82927a).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #35305      +/-   ##
===========================================
+ Coverage    71.27%   71.33%   +0.05%     
===========================================
  Files         4861     4866       +5     
  Lines       207137   207663     +526     
  Branches     36571    36303     -268     
===========================================
+ Hits        147631   148130     +499     
- Misses       54382    54415      +33     
+ Partials      5124     5118       -6

Flag	Coverage Δ
e2e	`58.94% <ø> (+0.02%)`	⬆️
e2e-api	`45.87% <ø> (-0.02%)`	⬇️
unit	`72.13% <93.84%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ggazzo · 2025-02-26T20:55:00Z

@kody start-review

kody-ai · 2025-02-26T20:55:12Z

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration

Interacting with Kody

Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.
Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Security	✅
Code Style	✅
Kody Rules	✅
Refactoring	✅
Error Handling	✅
Maintainability	✅
Potential Issues	✅
Documentation And Comments	✅
Performance And Optimization	✅

Access your configuration settings here.

coderabbitai · 2026-04-15T21:41:12Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

rc-layne · 2026-04-28T22:07:04Z

⚠️ Layne — scan incomplete

Layne could not analyze all changed content. Review the Check Run summary before merging.

sampaiodiego · 2026-09-23T20:03:07Z

 		this.services.forEach((service) => this.broker?.createService(service));
+
+		if (this.started) {
+			void this.broker.start();


why does it need to be started here?

NatsBroker awaited `created()`/`started()` directly, so a service whose hook rejected rejected `api.start()` too. Nothing catches that, so the unhandled rejection killed the container - silently, taking every other service hosted in the same process with it. Endpoints are already registered before any hook runs, so a service that fails to initialize can still answer with its defaults. Log the failure and keep going instead.

Moleculer holds a registry and waits for a service to appear; nats has no such thing, so a request to a subject nobody listens on fails at once with 503. That is the normal state of affairs while a peer boots or is rolled, and every caller was left to handle it - or not, as the crash on startup showed. Back off and retry for a few seconds. Only 503 is retried: it means nothing received the request, so a second attempt cannot duplicate a side effect. A timeout may have been delivered, so it still surfaces to the caller. `isNatsError` is declared by the nats typings but not exported at runtime, so the check uses `NatsError` itself.

Services that read configuration from another service need to do it on first use rather than at boot, and need a failed attempt to be retried rather than remembered. Three of them are about to want exactly that.

`started()` fetched `Accounts_LoginExpiration` from the settings service, which runs in another process. During a cold start it is not reachable yet, so the read threw and took the whole account-service down with it. The field is already kept current by `onSettingChanged`; the boot read only primed it. Prime it on the first login instead, where a transient failure fails that one login rather than the process.

`started()` fetched seven settings from the settings service, which runs in another process. During a cold start it is not reachable yet, so the read threw and took the whole omnichannel-transcript-service down with it. All seven fields already have defaults and are kept current by `onSettingChanged`; the boot read only primed them. Prime them on the first transcript instead, so a transient failure fails that one job - which the queue already retries - rather than the process.

`started()` read four settings from the settings service, which runs in another process. During a cold start it is not reachable yet, so the read threw and took the whole authorization-service down with it - and even once that is survivable, it left the service unconfigured: `this.pdp` stays null, and `checkUsernamesMatchAttributes` silently skips the check in that state. Keep the boot read, but make it repeatable and run it again on the two decision paths, so a service that came up before the settings service configures itself on the first decision rather than staying open until a setting happens to change. The read now only fills in what is still unknown, and leaves an already chosen PDP alone: settings events arrive independently of it, so whatever they delivered is newer than what it just fetched.

Moleculer dispatches a call to a locally registered service straight to the instance, without going near the serializer, and call sites depend on it: `ee/server/configuration/abac.ts` hands `LDAPEnterprise` a mongo cursor, which only survives because the argument arrives by reference. NatsBroker sent everything over nats, so EJSON met the cursor and threw `Converting circular structure to JSON`. Route through a LocalServiceRegistry that tracks the services running in this process. Calls to them keep argument and result identity, and skip a round trip through the nats server on the way. `getCallableMethods` now defines what a service answers to in one place, so the local and remote paths cannot expose a different set. Set BROKER_LOCAL_ROUTING=false to turn it off and send everything over the wire again.

LocalBroker already indexed its services by `<service>.<method>` and dispatched straight to the instance - the same thing NatsBroker just grew, down to its own copy of the method enumeration in both createService and destroyService. Move LocalServiceRegistry and getInstanceMethods into core-services, which both brokers already depend on, and let LocalBroker route through the registry too. MoleculerBroker takes getInstanceMethods from the same place. This also stops LocalBroker exposing lifecycle hooks and event handlers as callable: `api.call('some-service.started')` used to re-run the hook. Nothing calls those through a broker.

Covers all three implementations rather than only the new one: how a proxied call reaches a broker at all, what LocalBroker and MoleculerBroker each provide, and what NatsBroker had to rebuild to match.

Priming only covered the two decision paths, but five room attribute mutators and four other entry points read `this.pdp` as well. In microservices mode the monolith does not register AbacService at all, so every call lands on authorization-service - whose `started()` loses the race with the settings service and leaves `this.pdp` null. `ensurePdpAvailable` then reported the PDP as unavailable rather than as not yet configured, and the ABAC API answered 400 `error-pdp-unavailable` to 35 requests. Prime at `ensurePdpAvailable` instead, which is the chokepoint all five mutators already go through, and at the entry points that read `this.pdp` without it. `computeEffectiveStoreType` primes too, so attribute store selection stops depending on whether the boot read happened to succeed - that was only left out earlier because `started()` was gone at the time, and with it back the extra call is a memoised no-op. Two specs mocked core-services without `Settings`, which only showed up once those paths started reading it.

…st use NatsBroker ignored the dependencies every service declares - `settings` and `license` implicitly, plus any passed to `api.registerService` - so services that read configuration in `started()` lost the race with the monolith. The workaround was `primeOnce`: read on first use, retry on failure. That changed when and where those services configure themselves compared to Moleculer. Reproduce Moleculer's `waitForServices` instead. A service is registered, and its hooks run, only once every dependency is available: a local one as soon as it registers, a remote one through discovery every second. It waits forever by default, like Moleculer; a `dependencyTimeout` constructor option bounds the wait, which the tests use so that an unmet dependency fails fast instead of hanging. With that in place, revert the priming in Account, OmnichannelTranscript and AbacService back to their boot reads, and delete `primeOnce`. ddp-streamer now declares `meteor` as a dependency, since the client versions it publishes come from MeteorService, and loads them in `started()` again - still after the socket server is listening, and still non fatally. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

IBroker gained emitToOne on develop (#42415) for domain events that one instance of each listening service must handle, such as room.user-activity reaching FederationMatrix. Each service now also subscribes to emit.<event> in a queue group named after itself, so NATS delivers each emit to exactly one of its instances. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

As PID 1 in a container node ignores SIGTERM without a listener, so under NatsBroker every container was killed at the end of the grace period: no service ran stopped(), nothing was drained, and exit hooks such as the coverage writer never ran. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kody-ai Bot reviewed Feb 26, 2025

View reviewed changes

Comment thread ee/packages/network-broker/src/NatsBroker.ts Outdated

kody-ai Bot reviewed Feb 26, 2025

View reviewed changes

Comment thread ee/packages/network-broker/src/moleculer.ts

kody-ai Bot reviewed Feb 26, 2025

View reviewed changes

Comment thread ee/packages/network-broker/src/NatsBroker.ts

kody-ai Bot reviewed Feb 26, 2025

View reviewed changes

Comment thread ee/packages/network-broker/src/MoleculerBroker.ts

sampaiodiego force-pushed the nats-broker branch from 3b2cc8e to 6d69c4a Compare April 15, 2026 21:41

sampaiodiego force-pushed the nats-broker branch from 6d69c4a to 7b81a9c Compare April 15, 2026 21:57

sampaiodiego force-pushed the nats-broker branch from f48f4b5 to 05ab9ff Compare April 28, 2026 21:01

rc-layne Bot added the needs-security-review label Apr 28, 2026

sampaiodiego force-pushed the nats-broker branch 2 times, most recently from 9e2d731 to f6952ba Compare August 24, 2026 21:03

sampaiodiego force-pushed the nats-broker branch from 69d51e9 to 38cd813 Compare September 1, 2026 20:08

ggazzo force-pushed the nats-broker branch from 38cd813 to a49da52 Compare September 22, 2026 13:32

sampaiodiego force-pushed the nats-broker branch from a49da52 to 190dcf4 Compare September 23, 2026 19:57

sampaiodiego commented Sep 23, 2026

View reviewed changes

This was referenced Sep 23, 2026

refactor: prepare network-broker for a second broker implementation #42335

Merged

fix: ddp-streamer not listening when client versions fail to load #42336

Merged

fix: livechat transcript uploaded truncated to one of its rooms #42337

Merged

sampaiodiego force-pushed the nats-broker branch from 4554582 to 8f73eb7 Compare September 24, 2026 22:13

sampaiodiego mentioned this pull request Sep 28, 2026

fix: route stream fan-out through the service broker #42415

Merged

sampaiodiego force-pushed the nats-broker branch 2 times, most recently from 9bff77f to b501542 Compare September 30, 2026 04:10

sampaiodiego added 2 commits October 8, 2026 19:24

create helper function

b5a7264

new nats broker

bb2c2d0

sampaiodiego and others added 28 commits October 8, 2026 19:40

use nats broker on CI (remove before merge)

9a0bd20

add primeOnce, a lazy initializer that does not cache failures

e337663

Services that read configuration from another service need to do it on first use rather than at boot, and need a failed attempt to be retried rather than remembered. Three of them are about to want exactly that.

document the nats broker

fd31d68

document the service brokers

dedf72e

Covers all three implementations rather than only the new one: how a proxied call reaches a broker at all, what LocalBroker and MoleculerBroker each provide, and what NatsBroker had to rebuild to match.

document the two priming mistakes the nats broker surfaced

401bac6

add streams and context

95148ae

document emitToOne and the broadcastToServices gap on NatsBroker

856ba07

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs: improve service broker doc

d6c44ac

document graceful shutdown on NatsBroker

f8a4e37

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

drop Moleculer comparisons from NatsBroker comments

639cae8

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

remove comment

4c0e41e

reject setBroker once the API has started

34b4f94

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

share the default service dependencies between the network brokers

d6340b4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

read the NatsBroker environment in its factory, not in the class

36e6966

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

drop NatsBroker state derivable from its connection and dependencies

8298227

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

copy a payload only when it carries a stream

d113c31

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs: LocalBroker is installed only outside microservices mode

77dda08

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sampaiodiego force-pushed the nats-broker branch from 876ba6c to 77dda08 Compare October 8, 2026 23:02

docs: note that NatsBroker has no tracing, metrics or namespaces

d82927a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Navigation Menu

Repository navigation

chore: NATS broker for micro services - #35305

chore: NATS broker for micro services#35305
sampaiodiego wants to merge 44 commits into
developfrom
nats-broker

sampaiodiego commented Feb 21, 2025 •

edited

Loading

Uh oh!

dionisio-bot Bot commented Feb 21, 2025 •

edited

Loading

Uh oh!

changeset-bot Bot commented Feb 21, 2025 •

edited

Loading

Uh oh!

codecov Bot commented Feb 21, 2025 •

edited

Loading

Uh oh!

ggazzo commented Feb 26, 2025

Uh oh!

kody-ai Bot commented Feb 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 15, 2026 •

edited

Loading

Draft PR not reviewed

Uh oh!

rc-layne Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

sampaiodiego Sep 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sampaiodiego commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes (including videos or screenshots)

Broker selection

What NatsBroker implements

Shared with MoleculerBroker

Tests and docs

Not done yet

Issue(s)

Steps to test or reproduce

Further comments

Uh oh!

dionisio-bot Bot commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot Bot commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

codecov Bot commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ggazzo commented Feb 26, 2025

Uh oh!

kody-ai Bot commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Completed! 🔥

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Draft PR not reviewed

Uh oh!

rc-layne Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sampaiodiego Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sampaiodiego commented Feb 21, 2025 •

edited

Loading

What `NatsBroker` implements

Shared with `MoleculerBroker`

dionisio-bot Bot commented Feb 21, 2025 •

edited

Loading

changeset-bot Bot commented Feb 21, 2025 •

edited

Loading

codecov Bot commented Feb 21, 2025 •

edited

Loading

kody-ai Bot commented Feb 26, 2025 •

edited

Loading

coderabbitai Bot commented Apr 15, 2026 •

edited

Loading

rc-layne Bot commented Apr 28, 2026 •

edited

Loading