Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **A Call pipelined on an already-failed answer never received a Return.**
Found on the wire by the cross-impl L3 lane (`just e2e-l3-vatc`,
pipelined-provide scenarios): the C++ recipient pipelines its Call on the
Accept question without waiting, the host refuses the Accept
(`CrossPeerReceiverHostedTargetUnsupported`), and the Call — arriving after
the refusal already went out — parked in `pending_promises` forever. The
queued-call drain in `sendReturnException` only reaches calls queued
*before* the exception Return; results Returns are recorded in
`resolved_answers` precisely so late pipelined calls can resolve, but
exception Returns were recorded nowhere, so a late call on a failed answer
was indistinguishable from one on a still-pending answer. The C++ recipient
hung on the spec's exactly-one-Return-per-call guarantee. Not Accept-specific:
any answer failed synchronously during dispatch had the same window.

Exception Returns are now recorded in a `failed_answers` map (reason + type,
kept until Finish, the exact lifecycle of `resolved_answers`), and a
promised-target call that would otherwise queue is answered immediately with
a **copy of the recorded exception**, preserving the retryability signal —
the broken-pipeline behavior of the C++ reference. The record is written in
the one funnel every exception Return passes through, so transitive
pipelines (a call pipelined on a failed pipelined call) are covered, and the
drain path is unchanged. Ablation-proven unit tests pin both the refused-
Accept shape and the plain two-party shape; the e2e driver now observes the
refusal *through* a pipelined call instead of working around the gap with
`whenResolved()`.

- **`MessageBuilder.writeTo` and `writePackedTo` were neither type-checked nor
tested.** Both are frozen Stable API with **zero call sites in the tree**, and
Zig does not analyse an uninstantiated generic body — so making them concrete
Expand Down
4 changes: 4 additions & 0 deletions docs/api-snapshot-experimental.txt
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ capnpc-zig.rpc.peer.Peer.enableRuntimeThreadChecks: fn (*rpc.peer.mod.Peer, bool
capnpc-zig.rpc.peer.Peer.ensureExportAt: fn (*rpc.peer.mod.Peer, u32, rpc.peer.mod.Export) error{CapTableFull,OutOfMemory}!bool
capnpc-zig.rpc.peer.Peer.entropy: field ?rpc.peer.mod.EntropySource = null
capnpc-zig.rpc.peer.Peer.exports: field hash_map.HashMap(u32,rpc.peer.state.ExportEntry(rpc.peer.mod.Export),hash_map.AutoContext(u32),80)
capnpc-zig.rpc.peer.Peer.failed_answers: field hash_map.HashMap(u32,rpc.peer.state.FailedAnswer,hash_map.AutoContext(u32),80)
capnpc-zig.rpc.peer.Peer.finished_early_answers: field hash_map.HashMap(u32,bool,hash_map.AutoContext(u32),80)
capnpc-zig.rpc.peer.Peer.forgetImportRefsForHost: fn (*rpc.peer.mod.Peer, u32, u32) anyerror!void
capnpc-zig.rpc.peer.Peer.forwarded_questions: field hash_map.HashMap(u32,u32,hash_map.AutoContext(u32),80)
Expand Down Expand Up @@ -1391,6 +1392,9 @@ capnpc-zig.rpc.peer.resolve: struct
capnpc-zig.rpc.peer.seedEntropyCsprng: fn (Io) error{Canceled,EntropyUnavailable}!Random.ChaCha
capnpc-zig.rpc.peer.shutdown_reason: const *const [18:0]u8
capnpc-zig.rpc.peer.state.ExportEntry: fn (comptime type) type
capnpc-zig.rpc.peer.state.FailedAnswer.ex_type: field rpc.wire.protocol.ExceptionType
capnpc-zig.rpc.peer.state.FailedAnswer.reason: field []u8
capnpc-zig.rpc.peer.state.FailedAnswer: struct
capnpc-zig.rpc.peer.state.JoinKeyPart.join_id: field u32
capnpc-zig.rpc.peer.state.JoinKeyPart.part_count: field u16
capnpc-zig.rpc.peer.state.JoinKeyPart.part_num: field u16
Expand Down
17 changes: 17 additions & 0 deletions src/rpc/peer/call/peer_call_orchestration.zig
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,11 @@ pub fn handleCallPromisedTargetForPeer(
promised: protocol.PromisedAnswer,
resolve_promised_answer: *const fn (*PeerType, protocol.PromisedAnswer) anyerror!cap_table.ResolvedCap,
has_unresolved_promise_export: *const fn (*PeerType, u32) bool,
lookup_failed_answer: *const fn (*PeerType, u32) ?peer_call_targets.FailedAnswerView,
queue_promised_call: *const fn (*PeerType, u32, []const u8, InboundCapsType) anyerror!void,
queue_promise_export_call: *const fn (*PeerType, u32, []const u8, InboundCapsType) anyerror!void,
send_return_exception: *const fn (*PeerType, u32, []const u8) anyerror!void,
send_return_exception_typed: *const fn (*PeerType, u32, []const u8, protocol.ExceptionType) anyerror!void,
handle_resolved_call: *const fn (*PeerType, protocol.Call, *const InboundCapsType, cap_table.ResolvedCap) anyerror!void,
release_inbound_caps: *const fn (*PeerType, *InboundCapsType) anyerror!void,
report_nonfatal_error: *const fn (*PeerType, anyerror) void,
Expand All @@ -311,6 +313,7 @@ pub fn handleCallPromisedTargetForPeer(
promised,
resolve_promised_answer,
has_unresolved_promise_export,
lookup_failed_answer,
);

switch (target_plan) {
Expand All @@ -335,6 +338,16 @@ pub fn handleCallPromisedTargetForPeer(
try send_return_exception(peer, call.question_id, @errorName(err));
return;
},
.fail_broken_answer => |failed| {
// Pipelined on an answer that already returned an exception. The
// failure drain (`failQueuedPromisedCalls`) only reaches calls
// queued BEFORE the exception Return; this one arrived after, so
// fail it the same way — a copy of the answer's own exception,
// preserving the retryability signal.
release_caps = true;
try send_return_exception_typed(peer, call.question_id, failed.reason, failed.ex_type);
return;
},
.handle_resolved => |resolved| {
release_caps = true;
try handle_resolved_call(peer, call, &inbound_caps, resolved);
Expand All @@ -347,9 +360,11 @@ pub fn handleCallPromisedTargetForPeerFn(
comptime InboundCapsType: type,
comptime resolve_promised_answer: *const fn (*PeerType, protocol.PromisedAnswer) anyerror!cap_table.ResolvedCap,
comptime has_unresolved_promise_export: *const fn (*PeerType, u32) bool,
comptime lookup_failed_answer: *const fn (*PeerType, u32) ?peer_call_targets.FailedAnswerView,
comptime queue_promised_call: *const fn (*PeerType, u32, []const u8, InboundCapsType) anyerror!void,
comptime queue_promise_export_call: *const fn (*PeerType, u32, []const u8, InboundCapsType) anyerror!void,
comptime send_return_exception: *const fn (*PeerType, u32, []const u8) anyerror!void,
comptime send_return_exception_typed: *const fn (*PeerType, u32, []const u8, protocol.ExceptionType) anyerror!void,
comptime handle_resolved_call: *const fn (*PeerType, protocol.Call, *const InboundCapsType, cap_table.ResolvedCap) anyerror!void,
comptime release_inbound_caps: *const fn (*PeerType, *InboundCapsType) anyerror!void,
comptime report_nonfatal_error: *const fn (*PeerType, anyerror) void,
Expand All @@ -365,9 +380,11 @@ pub fn handleCallPromisedTargetForPeerFn(
promised,
resolve_promised_answer,
has_unresolved_promise_export,
lookup_failed_answer,
queue_promised_call,
queue_promise_export_call,
send_return_exception,
send_return_exception_typed,
handle_resolved_call,
release_inbound_caps,
report_nonfatal_error,
Expand Down
60 changes: 57 additions & 3 deletions src/rpc/peer/call/peer_call_targets.zig
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,22 @@ pub fn planImportedTarget(
return if (has_handler) .call_handler else .missing_export_handler;
}

/// Borrowed view of a recorded failed answer's exception (see
/// `state.FailedAnswer`): valid until the record is removed at Finish.
pub const FailedAnswerView = struct {
reason: []const u8,
ex_type: protocol.ExceptionType,
};

pub const PromisedTargetPlan = union(enum) {
queue_promised_call,
queue_export_promise: u32,
handle_resolved: cap_table.ResolvedCap,
send_exception: anyerror,
/// The target answer already returned an exception: answer the call with
/// a copy of that exception. Queueing would wedge it forever — a failed
/// answer never replays its queue.
fail_broken_answer: FailedAnswerView,
};

pub fn planPromisedTarget(
Expand All @@ -39,9 +50,19 @@ pub fn planPromisedTarget(
promised: protocol.PromisedAnswer,
resolve_promised_answer: *const fn (*PeerType, protocol.PromisedAnswer) anyerror!cap_table.ResolvedCap,
has_unresolved_promise_export: *const fn (*PeerType, u32) bool,
lookup_failed_answer: *const fn (*PeerType, u32) ?FailedAnswerView,
) PromisedTargetPlan {
const resolved = resolve_promised_answer(peer, promised) catch |err| {
if (err == error.PromiseUnresolved) return .queue_promised_call;
if (err == error.PromiseUnresolved) {
// `resolved_answers` records results Returns only. An answer that
// already FAILED misses there identically to one still pending —
// distinguish via the failed-answer record, or the call queues
// against a Return that will never come.
if (lookup_failed_answer(peer, promised.question_id)) |failed| {
return .{ .fail_broken_answer = failed };
}
return .queue_promised_call;
}
return .{ .send_exception = err };
};

Expand Down Expand Up @@ -108,10 +129,11 @@ test "peer_call_targets imported target planning covers all branches" {
);
}

test "peer_call_targets promised target planning handles unresolved, exception, queue-export and resolved" {
test "peer_call_targets promised target planning handles unresolved, failed-answer, exception, queue-export and resolved" {
const FakePeer = struct {
mode: enum {
unresolved,
failed_answer,
failure,
exported_unresolved,
exported_resolved,
Expand All @@ -123,7 +145,7 @@ test "peer_call_targets promised target planning handles unresolved, exception,
fn resolvePromisedAnswer(peer: *FakePeer, promised: protocol.PromisedAnswer) !cap_table.ResolvedCap {
_ = promised;
return switch (peer.mode) {
.unresolved => error.PromiseUnresolved,
.unresolved, .failed_answer => error.PromiseUnresolved,
.failure => error.TestExpectedError,
.exported_unresolved, .exported_resolved => .{ .exported = .{ .id = 9 } },
.imported_resolved => .{ .imported = .{ .id = 11 } },
Expand All @@ -133,6 +155,11 @@ test "peer_call_targets promised target planning handles unresolved, exception,
fn hasUnresolvedPromiseExport(peer: *FakePeer, export_id: u32) bool {
return peer.mode == .exported_unresolved and export_id == 9;
}

fn lookupFailedAnswer(peer: *FakePeer, question_id: u32) ?FailedAnswerView {
if (peer.mode != .failed_answer or question_id != 1) return null;
return .{ .reason = "broken", .ex_type = .overloaded };
}
};

// Neither the planner nor the fake hooks above inspect the transform, so
Expand All @@ -153,10 +180,33 @@ test "peer_call_targets promised target planning handles unresolved, exception,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
try std.testing.expectEqual(PromisedTargetPlan.queue_promised_call, plan);
}

{
// Same PromiseUnresolved from the resolver, but the answer is on
// record as FAILED: the plan must carry the recorded exception, not
// queue the call behind a Return that will never come.
var peer = FakePeer{ .mode = .failed_answer };
const plan = planPromisedTarget(
FakePeer,
&peer,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
switch (plan) {
.fail_broken_answer => |failed| {
try std.testing.expectEqualStrings("broken", failed.reason);
try std.testing.expectEqual(protocol.ExceptionType.overloaded, failed.ex_type);
},
else => return error.TestExpectedEqual,
}
}

{
var peer = FakePeer{ .mode = .failure };
const plan = planPromisedTarget(
Expand All @@ -165,6 +215,7 @@ test "peer_call_targets promised target planning handles unresolved, exception,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
switch (plan) {
.send_exception => |err| try std.testing.expectEqual(error.TestExpectedError, err),
Expand All @@ -180,6 +231,7 @@ test "peer_call_targets promised target planning handles unresolved, exception,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
switch (plan) {
.queue_export_promise => |export_id| try std.testing.expectEqual(@as(u32, 9), export_id),
Expand All @@ -195,6 +247,7 @@ test "peer_call_targets promised target planning handles unresolved, exception,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
switch (plan) {
.handle_resolved => |cap| switch (cap) {
Expand All @@ -213,6 +266,7 @@ test "peer_call_targets promised target planning handles unresolved, exception,
promised,
Hooks.resolvePromisedAnswer,
Hooks.hasUnresolvedPromiseExport,
Hooks.lookupFailedAnswer,
);
switch (plan) {
.handle_resolved => |cap| switch (cap) {
Expand Down
Loading
Loading