mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
fix(server): repair the initial-connect outbox envelope and unblock dispatch
ApplyInitialConnectPlan wrote a payload of {match_id,state,action},
omitting event, revision, resource_id, occurred_at and player_ids --
every field deliverStateOutboxEvent requires. Delivery rejected the row,
dispatch returned on the first error so it was never acknowledged, and
because reads are ordered oldest-first it was retried ahead of every
later state_changed event on every 100ms poll. One initial-connect
transition therefore blocked lifecycle delivery for all matches, not
just its own.
Two independent fixes, since either alone leaves the system fragile:
Build envelopes through one validating helper (MarshalOutboxEnvelope)
and convert all five writers to it. A writer that omits a required
field now fails its own transaction instead of committing a row that
can only ever poison the queue. The helper takes revision as int64 so
the -1 "nothing matched" sentinel some CTEs return surfaces as an error
rather than wrapping to a huge uint64.
Make dispatch resilient regardless: a delivery failure is now counted
against that row and the batch continues, with the row dead-lettered
after MaxOutboxDeliveryAttempts so a poison event degrades to one lost
notification instead of a stalled queue. Ordering within an aggregate
is still honoured -- later events of a failed match are deferred, so no
client observes that match's newer state before its older state. An ack
failure still stops the batch, being a database rather than a payload
problem.
Initial-connect events now address every participant, not just the
connected ones: a no-show needs to learn their ticket was failed and a
penalty applied.
This commit is contained in:
+33
-6
@@ -35,7 +35,7 @@ func RunProposalOutboxDispatcher(ctx context.Context, db *sql.DB, service *Servi
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
_ = dispatchOutboxEvents(ctx, dispatcher, events)
|
||||
_ = dispatchOutboxEvents(ctx, db, dispatcher, events)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -61,7 +61,7 @@ func RunResultOutboxDispatcher(ctx context.Context, db *sql.DB, service *Service
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
_ = dispatchOutboxEvents(ctx, dispatcher, events)
|
||||
_ = dispatchOutboxEvents(ctx, db, dispatcher, events)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -87,29 +87,56 @@ func RunStateOutboxDispatcher(ctx context.Context, db *sql.DB, service *Service)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
_ = dispatchOutboxEvents(ctx, dispatcher, events)
|
||||
_ = dispatchOutboxEvents(ctx, db, dispatcher, events)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func dispatchOutboxEvents(ctx context.Context, dispatcher *store.OutboxDispatcher, events []store.OutboxEvent) error {
|
||||
func dispatchOutboxEvents(ctx context.Context, db *sql.DB, dispatcher *store.OutboxDispatcher, events []store.OutboxEvent) error {
|
||||
if len(events) == 0 {
|
||||
return nil
|
||||
}
|
||||
// Use the same delivery-before-ack contract as the general dispatcher,
|
||||
// while keeping the already-filtered batch from being read a second time.
|
||||
//
|
||||
// A delivery failure does not abort the batch. Returning here meant one
|
||||
// undeliverable payload -- reads are oldest-first -- was retried ahead of
|
||||
// every later event of its type on every poll, forever. Instead the failure
|
||||
// is counted against that row (dead-lettering it once exhausted) and the
|
||||
// batch continues.
|
||||
//
|
||||
// Ordering within one aggregate is still honoured: once an event for a
|
||||
// match fails, its later events are left for a subsequent poll so a client
|
||||
// can never observe that match's newer state before its older state. Other
|
||||
// aggregates are independent and proceed.
|
||||
blocked := make(map[string]struct{})
|
||||
var firstErr error
|
||||
for _, event := range events {
|
||||
if event.EventID == "" {
|
||||
return fmt.Errorf("outbox event has no ID")
|
||||
}
|
||||
if _, skip := blocked[event.AggregateID]; skip {
|
||||
continue
|
||||
}
|
||||
if err := dispatcher.Deliver(ctx, event); err != nil {
|
||||
return err
|
||||
blocked[event.AggregateID] = struct{}{}
|
||||
if firstErr == nil {
|
||||
firstErr = err
|
||||
}
|
||||
if db != nil {
|
||||
if _, failErr := store.RecordOutboxDeliveryFailure(ctx, db, event.EventID, err, time.Now().UTC()); failErr != nil {
|
||||
return failErr
|
||||
}
|
||||
}
|
||||
continue
|
||||
}
|
||||
if err := dispatcher.Ack(ctx, event.EventID, time.Now().UTC()); err != nil {
|
||||
// An ack failure is a database problem, not a payload problem;
|
||||
// stop rather than counting it against the event.
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return firstErr
|
||||
}
|
||||
|
||||
func deliverProposalOutboxEvent(_ context.Context, event store.OutboxEvent, service *Service) error {
|
||||
|
||||
@@ -3,6 +3,8 @@ package api
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"slices"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -103,3 +105,53 @@ func TestDeliverStateOutboxEventRoutesLiveAbandonmentLifecycle(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// One malformed row used to abort the whole batch. Because reads are
|
||||
// oldest-first and the row was never acknowledged, it was re-read ahead of
|
||||
// every later event of its type on every 100ms poll -- blocking lifecycle
|
||||
// delivery for all matches indefinitely, not just its own.
|
||||
func TestDispatchOutboxEventsIsNotBlockedByOnePoisonRow(t *testing.T) {
|
||||
delivered := []string{}
|
||||
acked := []string{}
|
||||
dispatcher := &store.OutboxDispatcher{
|
||||
Read: func(context.Context, int) ([]store.OutboxEvent, error) { return nil, nil },
|
||||
Deliver: func(_ context.Context, event store.OutboxEvent) error {
|
||||
delivered = append(delivered, event.EventID)
|
||||
if event.AggregateID == "match-poison" {
|
||||
return errors.New("invalid state outbox payload")
|
||||
}
|
||||
return nil
|
||||
},
|
||||
Ack: func(_ context.Context, eventID string, _ time.Time) error {
|
||||
acked = append(acked, eventID)
|
||||
return nil
|
||||
},
|
||||
}
|
||||
|
||||
events := []store.OutboxEvent{
|
||||
{EventID: "poison-1", AggregateID: "match-poison"},
|
||||
{EventID: "healthy-1", AggregateID: "match-healthy"},
|
||||
{EventID: "poison-2", AggregateID: "match-poison"},
|
||||
{EventID: "healthy-2", AggregateID: "match-other"},
|
||||
}
|
||||
// nil db: the failure counter is exercised against a real PostgreSQL in
|
||||
// the store integration tests; here we assert only batch progress.
|
||||
err := dispatchOutboxEvents(context.Background(), nil, dispatcher, events)
|
||||
if err == nil {
|
||||
t.Fatal("expected the delivery failure to be reported to the caller")
|
||||
}
|
||||
|
||||
for _, eventID := range []string{"healthy-1", "healthy-2"} {
|
||||
if !slices.Contains(acked, eventID) {
|
||||
t.Fatalf("%s was not acknowledged; a poison row still blocks the batch (acked=%v)", eventID, acked)
|
||||
}
|
||||
}
|
||||
if slices.Contains(acked, "poison-1") {
|
||||
t.Fatal("a failed delivery must not be acknowledged")
|
||||
}
|
||||
// Ordering within the failing aggregate is preserved: poison-2 must wait
|
||||
// so no client sees that match's newer state before its older state.
|
||||
if slices.Contains(delivered, "poison-2") {
|
||||
t.Fatalf("later event of a failed aggregate was delivered out of order: %v", delivered)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user