fix(GWC-25,CLI-35,CLI-36): make the empty-ring ReplayGap resumable end to end
An empty replay ring reported oldest_available_sequence = 0 even when gap was
true. Clients follow the documented after_worker_sequence = oldest - 1 formula,
so an unsigned client computed ulong.MaxValue: the follow-up resume replayed
nothing, reported no gap, and the live filter dropped every subsequent event —
a silently dead stream in the headline detach-and-resume scenario, reachable on
default config once ReplayRetentionSeconds (300) age-evicts the ring.
GWC-25: SessionEventDistributor.RegisterWithReplay's empty-ring branch now
reports _highestSequenceSeen + 1 — the next sequence that can possibly be
delivered — when gap is true, so oldest - 1 lands exactly on the highest
observed sequence and the resume delivers everything newer. Still 0 when there
is no gap, where the field is meaningless and never emitted. Nothing is lost:
the evicted interval was unrecoverable either way, and the sentinel's job is to
say "re-snapshot".
CLI-35: the Python CLI fed every stream item into MessageToDict, which raised on
the ReplayGap dataclass and aborted the command after consuming the stream. A
new _event_row helper renders a gap as {"replayGap": {...}} — the same camelCase
shape the Rust CLI emits — and leaves proto events on the existing path.
CLI-36: the Go CLI formatted result.Event on every row, but the library
deliberately clears Event on a gap, so text mode printed
"0 MX_EVENT_FAMILY_UNSPECIFIED" and JSON mode an empty object, discarding the
resume cursors. The loop now branches on result.IsReplayGap() and renders the
typed row in both modes, counting it toward -limit like any other row. The JSON
row's cursors are typed by hand rather than marshalled with protojson: the
proto3 JSON mapping renders 64-bit integers as strings ("7") while the Rust and
Python CLIs emit numbers (7), so going through protojson would have made Go the
only canonical CLI with a different value type.
Docs in the same change: docs/Sessions.md documents the empty-ring sentinel
value and that oldest - 1 is the universal resume formula in both the retained
and fully-evicted cases; docs/CrossLanguageSmokeMatrix.md gains a per-CLI
gap-rendering table covering both client findings, and records exactly what is
and is not comparable across CLIs (same keys and numeric cursors for Rust/Go/
Python; quoted cursors for .NET/Java; differing key order, whitespace, and
container), so a matrix runner compares parsed values rather than raw bytes.
Tests, all written red first and each reproducing its defect verbatim:
- SessionEventDistributorTests: RegisterWithReplayReportsNextDeliverableSequence
WhenRingEmptiedByAge, ...WithRetentionDisabled, and
ResumeUsingSentinelFormulaAfterEmptyRingGapDeliversLiveEvents.
- GatewayEndToEndReconnectReplayTests.ReconnectAfterFullAgeEvictionResumesWith
SentinelFormula — fake-worker e2e resume walk on a fake clock; the fixture now
takes a retention window and a TimeProvider.
- clients/python test_stream_events_renders_replay_gap.
- clients/go TestRunStreamEventsPrintsReplayGap.
GWC-25's ReplayGap.oldest_available_sequence proto-comment amendment is
deliberately deferred to the later codegen wave (see the tracker change log): it
is comment-only but triggers the full five-client regen fan-out.
This commit is contained in:
@@ -57,6 +57,25 @@ type commandReplyOutput struct {
|
||||
Reply json.RawMessage `json:"reply"`
|
||||
}
|
||||
|
||||
// replayGapRow is the JSON row stream-events emits for a reconnect-replay gap:
|
||||
// {"replayGap":{"requestedAfterSequence":N,"oldestAvailableSequence":N}}.
|
||||
//
|
||||
// The cursors are typed by hand rather than marshalled with protojson on
|
||||
// purpose. The proto3 JSON mapping renders 64-bit integers as JSON *strings*
|
||||
// ("7"), but the Rust and Python CLIs emit JSON *numbers* (7) for this row —
|
||||
// routing through protojson would silently make Go the odd one out and break
|
||||
// the cross-language smoke matrix's row comparison. encoding/json renders
|
||||
// uint64 as a number, which is the canonical rendering here.
|
||||
type replayGapRow struct {
|
||||
ReplayGap replayGapCursors `json:"replayGap"`
|
||||
}
|
||||
|
||||
// replayGapCursors is the nested cursor object of replayGapRow.
|
||||
type replayGapCursors struct {
|
||||
RequestedAfterSequence uint64 `json:"requestedAfterSequence"`
|
||||
OldestAvailableSequence uint64 `json:"oldestAvailableSequence"`
|
||||
}
|
||||
|
||||
func main() {
|
||||
if err := runWithIO(context.Background(), os.Args[1:], os.Stdout, os.Stderr); err != nil {
|
||||
fmt.Fprintln(os.Stderr, err)
|
||||
@@ -970,7 +989,31 @@ func runStreamEvents(ctx context.Context, args []string, stdout, stderr io.Write
|
||||
if result.Err != nil {
|
||||
return result.Err
|
||||
}
|
||||
if *jsonOutput {
|
||||
// A reconnect-replay gap is a typed signal, not an event: the library
|
||||
// clears Event on it, so formatting Event here would print a meaningless
|
||||
// zero row and discard the resume cursors the operator needs. Render it
|
||||
// as its own row (matching the Rust CLI) and count it toward -limit like
|
||||
// any other emitted row.
|
||||
if result.IsReplayGap() {
|
||||
if *jsonOutput {
|
||||
row, err := json.Marshal(replayGapRow{
|
||||
ReplayGap: replayGapCursors{
|
||||
RequestedAfterSequence: result.ReplayGap.GetRequestedAfterSequence(),
|
||||
OldestAvailableSequence: result.ReplayGap.GetOldestAvailableSequence(),
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
fmt.Fprintln(stdout, string(row))
|
||||
} else {
|
||||
fmt.Fprintf(
|
||||
stdout,
|
||||
"REPLAY_GAP requested_after=%d oldest_available=%d\n",
|
||||
result.ReplayGap.GetRequestedAfterSequence(),
|
||||
result.ReplayGap.GetOldestAvailableSequence())
|
||||
}
|
||||
} else if *jsonOutput {
|
||||
fmt.Fprintln(stdout, string(mustMarshalProto(result.Event)))
|
||||
} else {
|
||||
fmt.Fprintf(stdout, "%d %s\n", result.Event.GetWorkerSequence(), result.Event.GetFamily())
|
||||
|
||||
@@ -617,3 +617,120 @@ func TestRunWriteBulkVariantRejectsMismatchedHandlesAndValues(t *testing.T) {
|
||||
t.Fatalf("write-bulk mismatched handles/values error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// replayGapFakeGateway streams the gateway's reconnect-replay sentinel (an MxEvent
|
||||
// carrying replay_gap, family UNSPECIFIED, body unset) followed by one normal data
|
||||
// event — exactly what a resume whose cursor predates the retained replay ring sees.
|
||||
type replayGapFakeGateway struct {
|
||||
pb.UnimplementedMxAccessGatewayServer
|
||||
}
|
||||
|
||||
func (g *replayGapFakeGateway) StreamEvents(
|
||||
req *pb.StreamEventsRequest,
|
||||
stream grpc.ServerStreamingServer[pb.MxEvent],
|
||||
) error {
|
||||
sentinel := &pb.MxEvent{
|
||||
SessionId: req.GetSessionId(),
|
||||
Family: pb.MxEventFamily_MX_EVENT_FAMILY_UNSPECIFIED,
|
||||
ReplayGap: &pb.ReplayGap{
|
||||
RequestedAfterSequence: 7,
|
||||
OldestAvailableSequence: 42,
|
||||
},
|
||||
}
|
||||
if err := stream.Send(sentinel); err != nil {
|
||||
return err
|
||||
}
|
||||
return stream.Send(&pb.MxEvent{
|
||||
SessionId: req.GetSessionId(),
|
||||
Family: pb.MxEventFamily_MX_EVENT_FAMILY_ON_DATA_CHANGE,
|
||||
WorkerSequence: 43,
|
||||
})
|
||||
}
|
||||
|
||||
func startReplayGapGateway(t *testing.T) string {
|
||||
t.Helper()
|
||||
|
||||
listener, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
server := grpc.NewServer()
|
||||
pb.RegisterMxAccessGatewayServer(server, &replayGapFakeGateway{})
|
||||
go func() { _ = server.Serve(listener) }()
|
||||
t.Cleanup(func() {
|
||||
server.Stop()
|
||||
_ = listener.Close()
|
||||
})
|
||||
return listener.Addr().String()
|
||||
}
|
||||
|
||||
// TestRunStreamEventsPrintsReplayGap pins CLI-36: the CLI must render the typed
|
||||
// ReplayGap signal in both output modes instead of formatting the library's
|
||||
// cleared Event field (which printed "0 MX_EVENT_FAMILY_UNSPECIFIED" in text mode
|
||||
// and an empty object in JSON mode, destroying the resume cursors).
|
||||
func TestRunStreamEventsPrintsReplayGap(t *testing.T) {
|
||||
endpoint := startReplayGapGateway(t)
|
||||
|
||||
baseArgs := []string{
|
||||
"stream-events",
|
||||
"-endpoint", endpoint,
|
||||
"-plaintext",
|
||||
"-api-key", "test",
|
||||
"-session-id", "gap-session",
|
||||
"-after-worker-sequence", "7",
|
||||
"-limit", "2",
|
||||
}
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
if err := runWithIO(t.Context(), baseArgs, &stdout, &stderr); err != nil {
|
||||
t.Fatalf("runWithIO() error = %v; stderr = %s", err, stderr.String())
|
||||
}
|
||||
text := stdout.String()
|
||||
if !strings.Contains(text, "REPLAY_GAP requested_after=7 oldest_available=42") {
|
||||
t.Fatalf("stream-events text output missing typed gap row: %q", text)
|
||||
}
|
||||
if strings.Contains(text, "0 MX_EVENT_FAMILY_UNSPECIFIED") {
|
||||
t.Fatalf("stream-events text output destroyed the gap into a zero row: %q", text)
|
||||
}
|
||||
if !strings.Contains(text, "43 MX_EVENT_FAMILY_ON_DATA_CHANGE") {
|
||||
t.Fatalf("stream-events text output dropped the normal event: %q", text)
|
||||
}
|
||||
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if err := runWithIO(t.Context(), append(baseArgs, "-json"), &stdout, &stderr); err != nil {
|
||||
t.Fatalf("runWithIO(-json) error = %v; stderr = %s", err, stderr.String())
|
||||
}
|
||||
|
||||
lines := strings.Split(strings.TrimSpace(stdout.String()), "\n")
|
||||
if len(lines) != 2 {
|
||||
t.Fatalf("stream-events -json emitted %d rows, want 2: %q", len(lines), stdout.String())
|
||||
}
|
||||
|
||||
// The cursors must decode as JSON numbers, not the strings the proto3 JSON
|
||||
// mapping would produce for 64-bit fields: the Rust and Python CLIs emit
|
||||
// numbers, and the cross-language matrix compares these rows across clients.
|
||||
var gapRow struct {
|
||||
ReplayGap *struct {
|
||||
RequestedAfterSequence uint64 `json:"requestedAfterSequence"`
|
||||
OldestAvailableSequence uint64 `json:"oldestAvailableSequence"`
|
||||
} `json:"replayGap"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(lines[0]), &gapRow); err != nil {
|
||||
t.Fatalf("parse gap row: %v\nrow: %s", err, lines[0])
|
||||
}
|
||||
if gapRow.ReplayGap == nil {
|
||||
t.Fatalf("stream-events -json first row is not a replayGap row: %s", lines[0])
|
||||
}
|
||||
if gapRow.ReplayGap.RequestedAfterSequence != 7 || gapRow.ReplayGap.OldestAvailableSequence != 42 {
|
||||
t.Fatalf("stream-events -json gap cursors = %+v, want 7/42", *gapRow.ReplayGap)
|
||||
}
|
||||
// Belt and braces on the value type: a protojson-rendered `"7"` already
|
||||
// fails the decode above (encoding/json rejects a JSON string for an
|
||||
// untagged uint64 field), but assert the raw bytes so a regression names
|
||||
// the real problem instead of surfacing as an opaque unmarshal error.
|
||||
if !strings.Contains(lines[0], `"requestedAfterSequence":7`) ||
|
||||
!strings.Contains(lines[0], `"oldestAvailableSequence":42`) {
|
||||
t.Fatalf("stream-events -json gap cursors must be JSON numbers, got: %s", lines[0])
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ from zb_mom_ww_mxgateway import __version__
|
||||
from zb_mom_ww_mxgateway.auth import redact_secret
|
||||
from zb_mom_ww_mxgateway.client import GatewayClient
|
||||
from zb_mom_ww_mxgateway.errors import MxGatewayError
|
||||
from zb_mom_ww_mxgateway.events import ReplayGap
|
||||
from zb_mom_ww_mxgateway.galaxy import GalaxyRepositoryClient
|
||||
from zb_mom_ww_mxgateway.generated import galaxy_repository_pb2 as galaxy_pb
|
||||
from zb_mom_ww_mxgateway.generated import mxaccess_gateway_pb2 as pb
|
||||
@@ -1103,7 +1104,7 @@ async def _stream_events(**kwargs: Any) -> dict[str, Any]:
|
||||
max_events=kwargs["max_events"],
|
||||
timeout=kwargs["timeout"],
|
||||
)
|
||||
return {"events": [_message_dict(event) for event in events]}
|
||||
return {"events": [_event_row(event) for event in events]}
|
||||
|
||||
|
||||
async def _stream_alarms(**kwargs: Any) -> dict[str, Any]:
|
||||
@@ -1500,14 +1501,14 @@ async def _collect_events(
|
||||
*,
|
||||
max_events: int,
|
||||
timeout: float,
|
||||
) -> list[pb.MxEvent]:
|
||||
) -> list[pb.MxEvent | ReplayGap]:
|
||||
if max_events > MAX_AGGREGATE_EVENTS:
|
||||
raise click.BadParameter(
|
||||
f"must be less than or equal to {MAX_AGGREGATE_EVENTS}",
|
||||
param_hint="--max-events",
|
||||
)
|
||||
|
||||
collected: list[pb.MxEvent] = []
|
||||
collected: list[pb.MxEvent | ReplayGap] = []
|
||||
iterator = events.__aiter__()
|
||||
try:
|
||||
while len(collected) < max_events:
|
||||
@@ -1630,3 +1631,26 @@ def _message_dict(message: Any) -> dict[str, Any]:
|
||||
preserving_proto_field_name=False,
|
||||
use_integers_for_enums=False,
|
||||
)
|
||||
|
||||
|
||||
def _event_row(item: Any) -> dict[str, Any]:
|
||||
"""Render one item of an event stream as a JSON row.
|
||||
|
||||
``Session.stream_events`` yields ``MxEvent | ReplayGap``. ``ReplayGap`` is a
|
||||
plain dataclass, so it has no protobuf descriptor and cannot go through
|
||||
``MessageToDict`` — it gets its own distinct row instead, matching the shape
|
||||
the Rust and Go CLIs emit so the cross-language matrix can compare rows.
|
||||
Keys are camelCase for the same reason ``_message_dict`` uses
|
||||
``preserving_proto_field_name=False``. The gap is always rendered: never
|
||||
dropped, and never re-synthesized into an event.
|
||||
"""
|
||||
|
||||
if isinstance(item, ReplayGap):
|
||||
return {
|
||||
"replayGap": {
|
||||
"requestedAfterSequence": item.requested_after_sequence,
|
||||
"oldestAvailableSequence": item.oldest_available_sequence,
|
||||
},
|
||||
}
|
||||
|
||||
return _message_dict(item)
|
||||
|
||||
@@ -817,3 +817,65 @@ def test_write_secured_command_does_not_echo_value_on_failure(
|
||||
def test_write_secured_and_authenticate_user_commands_are_registered() -> None:
|
||||
names = set(main.commands)
|
||||
assert {"write-secured", "authenticate-user"} <= names
|
||||
|
||||
|
||||
class _FakeReplayGapSession:
|
||||
"""Session stand-in whose event stream starts with a ReplayGap sentinel.
|
||||
|
||||
Mirrors what ``Session.stream_events`` yields on a resume that predates the
|
||||
gateway's retained replay ring: the typed gap first, then normal events.
|
||||
"""
|
||||
|
||||
def __init__(self, gap, event) -> None:
|
||||
self._gap = gap
|
||||
self._event = event
|
||||
|
||||
def stream_events(self, **_kwargs):
|
||||
async def _iterate():
|
||||
yield self._gap
|
||||
yield self._event
|
||||
|
||||
return _iterate()
|
||||
|
||||
|
||||
def test_stream_events_renders_replay_gap(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""CLI-35: a ReplayGap renders as its own JSON row instead of crashing the command."""
|
||||
from zb_mom_ww_mxgateway.events import ReplayGap
|
||||
from zb_mom_ww_mxgateway.generated import mxaccess_gateway_pb2 as pb
|
||||
|
||||
gap = ReplayGap(requested_after_sequence=7, oldest_available_sequence=42)
|
||||
event = pb.MxEvent(session_id="cli-test-session", worker_sequence=43)
|
||||
|
||||
async def fake_connect(options, **_kwargs):
|
||||
return _FakeAsyncClient()
|
||||
|
||||
monkeypatch.setattr(commands_module.GatewayClient, "connect", fake_connect)
|
||||
monkeypatch.setattr(
|
||||
commands_module,
|
||||
"_session",
|
||||
lambda _client, _session_id: _FakeReplayGapSession(gap, event),
|
||||
)
|
||||
|
||||
result = CliRunner().invoke(
|
||||
main,
|
||||
[
|
||||
"stream-events",
|
||||
"--plaintext",
|
||||
"--session-id",
|
||||
"cli-test-session",
|
||||
"--after-worker-sequence",
|
||||
"7",
|
||||
"--max-events",
|
||||
"2",
|
||||
"--json",
|
||||
],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
rows = json.loads(result.output)["events"]
|
||||
assert rows[0] == {
|
||||
"replayGap": {"requestedAfterSequence": 7, "oldestAvailableSequence": 42},
|
||||
}
|
||||
# The gap is rendered, never swallowed, and the normal event still follows it.
|
||||
assert "replayGap" not in rows[1]
|
||||
assert rows[1]["workerSequence"] == "43"
|
||||
|
||||
Reference in New Issue
Block a user