Skip to content

gorums: replace call options with typed call handles - #332

Open
meling wants to merge 5 commits into
gorums/api-renamesfrom
gorums/typed-call-handles
Open

gorums: replace call options with typed call handles#332
meling wants to merge 5 commits into
gorums/api-renamesfrom
gorums/typed-call-handles

Conversation

@meling

@meling meling commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Replaces the variadic opts ...CallOption on every generated call with typed handles.

CallOption held two unrelated things. IgnoreErrors switched a one-way call between blocking and fire-and-forget, and Interceptors carried type-erased interceptors that CallContext had to assert back to their concrete type at dispatch — so a caller could pass an interceptor for the wrong message types and find out at run time.

A quorum call now returns *Call[Req, Resp], which embeds *Responses[Resp] so the terminal methods are unchanged, and adds Intercept for interceptors typed to the call's own request and response:

resp, err := storage.ReadQC(ctx, req).Intercept(logging, audit).Majority()

A one-way call returns *OnewayCall[Req] with the two behaviors as named terminals:

err := storage.WriteMulticast(ctx, req).Send()          // blocks, reports every send failure
h := storage.WriteMulticast(ctx, req).Async(); h.Wait() // dispatches now, collect later

Async does what IgnoreErrors could not: keep several one-way calls in flight from a single goroutine and still learn whether they were sent.

Behavior changes worth noting. Dropping a one-way handle without consuming it sends nothing, so a call is dispatched only where the code says so. Consuming a handle twice panics rather than re-sending or blocking on confirmations the first dispatch drained; Intercept after dispatch panics too, since an interceptor can no longer affect an in-flight call. ErrSkipNode no longer counts as a node error in Threshold or Correctable — a node the caller's own request transform skipped is neither a success nor a failure.

Type parameters are constrained by proto.Message directly, replacing the msg alias that hid what the constraint was.

Verification: go test ./... -count=2, go test -C examples ./..., go vet -tags=integration ./..., gofmt -l.

Copilot AI lite review requested due to automatic review settings August 11, 2026 20:15
@deepsource-io

deepsource-io Bot commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

DeepSource Code Review

We reviewed changes in 200352f...ba1465a on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
Go Aug 12, 2026 12:38p.m. Review ↗
Shell Aug 12, 2026 12:38p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@meling meling changed the title gorums/typed call handles gorums: replace call options with typed call handles Aug 11, 2026
Copilot AI reviewed Aug 11, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Gorums’ client-side call APIs to return typed, lazily-dispatched handle types (Call for quorum calls and OnewayCall for one-way unicast/multicast), enabling fluent interceptor registration via .Intercept(...) and explicit consumption via terminal methods (and .Send() / .Async().Wait() for one-way calls).

Changes:

  • Replace option-based quorum/one-way call APIs with typed call handles (Call, OnewayCall) and move interceptor registration to Intercept(...).
  • Refactor call context dispatch/interceptor plumbing to enforce “no Intercept after dispatch/consumption” and to support lazy send semantics.
  • Update code generator templates, generated test protos, examples, docs, and tests to the new call-handle APIs.

Reviewed changes

Copilot reviewed 26 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
unicast.go Switch unicast to return *OnewayCall handle instead of error with call options.
multicast.go Switch multicast to return *OnewayCall handle instead of error with call options.
quorumcall.go Switch quorum call entrypoints to return *Call handle and remove call options from generated-call surface.
remotecall.go Update generics constraint from internal msg alias to proto.Message.
responses.go Remove msg alias usage, adjust generics constraints, and tighten ErrSkipNode handling; add dispatched-marking support.
responses_test.go Update unit tests for responses to new generics and newResponses constructor.
server_e2e_test.go Update e2e tests to consume one-way handles via .Send() and adjust call-handle access patterns.
inbound_manager_test.go Update reverse-direction multicast to use .Send() and rename test for clarity.
correctable.go Ensure correctable calls mark dispatch before goroutine spawn to make late Intercept panics deterministic.
call_context.go Add atomic dispatch tracking and move interceptor chaining to a pre-dispatch intercept(...) method; add newOnewayCallContext.
call.go Introduce Call, OnewayCall, OnewayAsync, and exported ClientInterceptor/MapRequest/MapResponse.
call_test.go Add tests for one-way handle behavior, dispatch/Intercept panics, and move RemoteCall tests here.
call_quorum_test.go Update tests to use *gorums.Call[...] and .Send() for multicast error handling.
call_async_test.go Update async tests to use embedded Responses and add Async.Done() coverage.
call_client_interceptor_test.go Update interceptor tests to use .Intercept(...) chaining instead of gorums.Interceptors(...) call option.
internal/tests/oneway/oneway_test.go Rework one-way tests to use .Send(), add bounded receive collection, and share clusters across subtests.
internal/tests/oneway/oneway_gorums.pb.go Regenerate oneway service client stubs to return *OnewayCall handles.
internal/tests/ordering/order_gorums.pb.go Regenerate quorum-call stub to return *Call and drop call options.
internal/tests/correctable/correctable_gorums.pb.go Regenerate correctable stubs to return *Call and drop call options.
internal/tests/config/config_gorums.pb.go Regenerate config stubs to return *Call and drop call options.
examples/storage/proto/storage_gorums.pb.go Regenerate example stubs to return call handles and update docs/examples in comments.
examples/storage/server.go Update nested multicast to call .Send().
examples/storage/repl.go Update REPL one-way calls to call .Send().
examples/storage/client.go Update helper signatures to accept *gorums.Call[...] and use .Results().
doc/user-guide.md Update user guide to reflect call-handle APIs and .Intercept(...) usage.
cmd/protoc-gen-gorums/gengorums/template_unicast.go Update generator template to emit *OnewayCall-returning unicast helpers.
cmd/protoc-gen-gorums/gengorums/template_multicast.go Update generator template to emit *OnewayCall-returning multicast helpers.
cmd/protoc-gen-gorums/gengorums/template_quorumcall.go Update generator template to emit *Call-returning quorum-call helpers.
cmd/protoc-gen-gorums/dev/zorums_unicast_gorums.pb.go Regenerated dev unicast stubs to return *OnewayCall.
cmd/protoc-gen-gorums/dev/zorums_multicast_gorums.pb.go Regenerated dev multicast stubs to return *OnewayCall.
cmd/protoc-gen-gorums/dev/zorums_quorumcall_gorums.pb.go Regenerated dev quorum-call stubs to return *Call.
remote_call_test.go Remove standalone RemoteCall tests (moved into call_test.go).
callopts.go Remove call-option machinery (IgnoreErrors/Interceptors call options).
callopts_test.go Remove tests/benchmarks for removed call-option machinery.
Files not reviewed (8)
  • cmd/protoc-gen-gorums/dev/zorums_multicast_gorums.pb.go: Generated file
  • cmd/protoc-gen-gorums/dev/zorums_quorumcall_gorums.pb.go: Generated file
  • cmd/protoc-gen-gorums/dev/zorums_unicast_gorums.pb.go: Generated file
  • examples/storage/proto/storage_gorums.pb.go: Generated file
  • internal/tests/config/config_gorums.pb.go: Generated file
  • internal/tests/correctable/correctable_gorums.pb.go: Generated file
  • internal/tests/oneway/oneway_gorums.pb.go: Generated file
  • internal/tests/ordering/order_gorums.pb.go: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread doc/user-guide.md Outdated
func ReadQC(ctx *gorums.ConfigContext, in *ReadRequest) *gorums.Call[*ReadRequest, *ReadResponse]
```

This function returns a `*gorums.Responses[*ReadResponse]` object that provides several ways to aggregate and process responses.
Comment thread internal/tests/oneway/oneway_test.go
Comment on lines +55 to +61
func (c *cluster) reset() {
for _, srv := range c.srvs {
for range len(srv.received) {
<-srv.received
}
}
}
Comment thread call.go
Comment on lines +169 to +171
// collect gathers one send confirmation per node and reports the failures,
// aggregating them for multicast and passing the single error through for
// unicast. Nodes skipped by a request transform are not failures.
@meling
meling force-pushed the gorums/typed-call-handles branch from a1b979a to ea0a4a0 Compare August 12, 2026 11:42
meling added 5 commits August 12, 2026 14:19
@meling
gorums: replace call options with typed call handles
7e88636 
Every generated call took a variadic opts ...CallOption, which held two
unrelated things. IgnoreErrors switched a one-way call between blocking and
fire-and-forget, and Interceptors carried type-erased interceptors that
CallContext had to assert back to their concrete type at dispatch. Neither is
expressible in the signature, so a caller could pass an interceptor for the
wrong message types and find out at run time.

A quorum call now returns *Call[Req, Resp], which embeds *Responses[Resp] so the
terminal methods are unchanged, and adds Intercept for interceptors typed to the
call's own request and response. A one-way call returns *OnewayCall[Req] with the
two behaviors as named terminals: Send blocks until every send completes and
reports the failures, Async dispatches and defers them to Wait. Async also does
what IgnoreErrors could not: keep several one-way calls in flight from a single
goroutine and still learn whether they were sent.

Dropping a one-way handle without consuming it sends nothing, so a call is now
dispatched only where the code says so.

Consuming a handle twice panics rather than re-sending the request or blocking on
confirmations the first dispatch already drained. Intercept after dispatch panics
too, since an interceptor can no longer affect an in-flight call; async and
correctable calls mark dispatch before starting their goroutine so that panic is
deterministic rather than a race.

ErrSkipNode no longer counts as a node error in Threshold or Correctable. A node
the caller's own request transform skipped is neither a success nor a failure,
and counting it as an error made a deliberate skip look like an outage.

The type parameters are constrained by proto.Message directly, replacing the msg
alias that hid what the constraint was, and newResponses is unexported now that
Call constructs it.
@meling
doc: update the user guide for the typed call handles
7eb23f7 
Generated signatures no longer take call options, so the examples register
interceptors with Intercept on the handle and consume one-way calls with Send or
Async. Records that Intercept must precede any terminal method.

Also repairs a set of section headings that a blanket Interceptor to
ServerInterceptor rename caught by mistake: they describe client-side
interceptors, not server-side ones.
@meling
gorums: regenerate for the typed call handles
4bed6cd 
Generated output only, produced by make genproto. Quorum calls return a Call
handle, one-way calls return a OnewayCall handle, and neither takes call options.
The one-way doc comments now show both terminals.
@meling
internal/tests/oneway: drain the received channels until empty
ee88514 
reset evaluated len(srv.received) once and consumed exactly that many
messages, so a straggler that arrived while it was draining stayed queued
and was counted against the next subtest. Drain until each channel reports
empty instead.
@meling
doc: name the handle the quorum call helper returns
ba1465a 
The signature above it was updated to return *gorums.Call, but the prose
still described a *gorums.Responses. Name the handle and say that it embeds
Responses, which is what keeps the aggregation methods described below
available on it.
@meling
meling force-pushed the gorums/typed-call-handles branch from ea0a4a0 to ba1465a Compare August 12, 2026 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@meling