amadan
ObserveRoutes returns an error when a read fails after the connection opens, where it panicked
The deferred cleanup closed the named result, and a failing return had already cleared it: a nil dereference in the agent's ownership snapshot on any failure past the connection (a busy database, a missing table). The cleanup now closes what was built. Reproduction: TestObserveRoutesFailingReadIsAnErrorNotAPanic, which panics without the change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+26
−2
internal/registry/observe.go+7 −2internal/registry/observe_test.go+19 −0
CI passed — run details
diff --git a/internal/registry/observe.go b/internal/registry/observe.go| index 43eb59bb7..9b6c91bd1 100644 |
| --- a/internal/registry/observe.go |
| +++ b/internal/registry/observe.go |
| @@ -41,10 +41,15 @@ func (r *Registry) ObserveRoutes(ctx context.Context) (obs *RouteObservation, er |
| db.Close() |
| return nil, err |
| } |
| - obs = &RouteObservation{db: db, conn: conn} |
| + // built, not the named result: "return nil, err" clears the named |
| + // result before the deferred cleanup runs, and the cleanup has to |
| + // close what was built. Closing through the cleared result was a nil |
| + // dereference: any failure after the connection opened was a panic. |
| + built := &RouteObservation{db: db, conn: conn} |
| + obs = built |
| defer func() { |
| if err != nil { |
| - obs.Close() |
| + built.Close() |
| obs = nil |
| } |
| }() |
diff --git a/internal/registry/observe_test.go b/internal/registry/observe_test.go| index dff144a73..83890ab31 100644 |
| --- a/internal/registry/observe_test.go |
| +++ b/internal/registry/observe_test.go |
| @@ -215,3 +215,22 @@ func TestObserveRoutesHelperProcess(t *testing.T) { |
| t.Fatal(err) |
| } |
| } |
| + |
| +// A read that fails after the connection has opened is an error, and the |
| +// connection is released. It used to be a panic: the cleanup closed the |
| +// named result, which the failing return had already cleared. |
| +func TestObserveRoutesFailingReadIsAnErrorNotAPanic(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + reg, err := Open(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + // The file opens and answers PRAGMAs; the rows cannot be read. |
| + if _, err := reg.sql.Exec(`DROP TABLE routes`); err != nil { |
| + t.Fatal(err) |
| + } |
| + obs, err := reg.ObserveRoutes(context.Background()) |
| + if err == nil || obs != nil { |
| + t.Fatalf("ObserveRoutes over a registry with no routes table: obs=%v err=%v, want an error and no observation", obs, err) |
| + } |
| +} |