Say things a player can act on, and test the crash containment

cli.go

@@ -90,6 +90,11 @@ func runCommand(args []string) bool {
 // gameIDRe matches a marketplace id, e.g. "aviorstudio/brickough".
 var gameIDRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*/[a-z0-9][a-z0-9-]*$`)
 
+// slugOnlyRe matches half of one — "brickough" with the author left off. The
+// character class is gameIDRe's without the slash, so nothing with a path, a
+// scheme, or a file extension can reach it.
+var slugOnlyRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)
+
 func cmdAdd(args []string) error {
 	if len(args) != 1 {
 		return fmt.Errorf("usage: termcade add <author/slug | file | url>")
@@ -105,6 +110,16 @@ func cmdAdd(args []string) error {
 		return fmt.Errorf("versions cannot be pinned — `termcade add %s` installs what %s currently ships", id, id)
 	}
 
+	// Forgetting the author is the likeliest way to get this wrong, and
+	// "open asteroid: no such file or directory" answers a question about the
+	// filesystem that nobody asked. Anything with a slash, a scheme, or a
+	// .tcade on the end meant a file or a URL and is left alone.
+	if slugOnlyRe.MatchString(src) {
+		if _, statErr := os.Stat(src); statErr != nil {
+			return fmt.Errorf("%q is missing an author — marketplace ids look like author/slug, e.g. aviorstudio/%s", src, src)
+		}
+	}
+
 	session, err := registry.LoadSession()
 	if err != nil {
 		return err
@@ -271,7 +286,16 @@ func cmdList() error {
 	rt := plugin.NewRuntime(context.Background())
 	defer rt.Close()
 
-	for _, g := range discoverGames(rt) {
+	games := discoverGames(rt)
+	// The TUI has empty states everywhere and the CLI had none, so removing
+	// your last game made `list` answer with nothing at all — which reads as
+	// a broken command rather than an empty arcade.
+	if len(games) == 0 {
+		fmt.Println("no games installed — run `termcade` and press m for the marketplace")
+		return nil
+	}
+
+	for _, g := range games {
 		if g.Err != nil {
 			fmt.Printf("%-24s %-28s broken: %v\n", g.Info.Title, g.Info.ID, g.Err)
 			continue

internal/engine/safe_test.go

@@ -0,0 +1,187 @@
+package engine
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/aviorstudio/termcade/sdk"
+)
+
+// panicAt is a game that panics in exactly one method and counts every call
+// it receives, so a test can prove the calls after a crash never arrive.
+type panicAt struct {
+	where  string
+	calls  map[string]int
+	closed bool
+	// closeErr is returned by Close; closePanics makes it panic instead.
+	closeErr    error
+	closePanics bool
+}
+
+func newPanicAt(where string) *panicAt {
+	return &panicAt{where: where, calls: map[string]int{}}
+}
+
+func (g *panicAt) hit(what string) {
+	g.calls[what]++
+	if g.where == what {
+		panic("boom in " + what)
+	}
+}
+
+func (g *panicAt) Info() sdk.Info {
+	g.hit("Info")
+	return sdk.Info{ID: "test/game", Title: "TEST", PixelW: 64, PixelH: 40}
+}
+func (g *panicAt) Reset()              { g.hit("Reset") }
+func (g *panicAt) HandleKey(sdk.Key)   { g.hit("HandleKey") }
+func (g *panicAt) HandleKeyUp(sdk.Key) { g.hit("HandleKeyUp") }
+func (g *panicAt) Draw(c *sdk.Canvas)  { g.hit("Draw") }
+func (g *panicAt) Update() sdk.Status  { g.hit("Update"); return sdk.StatusRunning }
+func (g *panicAt) Score() int          { g.hit("Score"); return 7 }
+func (g *panicAt) HUD() sdk.HUD        { g.hit("HUD"); return sdk.HUD{} }
+func (g *panicAt) Close() error {
+	g.closed = true
+	if g.closePanics {
+		panic("boom in Close")
+	}
+	return g.closeErr
+}
+
+// The claim the README makes: a broken game shows a crash screen instead of
+// taking the arcade down. Every method has to hold that, not just the ones
+// that happen to be covered elsewhere.
+func TestSafeGameContainsAPanicInEveryMethod(t *testing.T) {
+	for _, where := range []string{"Reset", "HandleKey", "HandleKeyUp", "Draw", "Update", "Score", "HUD"} {
+		t.Run(where, func(t *testing.T) {
+			s := Safe(newPanicAt(where))
+			if s.Err() != nil {
+				t.Fatalf("wrapping alone crashed: %v", s.Err())
+			}
+
+			// Whichever method panics, driving the whole surface must not
+			// propagate it — this call is what would take the arcade down.
+			drive(s)
+
+			if s.Err() == nil {
+				t.Fatalf("a panic in %s was swallowed without latching an error", where)
+			}
+			if got := s.Err().Error(); !strings.Contains(got, where) {
+				t.Errorf("error %q does not name the method that failed", got)
+			}
+		})
+	}
+}
+
+// A panic during Info is the awkward case: it happens inside Safe itself,
+// before the caller holds anything.
+func TestSafeGamePanicInInfoIsContained(t *testing.T) {
+	s := Safe(newPanicAt("Info"))
+	if s.Err() == nil {
+		t.Fatal("a panic in Info was swallowed")
+	}
+	// Info is still answerable, with a zero value rather than a crash, so a
+	// caller can render a crash screen without a second check.
+	_ = s.Info()
+	drive(s)
+}
+
+// Latching is what stops a game that panics every frame from burning the
+// arcade's time: the first crash wins and nothing reaches the game again.
+func TestSafeGameStopsCallingACrashedGame(t *testing.T) {
+	g := newPanicAt("Update")
+	s := Safe(g)
+
+	s.Update()
+	first := s.Err()
+	if first == nil {
+		t.Fatal("no error latched")
+	}
+	callsAtCrash := g.calls["Update"]
+
+	for range 5 {
+		s.Update()
+		s.Draw(nil)
+		s.HandleKey(sdk.KeyA)
+	}
+
+	if g.calls["Update"] != callsAtCrash {
+		t.Errorf("a crashed game was called %d more times", g.calls["Update"]-callsAtCrash)
+	}
+	if g.calls["Draw"] != 0 || g.calls["HandleKey"] != 0 {
+		t.Errorf("calls reached a crashed game: %v", g.calls)
+	}
+	if s.Err() != first {
+		t.Error("a later call replaced the original crash; the first one is the useful one")
+	}
+}
+
+// Cleanup runs after a crash, which is exactly when it matters — a wasm
+// instance still needs releasing.
+func TestSafeGameClosesAfterACrash(t *testing.T) {
+	g := newPanicAt("Update")
+	s := Safe(g)
+	s.Update()
+
+	if err := s.Close(); err != nil {
+		t.Fatalf("Close after a crash: %v", err)
+	}
+	if !g.closed {
+		t.Error("a crashed game was never closed")
+	}
+}
+
+func TestSafeGameContainsAPanicInClose(t *testing.T) {
+	g := newPanicAt("")
+	g.closePanics = true
+
+	err := Safe(g).Close()
+	if err == nil {
+		t.Fatal("a panic in Close escaped as success")
+	}
+	if !strings.Contains(err.Error(), "Close") {
+		t.Errorf("error %q does not name Close", err)
+	}
+}
+
+func TestSafeGameReportsACloseError(t *testing.T) {
+	sentinel := errors.New("released badly")
+	g := newPanicAt("")
+	g.closeErr = sentinel
+
+	if err := Safe(g).Close(); !errors.Is(err, sentinel) {
+		t.Fatalf("Close returned %v, want the game's own error", err)
+	}
+}
+
+// A healthy game is left alone: containment must not cost correctness.
+func TestSafeGamePassesThroughWhenHealthy(t *testing.T) {
+	g := newPanicAt("")
+	s := Safe(g)
+
+	if got := s.Info().ID; got != "test/game" {
+		t.Errorf("Info().ID = %q", got)
+	}
+	if got := s.Update(); got != sdk.StatusRunning {
+		t.Errorf("Update() = %v, want StatusRunning", got)
+	}
+	if got := s.Score(); got != 7 {
+		t.Errorf("Score() = %d, want 7", got)
+	}
+	if s.Err() != nil {
+		t.Errorf("a healthy game latched %v", s.Err())
+	}
+}
+
+// drive calls every method that can panic. Draw gets a nil canvas on purpose:
+// the wrapper must not care what the game does with it.
+func drive(s *SafeGame) {
+	s.Reset()
+	s.HandleKey(sdk.KeyA)
+	s.HandleKeyUp(sdk.KeyA)
+	s.Draw(nil)
+	s.Update()
+	s.Score()
+	s.HUD()
+}

internal/registry/client.go

@@ -1,11 +1,12 @@
 // Package registry is the termca.de client: marketplace catalog, package
 // resolution, account auth, and the user's library.
 //
-// The registry stores no packages. It is an index that answers "where does
-// this version live, and what must it hash to" — the bytes come from the
-// GitHub release the author published, and are verified here against the
-// digest the registry recorded when it validated them. Browsing and
-// installing are anonymous; account operations send the session token.
+// The registry stores no packages. It is an index that answers "which release
+// should I install, and what must it hash to" — the bytes originate from the
+// GitHub release the author published, arrive through the registry, and are
+// verified here against the digest it recorded when it validated them.
+// Browsing is anonymous; installing and account operations send the session
+// token.
 package registry
 
 import (
@@ -40,6 +41,26 @@ const maxPackageSize = 64 << 20
 // ErrLoginRequired distinguishes "you need an account" from real failures.
 var ErrLoginRequired = errors.New("login required")
 
+// ErrUnreachable is the marketplace not answering at all: no network, or
+// nothing listening where the registry is supposed to be.
+//
+// It carries no transport detail. Go's is accurate and useless to a player —
+// `Get "http://127.0.0.1:8080/v1/games/aviorstudio/tetris/resolve?abi=1":
+// dial tcp 127.0.0.1:8080: connect: connection refused` names a host, a port,
+// a query string and a syscall, none of which anyone can act on, and the
+// arcade renders it into a TUI notice. TERMCADE_DEBUG puts it back for
+// whoever is actually debugging.
+var ErrUnreachable = errors.New("the marketplace is not answering — check your connection and try again")
+
+// unreachable wraps a transport failure as ErrUnreachable, keeping the
+// original only when someone asked for it.
+func unreachable(err error) error {
+	if os.Getenv("TERMCADE_DEBUG") != "" {
+		return fmt.Errorf("%w: %v", ErrUnreachable, err)
+	}
+	return ErrUnreachable
+}
+
 // Game is one marketplace catalog entry. The version and requirements are
 // those of its newest release; a game with none reports has_package false.
 type Game struct {
@@ -133,20 +154,26 @@ func (c *Client) do(method, path string, body, out any) error {
 	}
 	resp, err := c.http.Do(req)
 	if err != nil {
-		return fmt.Errorf("registry unreachable: %w", err)
+		return unreachable(err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode == http.StatusUnauthorized {
 		return ErrLoginRequired
 	}
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		// The registry writes its own messages for a person to read, so they
+		// are passed through unprefixed — the CLI already says "termcade:",
+		// and "termcade: registry: ..." is a stutter, not attribution.
 		var msg apiMessage
 		raw, _ := io.ReadAll(io.LimitReader(resp.Body, 8<<10))
 		if json.Unmarshal(raw, &msg) == nil && msg.Message != "" {
-			return fmt.Errorf("registry: %s", msg.Message)
+			return errors.New(msg.Message)
+		}
+		if resp.StatusCode >= 500 {
+			return fmt.Errorf("the marketplace is having trouble (HTTP %d) — try again shortly", resp.StatusCode)
 		}
-		return fmt.Errorf("registry: HTTP %d", resp.StatusCode)
+		return fmt.Errorf("the marketplace refused that request (HTTP %d)", resp.StatusCode)
 	}
 	if out == nil {
 		return nil
@@ -211,7 +238,7 @@ func (c *Client) Download(author, slug string) (string, error) {
 
 	resp, err := c.http.Do(req)
 	if err != nil {
-		return "", fmt.Errorf("registry unreachable: %w", err)
+		return "", unreachable(err)
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode == http.StatusUnauthorized {
@@ -221,9 +248,12 @@ func (c *Client) Download(author, slug string) (string, error) {
 		var msg apiMessage
 		raw, _ := io.ReadAll(io.LimitReader(resp.Body, 8<<10))
 		if json.Unmarshal(raw, &msg) == nil && msg.Message != "" {
-			return "", fmt.Errorf("downloading %s: %s", resolved.ID, msg.Message)
+			return "", fmt.Errorf("could not download %s: %s", resolved.ID, msg.Message)
+		}
+		if resp.StatusCode >= 500 {
+			return "", fmt.Errorf("could not download %s: the marketplace is having trouble — try again shortly", resolved.ID)
 		}
-		return "", fmt.Errorf("downloading %s: HTTP %d", resolved.ID, resp.StatusCode)
+		return "", fmt.Errorf("could not download %s (HTTP %d)", resolved.ID, resp.StatusCode)
 	}
 
 	tmp, err := os.CreateTemp("", slug+"-*.tcade")

internal/shell/market.go

@@ -121,7 +121,10 @@ func (m Model) updateMarketMsg(msg tea.Msg) (Model, tea.Cmd, bool) {
 		m.market.busy = false
 		m.market.loaded = true
 		if msg.err != nil {
-			m.market.notice = "marketplace unreachable: " + msg.err.Error()
+			// Unprefixed: the client writes messages meant to be read, and
+			// "marketplace unreachable: the marketplace is not answering"
+			// says it twice.
+			m.market.notice = msg.err.Error()
 			return m, nil, true
 		}
 		m.market.games = msg.games