diff --git a/modules/messenger/cmd/messenger/details_test.go b/modules/messenger/cmd/messenger/details_test.go index 8dba0d35..def1cb06 100644 --- a/modules/messenger/cmd/messenger/details_test.go +++ b/modules/messenger/cmd/messenger/details_test.go @@ -164,3 +164,36 @@ func TestDetailsOnAMessageWithAnOpenAskNeverAnswersIt(t *testing.T) { t.Errorf("Details ended the ask: %s", st) } } + +// A channel that stays unready makes no ticket at its tries: composing a message makes its Details ticket, +// and tickets made at every try would push out those of the messages already shown. +func TestAnUnreadyChannelPushesOutNoShownDetails(t *testing.T) { + h, tg, _, c, _ := newHolder(t) + h.Desk = nil + k := cond("machine.ace.silent", Warning, "the home server is silent") + k.Raised = c.now() + h.Condition(EventRaised, k) + settle(h, c) + tok := detailsOf(t, tg.sends[0]) + k.Severity = Urgent + h.Condition(EventChanged, k) + tg.notReady = fmt.Errorf("telegram is away") + for i := 0; i < 3*MostDetails; i++ { + c.pass(2 * time.Minute) + h.Flush() + } + if n := len(h.open[k.Key].Tokens); n != 1 { + t.Errorf("%d tickets after the tries, want the one shown", n) + } + tg.notReady = nil + c.pass(2 * time.Minute) + h.Flush() + if len(tg.sends) != 2 { + t.Fatalf("%d messages, want 2", len(tg.sends)) + } + for _, tk := range []string{tok, detailsOf(t, tg.sends[1])} { + if out, err := h.Chosen("telegram", choiceOf(tk, "42", c.now())); err != nil || out != "shown" { + t.Errorf("Details: %q %v", out, err) + } + } +} diff --git a/modules/messenger/cmd/messenger/holder.go b/modules/messenger/cmd/messenger/holder.go index f72279b3..4ef00ac2 100644 --- a/modules/messenger/cmd/messenger/holder.go +++ b/modules/messenger/cmd/messenger/holder.go @@ -142,7 +142,7 @@ type Record struct { Explanation string `json:"explanation,omitempty"` Resolved string `json:"resolved,omitempty"` // Needs is what the operator does, empty for nothing; Actions the answers offered, each by a token - // (Tokens: token -> the action's index, or -1 for the details) a channel hands back when chosen. + // (Tokens: token -> its grant; Index -1 is Details, one per message shown) a channel hands back when chosen. Needs string `json:"needs,omitempty"` Actions []Action `json:"actions,omitempty"` // Kept as "grants": the build before ADR 0258 kept "tokens" as token -> index, which this form cannot diff --git a/modules/messenger/cmd/messenger/outbox.go b/modules/messenger/cmd/messenger/outbox.go index d3c473a3..1b264e4d 100644 --- a/modules/messenger/cmd/messenger/outbox.go +++ b/modules/messenger/cmd/messenger/outbox.go @@ -243,6 +243,14 @@ func (h *Holder) flushChannel(ch Channel, now time.Time, force bool) { if len(list) == 0 || (!force && !h.due(ch, list, now)) { return } + taken := len(list) + // Asked before the messages are composed: composing one makes its Details ticket, and a channel that + // stays unready would make one at every try, pushing out the tickets of messages already shown (issue 371). + if err := ch.Ready(); err != nil { + h.noteChannel(name, err) + h.failed(ch, list, taken, now, err) + return + } var entries []entry for _, it := range list { r, m := h.resolve(it, now, name) @@ -251,16 +259,10 @@ func (h *Holder) flushChannel(ch Channel, now time.Time, force bool) { } entries = append(entries, entry{it, r, *m}) } - taken := len(list) if len(entries) == 0 { h.drop(name, taken) return } - if err := ch.Ready(); err != nil { - h.noteChannel(name, err) - h.failed(ch, list, taken, now, err) - return - } if !h.allow(name) { // The last line of defence: said once, in a message of its own, and the rest held. h.mu.Lock()