Skip to content

fix(api): MultiBuilder rejects silently-dropped per-route consumer-wide settings#199

Merged
eschizoid merged 2 commits into
mainfrom
fix/multibuilder-reject-per-route-consumer-wide-settings
Jun 20, 2026
Merged

fix(api): MultiBuilder rejects silently-dropped per-route consumer-wide settings#199
eschizoid merged 2 commits into
mainfrom
fix/multibuilder-reject-per-route-consumer-wide-settings

Conversation

@eschizoid

Copy link
Copy Markdown
Owner

Summary

MultiBuilder.start() was a §12 silent-failure: per-route consumer-wide
settings like withRetry, withDeadLetterTopic, withErrorHandler,
withPollTimeout, withMetrics, withTracer, withCircuitBreaker,
and withBackpressure were quietly dropped when a route configurator
set them. The previous rejectPerRouteConsumerWideSettings guard only
caught withProcessingMode + withKeyOrderedMaxKeys; everything else
fell on the floor.

The bug

MultiBuilder.start() only invokes ds.buildPipeline() /
addBatchRoute(...) for each route. DefaultStream.applyCommonConsumerConfig
(which propagates retry / backpressure / metrics / errorHandler /
dlqTopic / pollTimeout / tracer onto the underlying KPipeConsumer.Builder)
is never called from the multi path. Every per-route consumer-wide
setting other than the two already guarded was silently lost.

Reproduction (before this PR)

KPipe.multi(props)
  .json("t", s -> s.withRetry(3, Duration.ofSeconds(1)).withDeadLetterTopic("dlq"))
  .start();

…built a consumer with neither retry NOR DLQ wired, and emitted no
error. The misconfig surfaced only as missing behavior at runtime —
exactly the overloaded-null / silent-drop footgun §12 exists to prevent.

Fix

Extend MultiBuilder.rejectPerRouteConsumerWideSettings to throw
IllegalArgumentException when a per-route configurator sets any
consumer-wide value. Two message shapes:

  • Has a symmetric MultiBuilder.with* mirror (withMetrics,
    withTracer, withCircuitBreaker): error points users at the
    consumer-level setter.
  • No mirror yet (withRetry, withBackpressure,
    withDeadLetterTopic, withErrorHandler, withPollTimeout):
    error points users at the explicit KPipeConsumer.Builder
    escape hatch and notes the deferred multi-builder mirror follow-up.

Deferred follow-up

Adding consumer-wide mirrors on MultiBuilder for withRetry /
withDeadLetterTopic / withErrorHandler / withPollTimeout /
withBackpressure is a separate change — they need design decisions
(e.g. should DLQ be a consumer-wide single topic or per-route routing?).
This PR's rejection message documents the gap so users hit a loud error
with a workable next step, instead of a silently-broken consumer.

Test plan

  • One test per rejected setting: withRetry, withBackpressure,
    withDeadLetterTopic, withErrorHandler, withPollTimeout,
    withMetrics, withTracer, withCircuitBreaker. Each asserts the
    thrown IllegalArgumentException names the offending setter and the
    offending route topic, and (for mirrored settings) the message points
    at the consumer-level MultiBuilder.with* setter.
  • Smoke test that the consumer-wide MultiBuilder setters
    (withMetrics, withTracer, withCircuitBreaker) still build
    cleanly together with the existing withProcessingMode /
    withKeyOrderedMaxKeys chain.
  • ./gradlew :lib:kpipe-api:test — all 21 cases in
    KPipeFacadeBuildTest pass; full module suite green.

…de settings

MultiBuilder.start() folds N per-topic routes into a single underlying
KPipeConsumer, but only replays each route's pipeline (buildPipeline /
addBatchRoute) — the route's consumer-level config (withRetry,
withDeadLetterTopic, withErrorHandler, withPollTimeout, withMetrics,
withTracer, withCircuitBreaker, withBackpressure) was silently dropped.
DefaultStream.applyCommonConsumerConfig is never called from the multi
path. The previous rejectPerRouteConsumerWideSettings guard only caught
withProcessingMode and withKeyOrderedMaxKeys; everything else fell on
the floor and produced a consumer with neither retry NOR DLQ wired and
no error.

This is a §12 silent-failure: the misconfig surfaces as missing
behavior at runtime instead of a loud rejection at construction.

Extend the guard to throw IllegalArgumentException for every silently-
dropped per-route setter. For settings with a symmetric MultiBuilder
mirror (withMetrics, withTracer, withCircuitBreaker), the message
points at it; for settings without a mirror yet (withRetry,
withBackpressure, withDeadLetterTopic, withErrorHandler,
withPollTimeout), the message points at the explicit KPipeConsumer.
Builder escape hatch and notes the deferred follow-up.

Tests pin one rejection per setting plus a smoke test that the existing
consumer-wide MultiBuilder setters still build cleanly.
@eschizoid

Copy link
Copy Markdown
Owner Author

@copilot please review

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.50%. Comparing base (4ccac98) to head (b1f8f06).
⚠️ Report is 9 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #199      +/-   ##
============================================
+ Coverage     77.82%   78.50%   +0.68%     
- Complexity      732      747      +15     
============================================
  Files            68       66       -2     
  Lines          2800     2815      +15     
  Branches        346      356      +10     
============================================
+ Hits           2179     2210      +31     
+ Misses          462      444      -18     
- Partials        159      161       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@eschizoid

Copy link
Copy Markdown
Owner Author

@copilot please review

…tion

Reviewer flagged exception-type inconsistency: the pre-existing
withProcessingMode / withKeyOrderedMaxKeys checks threw
IllegalStateException while the eight new rejections in this PR use
IllegalArgumentException. Same failure mode (route-configurator
misconfig at construction). Promote both legacy checks to
IllegalArgumentException so callers can catch one type.

Updates the two assertions in KPipeFacadeBuildTest that pinned the old
type.
@eschizoid eschizoid force-pushed the fix/multibuilder-reject-per-route-consumer-wide-settings branch from 4abf19e to b1f8f06 Compare June 20, 2026 00:02
@eschizoid eschizoid merged commit be1b873 into main Jun 20, 2026
4 checks passed
@eschizoid eschizoid deleted the fix/multibuilder-reject-per-route-consumer-wide-settings branch June 22, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant