Answer a panicking verb with an error, say flag errors in the answer, and leave refused logins out of D15

Read verbs now run in the serving process, where a panic would end every call; refused logins are nobody's reconnect loop (review of hq issue 327).
2026-10-08 21:08:34 +02:00
parent 1e04670052
commit cc7fb99f29
9 changed files with 110 additions and 33 deletions
@@ -121,6 +121,7 @@ func conditionsCommand(ctx context.Context, args []string) error {

 func listConditions(ctx context.Context, args []string, w io.Writer) error {
 	set := flag.NewFlagSet("conditions", flag.ContinueOnError)
+	usageTo(set, w)
 	scope := set.String("scope", "", "only this scope: "+strings.Join(conditions.Scopes, ", "))
 	severity := set.String("severity", "", "only urgent, or only warning")
 	machine := set.String("machine", "", "only those about this machine")
@@ -236,6 +237,7 @@ func conditionLines(list []conditions.Condition, now time.Time) []string {

 func showCondition(ctx context.Context, args []string, w io.Writer) error {
 	set := flag.NewFlagSet("conditions show", flag.ContinueOnError)
+	usageTo(set, w)
 	asJSON := set.Bool("json", false, "as data")
 	rest, err := parseAround(set, args)
 	if err != nil {
@@ -381,6 +383,7 @@ func parseFor(s string) (time.Duration, error) {

 func conditionHistory(ctx context.Context, args []string, w io.Writer) error {
 	set := flag.NewFlagSet("conditions history", flag.ContinueOnError)
+	usageTo(set, w)
 	days := set.Int("days", 7, "how many days back, at most 90")
 	key := set.String("key", "", "only this condition")
 	asJSON := set.Bool("json", false, "as data")
@@ -254,6 +254,7 @@ func handActCommand(ctx context.Context, args []string) error {
 // listHandActs is `hand-acts`: what was done by hand lately, and the causes done more than once.
 func listHandActs(ctx context.Context, args []string, w io.Writer) error {
 	set := flag.NewFlagSet("hand-acts", flag.ContinueOnError)
+	usageTo(set, w)
 	days := set.Int("days", 14, "how many days back")
 	asJSON := set.Bool("json", false, "as data")
 	if _, err := parseAround(set, args); err != nil {
@@ -52,6 +52,7 @@ func queueCommand(ctx context.Context, args []string) error {
 // listQueue is `queue`: the build queue, as a person reads it or as JSON.
 func listQueue(ctx context.Context, args []string, w io.Writer) error {
 	set := flag.NewFlagSet("queue", flag.ContinueOnError)
+	usageTo(set, w)
 	asJSON := set.Bool("json", false, "the queue as JSON")
 	if _, err := parseAround(set, args); err != nil {
 		return err
@@ -64,7 +64,12 @@ func probeReconnects(ctx context.Context, d *doctor) ([]conditions.Observation,
 	if len(on) == 0 {
 		return nil, nil // no bus module assigned: a mesh whose bus is not the mesh's module
 	}
-	read, err := askClosed(ctx, d.js.Conn(), on[0])
+	return reconnectsOn(ctx, d.js.Conn(), on[0])
+}
+
+// reconnectsOn asks the bus module on its machine and says who reconnects in a loop.
+func reconnectsOn(ctx context.Context, conn *nats.Conn, node string) ([]conditions.Observation, error) {
+	read, err := askClosed(ctx, conn, node)
 	if isNothingServes(err) {
 		// A bus module older than its tool has nothing it can say, which is not a failure of the probe.
 		return nil, nil
@@ -100,6 +105,11 @@ func reconnecting(read closedConnections) []conditions.Observation {
 	}
 	var out []conditions.Observation
 	for _, u := range read.Users {
+		if u.User == "" || strings.HasPrefix(u.User, "(") {
+			// Refused before it logged in: no client of the mesh's, so nobody's reconnect loop. A login
+			// refused again and again is a question of its own, not this probe's.
+			continue
+		}
 		perHour := float64(u.Dropped) / hours
 		if perHour <= reconnectBound {
 			continue
@@ -112,6 +112,8 @@ func TestAClientReconnectingInALoopIsSaid(t *testing.T) {
 			{"name":"ace.node-tools","closed":40,"dropped":40,"reasons":{"Stale Connection":30,"Read Error":10}}]},
 		{"user":"controller","closed":300,"dropped":0,"names":[
 			{"name":"mesh-controller verb delivery-check","closed":300,"dropped":0,"reasons":{"Client Closed":300}}]},
+		{"user":"(no user: refused before it logged in)","closed":90,"dropped":90,"names":[{"name":"","closed":90,
+			"dropped":90,"reasons":{"Authentication Failure":90}}]},
 		{"user":"node.anchor","closed":5,"dropped":5,"names":[{"name":"mesh-host/anchor","closed":5,"dropped":5,
 			"reasons":{"Read Error":5}}]}]}`), &read); err != nil {
 		t.Fatal(err)
@@ -154,3 +156,34 @@ func TestTheBusModuleIsAskedWhoClosedConnections(t *testing.T) {
 		t.Fatalf("%+v", read)
 	}
 }
+
+// A bus module older than the tool answers nothing to the question: the probe passes over it quietly.
+func TestAnOlderBusModuleIsPassedOverQuietly(t *testing.T) {
+	bus := testbus.Start(t)
+	conn, err := nats.Connect(bus.ClientURL())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+	found, err := reconnectsOn(context.Background(), conn, "anchor")
+	if err != nil || len(found) != 0 {
+		t.Fatalf("a bus module without the tool: %v %v", found, err)
+	}
+}
+
+// A verb answered in the serving controller says its flag errors in its answer, not in the controller's log.
+func TestAFlagErrorIsSaidInTheAnswer(t *testing.T) {
+	bus := testbus.Start(t)
+	serving, err := broker.Dial(bus.ClientURL())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer serving.Close()
+	was := servingBus.Load()
+	servingBus.Store(serving)
+	defer servingBus.Store(was)
+	answer, read := readHere(context.Background(), []string{"hand-acts", "--bogus"})
+	if !read || answer.OK || !strings.Contains(answer.Output, "flag provided but not defined") {
+		t.Fatalf("answered %+v", answer)
+	}
+}
@@ -3,10 +3,8 @@ package main
 import (
 	"context"
 	"encoding/json"
-	"os"
 	"os/exec"
 	"path/filepath"
-	"sync"
 	"testing"
 	"time"

@@ -70,45 +68,21 @@ func TestReplay327(t *testing.T) {
 	}
 }

-// The binary a verb run as a process of its own is, built once for the tests that run one.
-var verbBinary struct {
-	once sync.Once
-	path string
-	err  error
-}
-
-// asAProcess makes a verb that runs as a process of its own run this package's binary, on the bus at url.
+// asAProcess makes a verb that runs as a process of its own run this package's binary, on the bus at url:
+// built into the test's own directory, which goes with the test.
 func asAProcess(t *testing.T, url string) {
 	t.Helper()
-	verbBinary.once.Do(func() {
-		dir, err := os.MkdirTemp("", "mesh-controller-verb-")
-		if err != nil {
-			verbBinary.err = err
-			return
-		}
-		verbBinary.path = filepath.Join(dir, "mesh-controller")
-		out, err := exec.Command("go", "build", "-o", verbBinary.path, ".").CombinedOutput()
-		if err != nil {
-			verbBinary.err = &buildError{out: string(out), err: err}
-		}
-	})
-	if verbBinary.err != nil {
-		t.Fatalf("the controller could not be built to run a verb as its own process: %v", verbBinary.err)
+	path := filepath.Join(t.TempDir(), "mesh-controller")
+	if out, err := exec.Command("go", "build", "-o", path, ".").CombinedOutput(); err != nil {
+		t.Fatalf("the controller could not be built to run a verb as its own process: %v: %s", err, out)
 	}
 	was := ownImage
-	ownImage = func() string { return verbBinary.path }
+	ownImage = func() string { return path }
 	t.Cleanup(func() { ownImage = was })
 	t.Setenv(broker.NATSVar, url)
 	t.Setenv(broker.CertificateVar, "")
 }

-type buildError struct {
-	out string
-	err error
-}
-
-func (e *buildError) Error() string { return e.err.Error() + ": " + e.out }
-
 // closedNames are the names of the connections the bus saw closed.
 func closedNames(t *testing.T, bus *server.Server) []string {
 	t.Helper()
@@ -1,7 +1,10 @@
 package main

 import (
+	"flag"
 	"fmt"
+	"io"
+	"os"
 	"sync/atomic"

 	"github.com/novox/mesh-controller/internal/broker"
@@ -36,3 +39,11 @@ func aBus() (*broker.JetStream, error) {
 	}
 	return js, nil
 }
+
+// usageTo sends a command's flag errors and usage to where its answer goes when that is not this process's
+// output: a verb answered in the serving controller says them in its answer, not in the controller's log.
+func usageTo(set *flag.FlagSet, w io.Writer) {
+	if w != os.Stdout {
+		set.SetOutput(w)
+	}
+}