fix: attach resource context to reported listener errors; stop retrying permission-denied

AGENTS.md

@@ -101,6 +101,16 @@ Preserve these unless the task explicitly changes them.
   replace a document.
 - Collection `add`, `update`, and `remove` require the first snapshot. They
   bail before the initial snapshot to avoid clobbering unknown server fields.
+- `store.reportError` wraps every error in a `FirestateError` before calling
+  `onError`. The wrapper carries `type`, `path`, `operation`, and (when present)
+  the Firestore `code` on own fields, puts the path in the message, and keeps
+  the original error on `cause`. This lets a consumer that forwards only the
+  first argument to a tracker keep the context and get a distinct fingerprint
+  per resource. Context still travels as the second `onError` argument.
+- A listener error with a terminal Firestore code (`permission-denied`,
+  `unauthenticated`) is never retried, even with `retryOnError: true` — a retry
+  can never clear it. The handler reports it, sets `state.error`, and clears
+  `isLoading`. Only transient codes re-attach the listener on `retryInterval`.
 - `enabled: false` on hooks must not resolve paths or create subscriptions. It
   returns stable no-op handles.
 - `queryConstraints` are keyed by *semantic query identity*, not by array

README.md

@@ -1095,6 +1095,39 @@ subscription.load()
 subscription.stop()
 ```
 
+### Error Handling
+
+Firestate reports every error through `onError`. The first argument is always a
+`FirestateError` that carries its own context, so a consumer that forwards only
+the error to a tracker keeps a usable path and a distinct fingerprint per
+resource:
+
+```typescript
+import { FirestateError } from '@hvakr/firestate'
+
+const store = createStore({
+    firestore: db,
+    onError: (error) => {
+        // error is a FirestateError with own fields:
+        //   error.path      → 'projects/123/tasks/t1'
+        //   error.type      → 'document' | 'collection' | 'undo'
+        //   error.operation → 'read' | 'write' | 'undo' | 'redo'
+        //   error.code      → Firestore code, e.g. 'permission-denied'
+        //   error.cause     → the original FirebaseError
+        Sentry.captureException(error)
+    },
+})
+```
+
+The `context` object still arrives as the second argument for consumers that
+prefer it.
+
+`retryOnError: true` re-attaches a listener after a transient error (e.g.
+`unavailable`). A non-transient code — `permission-denied` or
+`unauthenticated` — is treated as terminal even so: Firestate reports it, sets
+`state.error`, and clears the loading flag instead of retrying forever behind a
+spinner.
+
 ### Custom Undo Manager
 
 Create a standalone undo manager with navigation support:

src/__tests__/listener-error.test.ts

@@ -0,0 +1,212 @@
+/**
+ * Listener error handling: what a document / collection subscription does when
+ * `onSnapshot` delivers an error.
+ *
+ * Two contracts are pinned here:
+ *
+ *   - Reported errors carry context. `store.reportError` wraps the raw
+ *     FirebaseError in a FirestateError whose message and own fields hold the
+ *     resource path, so a consumer that forwards only the error to a tracker
+ *     still gets a usable path and a distinct fingerprint per resource.
+ *   - permission-denied is terminal. A retry can never clear it, so even with
+ *     `retryOnError: true` the subscription reports it, sets `state.error`, and
+ *     clears `isLoading` instead of re-attaching the listener forever. A
+ *     transient code (e.g. `unavailable`) still retries.
+ */
+import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'
+
+vi.mock('firebase/firestore', async () => {
+    const actual =
+        await vi.importActual<typeof import('firebase/firestore')>(
+            'firebase/firestore'
+        )
+    const { buildFirestoreMock } = await import('./test-harness')
+    return buildFirestoreMock(actual as unknown as Record<string, unknown>)
+})
+
+import { createHarness, firestoreError, type Harness } from './test-harness'
+import { createCollectionSubscription } from '../core/collection'
+import { createDocumentSubscription } from '../core/document'
+import { defineCollection, defineDocument } from '../registry/schema'
+import { createStore, type FirestateStore } from '../core/store'
+import { FirestateError } from '../core/errors'
+import type { ErrorContext } from '../types'
+
+interface Doc {
+    field1?: string
+}
+
+interface Item {
+    id?: string
+    name?: string
+}
+
+describe('listener error handling', () => {
+    let onError: ReturnType<typeof vi.fn>
+    let store: FirestateStore
+    let h: Harness
+
+    beforeEach(() => {
+        vi.useFakeTimers()
+        vi.spyOn(console, 'error').mockImplementation(() => {})
+        vi.spyOn(console, 'warn').mockImplementation(() => {})
+        onError = vi.fn()
+        store = createStore({ firestore: {} as never, autosave: 0, onError })
+        h = createHarness()
+    })
+
+    afterEach(() => {
+        vi.clearAllTimers()
+        vi.useRealTimers()
+        vi.restoreAllMocks()
+    })
+
+    const lastReported = () => {
+        const call = onError.mock.calls.at(-1) as
+            | [FirestateError, ErrorContext]
+            | undefined
+        return call?.[0]
+    }
+
+    describe('reported error carries context', () => {
+        it('wraps a document listener error with its path', () => {
+            const def = defineDocument<Doc>({
+                collection: 'projects/p1/tasks',
+                id: 't1',
+            })
+            const sub = createDocumentSubscription({
+                store,
+                definition: def,
+                docId: 't1',
+                collectionPath: 'projects/p1/tasks',
+            })
+            sub.load()
+
+            h.fireListenerError(firestoreError('permission-denied'))
+
+            const reported = lastReported()
+            expect(reported).toBeInstanceOf(FirestateError)
+            expect(reported?.type).toBe('document')
+            expect(reported?.path).toBe('projects/p1/tasks/t1')
+            expect(reported?.operation).toBe('read')
+            expect(reported?.code).toBe('permission-denied')
+            // The path is in the message, so a tracker fingerprints on it.
+            expect(reported?.message).toContain('projects/p1/tasks/t1')
+            sub.stop()
+        })
+
+        it('gives two resources distinct fingerprints', () => {
+            const mk = (path: string, id: string) => {
+                const sub = createDocumentSubscription({
+                    store,
+                    definition: defineDocument<Doc>({ collection: path, id }),
+                    docId: id,
+                    collectionPath: path,
+                })
+                sub.load()
+                return sub
+            }
+
+            const a = mk('projects/p1/tasks', 't1')
+            h.fireListenerError(firestoreError('permission-denied'))
+            const first = lastReported()?.message
+
+            const b = mk('projects/p2/notes', 'n9')
+            h.fireListenerError(firestoreError('permission-denied'))
+            const second = lastReported()?.message
+
+            expect(first).not.toBe(second)
+            a.stop()
+            b.stop()
+        })
+    })
+
+    describe('permission-denied is terminal', () => {
+        it('reports and stops loading on a document even with retryOnError', () => {
+            const def = defineDocument<Doc>({
+                collection: 'docs',
+                id: 'd1',
+                retryOnError: true,
+            })
+            const sub = createDocumentSubscription({
+                store,
+                definition: def,
+                docId: 'd1',
+                collectionPath: 'docs',
+            })
+            sub.load()
+            expect(sub.getState().isLoading).toBe(true)
+
+            h.fireListenerError(firestoreError('permission-denied'))
+
+            expect(onError).toHaveBeenCalledTimes(1)
+            expect(sub.getState().error).toBeInstanceOf(Error)
+            expect(sub.getState().isLoading).toBe(false)
+
+            // No retry was scheduled: advancing past the interval attaches no
+            // new listener and reports nothing more.
+            const before = h.listeners().length
+            vi.advanceTimersByTime(10000)
+            expect(h.listeners().length).toBe(before)
+            expect(onError).toHaveBeenCalledTimes(1)
+            sub.stop()
+        })
+
+        it('reports and stops loading on a collection even with retryOnError', () => {
+            const def = defineCollection<Item>({
+                path: 'items',
+                retryOnError: true,
+            })
+            const sub = createCollectionSubscription({
+                store,
+                definition: def,
+                collectionPath: 'items',
+            })
+            sub.load()
+            expect(sub.getState().isLoading).toBe(true)
+
+            h.fireListenerError(firestoreError('permission-denied'))
+
+            expect(onError).toHaveBeenCalledTimes(1)
+            expect(lastReported()?.path).toBe('items')
+            expect(sub.getState().error).toBeInstanceOf(Error)
+            expect(sub.getState().isLoading).toBe(false)
+
+            const before = h.listeners().length
+            vi.advanceTimersByTime(10000)
+            expect(h.listeners().length).toBe(before)
+            sub.stop()
+        })
+    })
+
+    describe('transient error still retries', () => {
+        it('re-attaches the listener and does not report on a document', () => {
+            const def = defineDocument<Doc>({
+                collection: 'docs',
+                id: 'd1',
+                retryOnError: true,
+                retryInterval: 5000,
+            })
+            const sub = createDocumentSubscription({
+                store,
+                definition: def,
+                docId: 'd1',
+                collectionPath: 'docs',
+            })
+            sub.load()
+            const before = h.listeners().length
+
+            h.fireListenerError(firestoreError('unavailable'))
+
+            // Transient: no report, no terminal state, retry scheduled.
+            expect(onError).not.toHaveBeenCalled()
+            expect(sub.getState().error).toBeUndefined()
+            expect(sub.getState().isLoading).toBe(true)
+
+            vi.advanceTimersByTime(5000)
+            // A fresh listener was attached by the retry.
+            expect(h.listeners().length).toBe(before + 1)
+            sub.stop()
+        })
+    })
+})

src/core/collection.ts

@@ -22,6 +22,7 @@ import type {
     UpdateOptions,
 } from '../types'
 import type { FirestateStore } from './store'
+import { isTerminalListenerError } from './errors'
 import {
     applyDiff,
     applyDiffMutable,
@@ -633,25 +634,30 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     }
 
     const handleError = (error: Error) => {
-        if (retryOnError) {
+        // A terminal code (permission-denied, unauthenticated) never clears on
+        // retry. Re-attaching the listener every retryInterval would spin
+        // forever behind a loading spinner and never report, so treat it as
+        // terminal even when retryOnError is set and fall through to reporting.
+        if (retryOnError && !isTerminalListenerError(error)) {
             console.warn('Collection listener error, retrying:', error)
             retryTimeout = setTimeout(() => {
                 stop()
                 startListener()
             }, retryInterval)
-        } else {
-            state.error = error
-            // Don't leave consumers stuck on a loading spinner — the listener
-            // has reported a terminal error, so loading is done.
-            state.isLoading = false
-            loaded = true
-            store.reportError(error, {
-                type: 'collection',
-                path: collectionPath,
-                operation: 'read',
-            })
-            notify()
+            return
         }
+
+        state.error = error
+        // Don't leave consumers stuck on a loading spinner — the listener
+        // has reported a terminal error, so loading is done.
+        state.isLoading = false
+        loaded = true
+        store.reportError(error, {
+            type: 'collection',
+            path: collectionPath,
+            operation: 'read',
+        })
+        notify()
     }
 
     const startListener = () => {

src/core/document.ts

@@ -20,6 +20,7 @@ import type {
     UpdateOptions,
 } from '../types'
 import type { FirestateStore } from './store'
+import { isTerminalListenerError } from './errors'
 import {
     applyDiff,
     applyDiffMutable,
@@ -648,25 +649,30 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     }
 
     const handleError = (error: Error) => {
-        if (retryOnError) {
+        // A terminal code (permission-denied, unauthenticated) never clears on
+        // retry. Re-attaching the listener every retryInterval would spin
+        // forever behind a loading spinner and never report, so treat it as
+        // terminal even when retryOnError is set and fall through to reporting.
+        if (retryOnError && !isTerminalListenerError(error)) {
             console.warn('Document listener error, retrying:', error)
             retryTimeout = setTimeout(() => {
                 stop()
                 load()
             }, retryInterval)
-        } else {
-            state.error = error
-            // Don't leave consumers stuck on a loading spinner — the listener
-            // has reported a terminal error, so loading is done.
-            state.isLoading = false
-            loaded = true
-            store.reportError(error, {
-                type: 'document',
-                path: `${collectionPath}/${documentId}`,
-                operation: 'read',
-            })
-            notify()
+            return
         }
+
+        state.error = error
+        // Don't leave consumers stuck on a loading spinner — the listener
+        // has reported a terminal error, so loading is done.
+        state.isLoading = false
+        loaded = true
+        store.reportError(error, {
+            type: 'document',
+            path: `${collectionPath}/${documentId}`,
+            operation: 'read',
+        })
+        notify()
     }
 
     const load = () => {