Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… 10) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A second, deeper audit (one reviewer per page + cross-cutting sweeps,
every finding re-verified against packages/*/src) surfaced 92 distinct
issues across 51 pages. Each fix was re-confirmed against source before
editing; 124 edits applied, one PLAUSIBLE-only finding intentionally
skipped. Build + astro check are clean.
Corrects two regressions from the previous docs commit:
- RequestTimeoutError partial set is `partialReplies`, not `replies`
(the prior `err.replies` was undefined and threw) — scatter-gather,
request-reply, reference/bus.
- The retry-count header is the PascalCase wire key `ctx.headers.RetryCount`;
the camelCase `MessageHeaders.retryCount` field is never populated
(headers are cast verbatim in dispatch) — handlers, error-handling.
Recurring root causes fixed across pages:
- Logger calls are message-first `logger.info('msg', { meta })`; flipped
the pino-style `(obj, msg)` examples that did not compile.
- `defaultRequestTimeout` / `DEFAULT_REQUEST_TIMEOUT_MS` are inert:
sendRequest/sendRequestMulti require a positive per-call timeoutMs
(throw ArgumentOutOfRangeError otherwise); publishRequest uses an
inlined 10_000. Documented accordingly; dropped the dead option from
examples.
- `maxRetries` is total deliveries (strict `<`): N ⇒ initial + N-1
retries; maxRetries=1 ⇒ no retries.
- ConsumeContext has no `.envelope` and no top-level `.sourceAddress`
(those live on PipelineContext / ctx.headers.*); left genuine
PipelineContext.envelope usage in filters/middleware intact.
- Documented-but-dead surface reworded to real behavior: bus.stop(signal)
ignores the signal; RabbitMQTopologyMismatchError /
RabbitMQPublishConfirmTimeoutError are never thrown;
publishConfirmTimeoutMs is dormant.
Other notable corrections:
- messages.mdx: unknown message types are classified notHandled and
acked/dropped by default (not "rejected, no silent drop") unless
deadLetterUnhandled + an error queue are set.
- reference/bus/bus.mdx: process builder methods are `startsWith`/`handles`
(the documented startedBy/whenReceived/whenTimeoutFires do not exist);
start() throws after stop(); publishRequest rejects an endpoint with
ArgumentError.
- observability.mdx: wrap telemetry producer at createBus (producer is
readonly); corrected metric instrument names and span naming.
- consume-result.mdx: terminalFailure is set only on the deserialize step;
a handler throw is always non-terminal even for those error types.
- imageserializer/persistence/transport/extension-point pages: corrected
ValidationError (terminal), store keying (dataType, correlationId),
audit semantics (successfully-handled only, off by default), and the
resolved transport defaults.
- Repointed broken in-page anchors (filters, bus-options) to real
headings; softened the reference "covers every exported name" promise
to match actual coverage; samples/migration table corrections.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comments referencing project phases (Phase B/C/D/E) and delivery state
("landed in", "Added in") carried no code-level meaning. Reworded each to
describe only the code. The sequential Step 1..8 dispatch comments stay —
they describe the actual consume flow, not a project phase.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…elemetry conventions Make the Node telemetry package emit telemetry consistent with the C# ServiceConnect.Telemetry / ServiceConnectMeter stack so a mixed C#/Node deployment aggregates onto one schema in a shared backend. Metrics: - Rename to messaging.client.published.messages, messaging.client.consumed.messages, messaging.publish.duration, messaging.process.duration. - Record durations in seconds (was ms); drop explicit bucket advice so both stacks use the OTel SDK default boundaries. - Remove the error counter; carry failure via the messaging.outcome dimension (success/error/retry) and error.type, matching the C# producer/consumer. Spans: - Use messaging.operation.type + messaging.operation.name instead of the deprecated messaging.operation attribute. - Add network.protocol.name, server.address, server.port (new TelemetryOptions fields), messaging.rabbitmq.destination.routing_key, and anonymous-destination handling. - Derive the consume destination from the destinationAddress header or a new queueName option, falling back to an anonymous destination. Scope + headers: - Rename the tracer and meter scope to "ServiceConnect.Bus" to match the C# ActivitySource/Meter name. - Read well-known headers case-insensitively so C# (PascalCase) and Node (camelCase) messages both label spans correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The intro linked to #registerprocessdata and #registerprocess, but the target headings carried full signatures, so their generated slugs didn't match. Simplify the two headings to clean method names (matching the bus.mdx convention); the signatures remain in the code blocks below. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nsport Move metric emission out of the opt-in @serviceconnect/telemetry wrapper into always-on code, mirroring the C# stack (meter in core, emission in the transport, tracing in the telemetry layer). Metrics now flow from a plain transport + bus as soon as an OTel MeterProvider is registered — no wrapper required. core: - Add diagnostics/meter.ts (serviceConnectMeter) hosting the four instruments, bound lazily to the global meter and rebound on identity change so it upgrades from the no-op meter to the SDK meter when a provider is registered later. - Add diagnostics/conventions.ts (OTel attribute keys, metric names, operation and outcome values) and diagnostics/attributes.ts (messagingSystemAttributes, the single shared builder for messaging.system / protocol / server.* tags). - Declare @opentelemetry/api as a regular dependency (the meter uses it at load, so all core consumers need it). rabbitmq: - Producer records publish.duration (always) + published.messages (on success); consumer records process.duration + consumed.messages tagged by outcome derived from the disposition action (ack=success, retry, dead-letter/log=error). - Parse server.address/server.port from the AMQP url; take the first host of a comma-separated cluster URL (matching the C# transport). - Build the constant system/protocol/server tag set once per producer/consumer via the core builder instead of rebuilding it on every message. telemetry: - Reduced to tracing-only: removed the metric instruments and the meter option; applySystemAttributes now delegates to the shared core builder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The telemetry-alignment change replaced the deprecated messaging.operation attribute with messaging.operation.type / messaging.operation.name, but the telemetry example's span-verification smoke check still queried messaging.operation, so it found no producer/consumer spans and failed the Examples smoke CI job. Query messaging.operation.name (send / process) instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reverts the always-on metrics refactor (c151172). Metric emission moves back into the opt-in @serviceconnect/telemetry wrapper (which emits both spans and metrics), and @serviceconnect/core returns to having zero runtime dependencies — no @opentelemetry/api. This deliberately deviates from the C# stack, where metrics are always-on from the core meter: keeping core dependency-free is the priority here. The telemetry C# convention alignment (6a7660e) and the example fix (a00e569) are retained; only the move of emission into core/transport is undone. The e2e telemetry test keeps the corrected consume span name (`<endpoint> process`, from the destinationAddress header) since that span behavior comes from the retained alignment, not from the reverted refactor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e format) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pe identity) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… + inbound) Outbound paths (publish/send/sendToMany/route/sendRequest/sendRequestMulti/ publishRequest) now emit master PascalCase wire headers via encodeWireHeaders instead of stringifyHeaders; correlationId travels in the body, not a header. Inbound, withHeartbeat decodes wire headers once before dispatch, and the dispatcher sources correlationId from the deserialized body so ctx.correlationId keeps working. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stamping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Captures the Phase 1 exit-criterion verification (send + request/reply both directions). Skipped pending a running broker + a C# master fixture service — the only piece not purely in this repo.
…format parity) The handler dispatch branch was already body-sourcing correlationId (master carries it in the body, not a header); the saga branch was not, leaving ProcessContext.correlationId undefined. Mirror the same fix.
…e format - round-trip e2e: a raw transport send carries caller headers verbatim (core owns TypeName/MessageType now), so pass + assert TypeName instead of expecting the transport to stamp MessageType=<type>. - filters example: discriminate on a custom header (noteId) since correlationId moved to the body and incoming filters run before deserialization.
…aster convention) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hestration Adds interop/csharp-fixture (a minimal C# master ServiceConnect responder) and interop/run.sh (broker + fixture + gated Node e2e). The interop-master e2e suite (gated on INTEROP=1) now passes 2/2 against the published C# master: point-to-point sendRequest and publishRequest both round-trip a Ping->Pong, proving type identity, PascalCase body, the FullName-stripped pub/sub exchange, and request/reply correlation in both directions. Validates Phase 1 + Phase 2(a) end-to-end.
…ism model) Switch producer from exchange-to-exchange bind (derived→parent) to the C# master model: publish() fans out to the type's own exchange AND every ancestor exchange via transitive closure of parentsOf, deduped and cycle-guarded. Removes declaredBindings and all exchangeBind calls from the producer path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-publish parity) Multi-publish (5c99747) delivers a derived message to its own exchange AND each ancestor exchange. The bus was binding the consumer queue to every REGISTERED type, so a queue bound to both a type and its ancestor received a derived publish twice. Bind only to CONSUMED types (handlers + sagas + aggregators); types registered only for resolution/deserialization are not bound. Matches C# master. Polymorphic e2e now delivers exactly one copy; sagas/aggregators unaffected.
Replace two-exchange retry routing (Retries.Exchange + MainBack.Exchange) with a single direct durable dead-letter exchange (<q>.Retries.DeadLetter); republish to retry via DEFAULT exchange + retry-queue routing key; keep main queue plain (no retry DLX args); declare error/audit as non-durable direct exchanges bound to durable same-named queues; omit retry topology when maxRetries is 0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The polymorphic stress pattern subscribes to a base type and publishes a derived type. Under the master multi-publish model the producer must fan a derived message out to its ancestor exchanges, which requires parentsOf on the transport. The harness created transports without it, so the derived message only reached its own exchange and the base-type subscriber never fired — the smoke run timed out on the polymorphic pattern in CI. Give each bus its own MessageTypeRegistry and feed that same registry to its transport's parentsOf, so registered parent relationships resolve to ancestor exchanges at publish time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The documentation site was last audited before the wire-protocol alignment work, so several pages described mechanisms that the changes removed or inverted. A fresh source-grounded audit found and this corrects: - envelope.mdx: the "C# wire-format compatibility" section was inverted — it claimed interop was not implemented, that outgoing headers are camelCase, and that inbound type resolution is camelCase-only. In fact the bus encodes PascalCase wire headers (TypeName/MessageType=operation/ConsumerType/Language) outbound and decodes TypeName/FullTypeName inbound; correlationId is a body property, not a header; and Node<->.NET interop is real and CI-tested. - polymorphic-messages.mdx, messages.mdx: polymorphism was described as producer-side exchange-to-exchange bindings. It is producer multi-publish — a derived message is published to its own exchange and every ancestor exchange; no exchange-to-exchange bindings exist. - pub-sub.mdx: the consumer was said to bind to every registered type. It binds only to consumed types (handlers, sagas, aggregators), and exchange names are the FullName with dots stripped. - itransportproducer.mdx, imessageserializer.mdx, options.mdx, migrating doc: note the multi-publish fan-out, the PascalCase/camelCase body transform, and that correlationId is not a stamped header. - observability.mdx, telemetry/index.mdx: corrected stale metric names to the actual messaging.* instruments (no errorCount counter; consumed-messages is tagged by messaging.outcome). - options.ts: fix the parentsOf JSDoc that still described exchange-to-exchange bindings (the source the stale prose was drawn from). astro check + astro build pass clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the same comment-hygiene rule the C# repo enforces: source comments (production + test) and user-facing docs describe the code as it stands, not the roadmap phase or work stream that produced it. Removed: - topology.ts: "(Phase 1)" aside on the FullName convention - harness/stress chaos: "Phase H" from the placeholder doc comment and its log message - core errors test: renamed the "Phase E errors" describe block to "routing-slip and stream errors" (what it actually covers) - interop-master e2e: dropped "(Phase 1 + Phase 2a)" from the proof header - options.mdx: dropped "Added in Phase D." from the signal option Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
….0.0) The published legacy `service-connect` latest is 2.0.0 (npm `latest` dist-tag), so "migrating from v1" / "v1.x" / "v1.0.12 is the final release" / "Node 16+" were all inaccurate. Relabel the migration guide and references to v2: - Rename migrating-v1-to-v3.mdx -> migrating-v2-to-v3.mdx (slug change) and relabel every v1 reference in it to v2. - Fix the version facts: the legacy latest is 2.0.0 (releases.mdx), and v2 requires Node 18+ (service-connect@2.0.0 engines: node >=18), not Node 16. - Update the links to the new slug in README, the Starlight sidebar (astro.config.mjs), and releases.mdx. Left as-is: the `urn:orders:placed:v1` message-type-name example and the generic `v1.0.0` git-tag-format examples in the release-process notes — neither refers to the legacy package version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
master's 70 commits (SSL/TLS, robustness fixes, request/reply overhaul) live entirely in the old single-package architecture (src/, test/, integration-test/). The v3 rewrite replaces that architecture with the pnpm/turbo monorepo, so none of those files carry over — the merged tree is v3's tree. Recorded as a merge so master is an ancestor of v3 and the release guard's master-ancestry check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
Full rewrite of the ServiceConnect Node.js client as a TypeScript pnpm/turbo monorepo, mirroring the C# client's feature set and aligning on the shared wire format. Replaces the legacy single-package client.
411 files changed, +32,364 / -11,237across the new package set.What's included
@serviceconnect/core— bus, message handling, pipeline, retry/error/audit flow@serviceconnect/rabbitmq— RabbitMQ transport (topology, consumer concurrency, aggregator, routing slip)@serviceconnect/persistence-memory,@serviceconnect/persistence-mongodb— process-manager/saga persistence@serviceconnect/healthchecks,@serviceconnect/telemetryWire compatibility
Node and C# v7 align to the C# master wire format (PascalCase,
FullTypeNameoptional,FullName-stripped exchange), so this client interoperates with existing C# services.Release
Merging to
masterunblocks the release pipeline (.github/workflows/release.yml), which is gated to master and publishes every@serviceconnect/*package on av*tag push.Test plan
🤖 Generated with Claude Code