Repository navigation
Conversation
PublishBatch always returned an empty PublishResponse regardless of the
batch's outcome. The node-side batch client
(chainlink-common/pkg/chipingress/batch) requires one PublishResult per
event to resolve delivery: since chainlink-common#2326 an empty results
array is reported to every event's callback as ErrCodeResultsMismatch,
so a chainlink v2.67 node's durable emitter against this router treated
every event as undelivered and retransmitted the whole stream every 60s,
forever — measured at 301k 'failed to deliver event. Relying on
retransmit.' warnings on a single node within minutes of startup, a ~20x
duplicated Kafka topic, and metric delivery-latency gaps long enough to
break downstream freshness gates.
- Ack each event (PublishResult{EventId}, nil error) when the batch was
handed to at least one subscriber; the events were forwarded in that
case, so the caller can resolve delivery and stop retransmitting.
- Report a per-event PublishError when no subscriber accepted the batch,
so the caller retains and retries (at-least-once preserved).
- Return Unavailable when no subscribers are registered instead of an
empty success — the previous behavior acked events that went nowhere,
letting a durable store delete them undelivered (the data-loss shape
flagged by the audit behind chainlink-common#2326).
Bumps the module's chainlink-common/pkg/chipingress pin (Dec 2025 ->
Sep 2026) — the old pin's pb predates PublishResult.Error.
…PC when nothing accepts Addresses the review findings: - A nil downstream RPC error does not mean every event was accepted — the protocol supports per-event rejections in a successful response. The router now aggregates the subscribers' per-event results and passes a rejection through only while no subscriber has accepted the event. Subscribers that accept without per-event detail (nil-error response, empty results array) count as accepting the whole batch — the router trusts a subscriber's RPC success at the forwarding boundary, the same as Publish does. - When no subscriber accepts the batch, return Unavailable instead of a nil-error response with per-event errors: transactional callers resolve the whole batch from the RPC outcome alone, so per-event errors would still acknowledge everything. An RPC error makes every caller mode retain and retry.
…on fails the RPC A subscriber's nil RPC error does not mean its events were accepted: the pinned protocol reports per-event rejections in a successful response. Renamed the flag to anyForwarded (what an RPC success actually tells the router) and gated the response on the batch's own PublishOptions: - transaction_enabled: any unaccepted event fails the RPC (Unavailable) — transactional callers resolve the whole batch from the RPC outcome alone, so per-event errors would still acknowledge everything, including events no subscriber produced. - otherwise: per-event results as before (rejections passed through, acceptance wins over rejection across subscribers).
…ntics (#2873) * fix(chip-router): match atlas write-service Publish/PublishBatch semantics Aligns the router with the atlas chip-ingress write-service, addressing the review findings on #2861: - Publish (single event) now follows the write-service's all-or-nothing contract (atlas chip-ingress internal/server: Publish delegates to PublishCloudEvents with transaction_enabled=true): it fails the RPC when no subscribers are registered, when every forward fails, or when no subscriber accepts the event, and returns exactly one PublishResult on success. Previously it returned an empty PublishResponse with a nil error unconditionally, acking events that went nowhere. - Failure RPCs use codes.Internal, the write-service's failure code, in place of codes.Unavailable. - An empty batch short-circuits to a no-op success before the subscriber check, matching the write-service's empty-batch path. - The per-event detail merge is now order-independent and sized to the batch: a later subscriber's results can extend detail beyond the first provider's length (previously trailing events were acked with no detail), and a detail-less success (empty results, nil error) now accepts the whole batch, clearing other subscribers' rejections — the intent documented in #2861 but not enforced by its merge. Tests: single-event success/failure paths, empty batch, detail-less acceptance, merge extension past the first provider's detail, rejection surviving another subscriber's RPC failure; stub simplified to an immutable struct. * fix(chip-router): bump builder image to golang:1.26.6 to match go.mod go.mod requires go >= 1.26.6 (bumped alongside the pkg/chipingress pin), so the pinned golang:1.25.3 builder fails at 'go mod download' with 'go: go.mod requires go >= 1.26.6 (running go 1.25.3; GOTOOLCHAIN=local)'. * fix(chip-router): implement Ping for write-service surface parity The write-service answers Ping with a static pong; the router inherited UnimplementedChipIngressServer's Unimplemented for it, which the new atlas parity suite's P0 surface probe flags as drift. Ping never needs to reach subscribers, so answer directly like the write-service. * fix(chip-router): make the image build self-contained (no framework dep in the binary) The image build recipe (chainlink cre environment: docker_ctx = framework/components/chiprouter) passes the module dir as the build context, so go.mod's `replace framework => ../../` (a monorepo-dev convenience inherited from main) cannot resolve inside the container and 'go mod download' fails. Published framework releases also do not compile against this module's moby pins, so the replace cannot simply be dropped for the module as a whole. The router binary only used framework for its global logger, so: - cmd/chip-router now carries its own zerolog logger, mirroring framework.L's setup (CTF_LOG_LEVEL env var, console writer to stderr); output format is unchanged. - the Dockerfile drops the replace for the isolated build and runs 'go mod tidy' so the published framework — needed only by the deploy-side library, which is not compiled into the image — resolves from the proxy for module-graph completeness. The deploy-side library (chiprouter.go) keeps its framework dependency and the monorepo replace. * fix(chip-router): reject mismatched batch results and tighten Publish failure path PublishBatch now treats a non-empty subscriber response whose result count differs from the batch size as a contract violation (the write-service returns exactly one result per event in input order): it is logged at Warn and counted as a failed forward instead of silently acking the events it does not cover. Empty responses stay a detail-less success, which chainlink's chip-testsink subscriber relies on. Publish drops the redundant anyForwarded flag and surfaces the subscriber's rejection reason in the Internal error when no subscriber accepted the event. Tests: rewrite the merge test into mismatch cases, make the detail-less acceptance test genuinely detail-less, and cover the single-subscriber empty-response Publish path. Also fix gofmt import order and make the package logger unexported.
…eset chiprouter is its own Go module (tags framework/components/chiprouter/vX.Y.Z) and consumers pin it separately. release-go-module.yml reads .changeset/<version>.md from the module directory and fails when it is missing, so the notes in framework/.changeset/v0.16.10.md could not release the router, while a framework release would have carried no framework change. Move them to chiprouter v1.0.5.
…ssage
A subscriber may reject an event without a PublishError.Reason, which made the
router return a Publish error with an empty gRPC message. Prefix the message
('event rejected by subscribers: <reason>') so it is never empty.
This was referenced Oct 26, 2026
📊 API Diff Results
|
📊 API Diff Results
|
… actionable When no subscriber was usable the RPC failed with a bare 'all N subscribers failed to accept the batch'; the reason (an RPC error, or a subscriber returning a result list whose length differs from the batch) was only in the router's log, so a caller could not tell what to fix. Record why each subscriber was ignored and put it in the status message: subscriber name and endpoint, what it did (RPC error, or 'returned N results for M events; a subscriber must return exactly one PublishResult per event, in input order, or no results to accept the whole batch'). Entries are sorted by subscriber so the text does not depend on completion order, and bounded (5 subscribers, 200 chars of a downstream error). Also: the no-subscribers error says how to register one (POST /subscribers); a transactional rejection lists which events were rejected and why, and that sending the batch without transaction_enabled delivers the accepted ones; a rejection without a reason is worded instead of left empty. Tests pin the message contents.
pkcll
marked this pull request as ready for review
October 27, 2026 12:47
pkcll
marked this pull request as draft
October 27, 2026 12:48
…ter module The Dockerfile deleted go.mod's replace and ran 'go mod tidy' at build time. Tidy resolves packages missing from go.mod at latest and shifts direct deps (x/sync 0.22->0.23, x/net, x/sys, x/text, x/crypto, genproto, otel 1.46->1.47 in a scratch simulation), so an image built from a pinned commit was not reproducible, and an upstream release needing a newer go than the pinned builder broke the build with no change in this repo. The router binary imports neither the framework module nor the deploy-side library, so give cmd/chip-router its own module (go.mod/go.sum checked in; versions equal to what the parent resolved) and build from those pins only: go mod download from go.mod+go.sum, then go build -mod=readonly. Same pattern as framework/leak/cmd. The library module's go.mod no longer requires the binary's dependencies.
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Delivery-critical acknowledgement changes and the isolated image build warrant human integration validation, and rejection-message bounds remain unresolved.
1 open issue
What changed in this PR
This PR fixes chip-router acknowledgements so durable emitters can resolve delivered events instead of repeatedly retransmitting them, and aligns publishing behavior with the chip-ingress write-service.
Changes:
- Returns per-event results, merges subscriber outcomes, and handles transactional failures.
- Adds
Ping, actionable failure messages, and publishing regression tests. - Makes the router image build self-contained and updates dependencies and release notes.
| File | Description |
|---|---|
framework/examples/chip_router/go.mod |
Aligns the example’s Go version. |
framework/components/chiprouter/go.sum |
Updates dependency checksums. |
framework/components/chiprouter/go.mod |
Updates Go and publishing dependencies; makes zerolog direct. |
framework/components/chiprouter/Dockerfile |
Updates the builder and isolates dependency resolution. |
framework/components/chiprouter/cmd/chip-router/main.go |
Implements acknowledgement semantics, diagnostics, Ping, and local logging. |
framework/components/chiprouter/cmd/chip-router/main_test.go |
Adds publishing and failure-message regression coverage. |
framework/components/chiprouter/.changeset/v1.0.5.md |
Documents delivery and image-build changes. |
Review effort: Balanced
| if e.GetReason() == "" { | ||
| return fmt.Sprintf("rejected (%s, no reason given)", e.GetErrorCode()) | ||
| } | ||
| return e.GetReason() |
… a nested module The image build failed because go.mod's 'replace framework => ../../' (added by #2510) cannot resolve inside the container, whose build context is the module directory. The earlier workaround deleted the replace and ran 'go mod tidy' at build time, which resolves missing packages at latest and shifts versions, so a rebuild of the same commit could differ. Remove the replace and require the published framework instead. v0.16.0 is the lowest release that compiles with this module's moby pins (v0.15.21 and earlier fail in promtail.go on container.HostConfig); consumers already pin framework >= v0.16.6, so nothing is lifted for them. The module now builds standalone: the Dockerfile is a plain COPY go.mod go.sum, go mod download, go build -mod=readonly, with no go.mod editing and no tidy. This supersedes the nested cmd/chip-router module (reverted): one module, cmd/ stays where it was, the library's dependencies are unchanged. examples/chip_router keeps replacing framework and chiprouter locally; its go.mod only picks up the framework version bump from 'go mod tidy'.
This branch has not been deployed
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.
Recreates #2861 (opened by cawthorne, closed in favor of this PR). Same branch and commit history; earlier review threads and CI history stay on the original.
A previous attempt to fix think in chainlink-common by reverting smartcontractkit/chainlink-common@dc8a1dc was here:
smartcontractkit/chainlink-common#2446 (comment)
But that PR has been closed for now, since it was intentional and implemented due to an audit finding:
https://ainv.smartcontract.com/findings/79362753-0fef-4a50-934c-572e8f7c504d
Summary
PublishBatchalways returned an emptyPublishResponseregardless of the batch's outcome. The node-side batch client (chainlink-common/pkg/chipingress/batch) requires onePublishResultper event to resolve delivery: since chainlink-common#2326 (an audit remediation for a data-loss finding) an empty results array is reported to every event's callback asErrCodeResultsMismatch.Against this router, a chainlink v2.67.2-rc.0 node's durable emitter therefore treated every event as undelivered and retransmitted the whole stream every 60s, forever:
failed to deliver event. Relying on retransmit.warnings on a single node, first at 12s after boot, zero successful acks;time() - <metric>_ts < 120gates).The events were forwarded to Kafka on every attempt — only the acks were missing — so the storm duplicated the stream rather than losing data.
The fix
The router now follows the atlas chip-ingress write-service (
internal/server/server.go,internal/service/write_service.go). This includes the follow-up #2873 that was merged into this branch.PublishBatchPublishResultper event, index-aligned with the batch (EventIdechoed), merged across subscribers. An acceptance always wins over a rejection and a rejection only fills an empty slot, so the outcome does not depend on subscriber completion order. A nil-error response with no results (a detail-less success) accepts the whole batch.Internalerror says so: which subscriber (name, endpoint), what it returned (returned N results for M events), and what a correct response is (exactly onePublishResultper event in input order, or no results to accept the whole batch).transaction_enabled=true, any unaccepted event fails the RPC, because transactional callers resolve the whole batch from the RPC outcome alone.codes.Internalwhen no subscribers are registered or no forward succeeded, so durable callers retain and retry (at-least-once) instead of acking events nothing carried.Publish(single event) is all-or-nothing like the write-service: no subscribers, or no subscriber accepted the event →codes.Internal(the message carries the subscriber's rejection reason); success → exactly onePublishResult.Also
Pingis implemented (staticpong) for write-service surface parity.Internalfailure message is meant to be actionable on its own: per-subscriber causes (RPC error or result-count mismatch, sorted and bounded), how to register a subscriber when none is registered, and for atransaction_enabledbatch which events were rejected and that sending it withouttransaction_enableddelivers the accepted ones.codes.Internal, the write-service's failure code, instead ofcodes.Unavailable.main's Dockerfile cannot build today:go.mod'sreplace ... => ../../(added by update to testcontainers-go version with moby #2510) does not resolve inside the container, sogo mod downloadfails. Instead of deleting thereplaceand runninggo mod tidyat build time (which resolves missing packages at latest and shifts versions, so a rebuild of the same commit can differ), this PR removes thereplaceand requires the publishedframework v0.16.0, the first release that compiles with this module's moby pins (v0.15.21 and earlier fail inpromtail.go; consumers such as chainlink already pin >= v0.16.6, so nothing is lifted for them). The Dockerfile is then plaingo mod download→go build -mod=readonlyfrom the checked-ingo.mod/go.sum. The binary no longer importsframework(own zerolog logger mirroringframework.L); buildergolang:1.26.6. Trade-off: the library is built against the publishedframework, not the in-repo one;examples/chip_routerstill replaces both modules locally.chainlink-common/pkg/chipingresspin (Dec 2025 → Sep 2026): the old pin's pb predatesPublishResult.Error.framework/components/chiprouter/.changeset/v1.0.5.md.Tests
go test -race -count=20 ./cmd/chip-router/— 26 tests covering: per-event results with matching ids; empty batch; no-subscribers and all-forwards-failed (Internal); downstream rejection pass-through and survival of another subscriber's RPC failure; acceptance-wins / order independence; detail-less acceptance; length-mismatch handling; transactional partial / all-rejected / all-accepted; single-eventPublish(one result, no subscribers, all forwards failed, all rejected, detail-less success, empty rejection reason);Ping; and the contents of the failure messages above.golangci-lint(repo config): 0 issues.Docker image
Built and exercised for real from this branch (
06798d63; Docker Desktop 29.8.2, arm64 host, context = the module dir as in chainlink'sdocker_ctx):docker build -f framework/components/chiprouter/Dockerfile framework/components/chiproutersucceeds:go mod download5.6s,go build -mod=readonly45.6s, nosed/tidy. With an empty module cache andGOFLAGS=-mod=readonlythe binary also builds (exit 0) and leavesgo.mod/go.sumunchanged. The image islinux/amd64(the Dockerfile's existingTARGETARCH=amd64default), non-root (uid 65532), entrypoint/chip-router.Ping→pong;Publish/PublishBatchwith no subscribers →Internalwith how to register one; empty batch → OK with 0 results; accepting subscriber → one positional result per event / exactly one forPublish; only subscriber returning a short list, atransaction_enabledrejection and a dead subscriber →Internalwith the actionable messages; partial mode → OK with the per-event rejection.go mod downloadlayerCACHED(only the compile step reruns).go mod tidyis stable inchiprouterand inexamples/chip_router, which only picks up theframeworkversion bump.