-
Notifications
You must be signed in to change notification settings - Fork 12
Description
The generated service trait signatures are harder to read and implement
than they need to be. This issue collects the specific problems and a
proposed redesign.
The problems
Context conflates input and output
The current Context struct mixes request-side data with response-side
mutation targets:
pub struct Context {
pub headers: http::HeaderMap, // input: request headers
pub response_headers: http::HeaderMap, // output: set by handler
pub trailers: http::HeaderMap, // output: set by handler
pub deadline: Option<Instant>, // input: parsed from request
pub compress_response: Option<bool>, // output: set by handler
}The handler takes ownership, maybe mutates the output fields, then hands
the whole struct back. This means:
- Even handlers that don't touch response metadata have to thread
ctx
through - the simplest possible impl still ends inOk((resp, ctx)). - It's not obvious from the struct definition which fields you're meant
to read vs. write. Theheadersfield is request headers, but that's
only clear from the doc comment. - Setting a response header when you haven't bound
ctxasmutis a
two-step fix (addmut, thenctx.response_headers.insert(...))
rather than a natural builder chain.
Tuple return forces ceremony on the happy path
Every handler returns Result<(Res, Context), ConnectError>. The ELIZA
say handler - one line of actual logic - ends up as:
async fn say(&self, ctx: Context, request: OwnedView<SayRequestView<'static>>)
-> Result<(SayResponse, Context), ConnectError>
{
let (reply, _) = eliza::reply(request.sentence);
Ok((SayResponse { sentence: reply, ..Default::default() }, ctx))
}The (, ctx) at the end is pure noise. Compare tonic, where the happy
path is Ok(Response::new(reply)).
OwnedView<FooView<'static>> is noisy
The request parameter type is always OwnedView<FooRequestView<'static>>.
In the generated trait it expands to the full module path, so the
three-method ElizaService trait is 77 lines. Users hand-writing the
impl can use the view type, but they still write the OwnedView<...>
wrapper and 'static every time.
Streaming signatures are unreadable
Pin<Box<dyn Stream<Item = Result<T, ConnectError>> + Send>> appears
verbatim on both sides of streaming methods. The generated converse
method signature (bidi) is 35 lines. Implementers have to repeat the
full type in their impl signature, then write an explicit coercion at
the return site:
Ok((
Box::pin(response_stream) as Pin<Box<dyn Stream<Item = _> + Send>>,
ctx,
))Proposed direction
Split Context into a read-only RequestContext (headers, deadline,
peer info) passed in, and a Response<T> returned. Add codegen-emitted
type aliases so the trait reads as:
fn say(&self, ctx: RequestContext, req: OwnedSayRequestView)
-> impl Future<Output = ServiceResult<SayResponse>> + Send;where:
type OwnedSayRequestView = OwnedView<SayRequestView<'static>>;-
emitted alongside each message typetype ServiceResult<T> = Result<Response<T>, ConnectError>;type ServiceStream<T> = Pin<Box<dyn Stream<Item = Result<T, ConnectError>> + Send>>;Response<T>wraps the message with response headers/trailers/
compression hint;From<T> for Response<T>makes the happy path
Ok(resp.into())RequestContextis the natural home for peer identity (mTLS peer
certs, remote address)
Happy-path impl becomes:
async fn say(&self, _ctx: RequestContext, req: OwnedSayRequestView)
-> ServiceResult<SayResponse>
{
let (reply, _) = eliza::reply(req.sentence);
Ok(SayResponse { sentence: reply, ..Default::default() }.into())
}Setting response metadata:
Ok(Response::new(reply)
.with_header("x-request-id", id)
.with_trailer("x-timing", elapsed))Scope
Breaking change to the generated trait shape and Context/Response
types. Touches codegen, the handler/service/router plumbing, and every
service impl in the test and example suite. Target is 0.3.0 as one
focused PR.