fix: surface observable errors via status instead of re-throwing

.gitignore

@@ -1,3 +1,4 @@
+CLAUDE.local*
 .DS_Store
 npm-debug.log
 

docs/upgrade-guide.md

@@ -1,3 +1,27 @@
+# Upgrade from ReactFire v4.2 to v4.3
+
+ReactFire v4.3.0 changes how errors are surfaced in non-suspense mode (the default).
+
+## Error handling behavior change
+
+Previously, errors from any reactfire hook were thrown unconditionally, making `status: 'error'` unreachable in practice. In v4.3.0, error handling depends on the mode:
+
+- **Non-suspense mode** (default, or `suspense: false`): errors are returned via `status: 'error'` so components can handle them locally.
+- **Suspense mode** (`suspense: true`): errors are re-thrown so a React Error Boundary can catch them. No change from prior behavior.
+
+**If you rely on a React Error Boundary to catch Firebase errors in non-suspense mode**, you must add an explicit re-throw in your component:
+
+```tsx
+const { status, error } = useStorageDownloadURL(ref);
+if (status === 'error') throw error; // re-throw to reach your Error Boundary
+```
+
+**If you already check `status` before using `data`**, no change is needed.
+
+Note: once an observable errors there is no automatic retry. The errored observable remains in the global cache under its `observableId`, so unmounting and remounting the same component rejoins the same errored state. Today the only workaround is to change the `observableId`. A proper retry mechanism is tracked in [#742](https://github.com/FirebaseExtended/reactfire/issues/742).
+
+---
+
 # Upgrade from ReactFire v3 to v4
 
 As announced in [Discussion 402](https://github.com/FirebaseExtended/reactfire/discussions/402), ReactFire v4 contains breaking changes. This guide details how to upgrade from v3 to v4.

docs/use.md

@@ -426,12 +426,16 @@ function CatImage() {
   const storage = useStorage();
   const catRef = ref(storage, 'cats/newspaper');
 
-  const { status, data: imageURL } = useStorageDownloadURL(catRef);
+  const { status, data: imageURL, error } = useStorageDownloadURL(catRef);
 
   if (status === 'loading') {
     return <span>loading...</span>;
   }
 
+  if (status === 'error') {
+    return <span>Error: {error.message}</span>;
+  }
+
   return <img src={imageURL} alt="cat reading the newspaper" />;
 }
 ```

package.json

@@ -1,5 +1,5 @@
 {
-  "version": "4.2.3",
+  "version": "4.3.0",
   "license": "MIT",
   "type": "module",
   "main": "dist/index.umd.cjs",

src/storage.tsx

@@ -1,5 +1,6 @@
 import * as React from 'react';
 import { getDownloadURL, fromTask } from 'rxfire/storage';
+import { defer } from 'rxjs';
 import { ReactFireOptions, useObservable, ObservableStatus, useStorage } from './';
 import { useSuspenseEnabledFromConfigAndContext } from './firebaseApp';
 import { ref } from 'firebase/storage';
@@ -28,7 +29,7 @@ export function useStorageTask<T = unknown>(task: UploadTask, ref: StorageRefere
  */
 export function useStorageDownloadURL<T = string>(ref: StorageReference, options?: ReactFireOptions<T>): ObservableStatus<string | T> {
   const observableId = `storage:downloadUrl:${ref.toString()}`;
-  const observable$ = getDownloadURL(ref);
+  const observable$ = defer(() => getDownloadURL(ref));
 
   return useObservable(observableId, observable$, options);
 }

src/useObservable.ts

@@ -71,7 +71,6 @@ export interface ObservableStatusSuccess<T> extends ObservableStatusBase<T> {
 
 export interface ObservableStatusError<T> extends ObservableStatusBase<T> {
   status: 'error';
-  isComplete: true;
   error: Error;
 }
 
@@ -83,6 +82,17 @@ export interface ObservableStatusLoading<T> extends ObservableStatusBase<T> {
 
 export type ObservableStatus<T> = ObservableStatusLoading<T> | ObservableStatusError<T> | ObservableStatusSuccess<T>;
 
+/**
+ * Subscribe to an Observable and return its current status.
+ *
+ * Error handling depends on the suspense mode:
+ * - Non-suspense mode (default): errors are returned as `{ status: 'error', error }` so the
+ *   component can handle them locally without needing a React Error Boundary.
+ * - Suspense mode (`suspense: true`): errors are re-thrown so a React Error Boundary can catch them.
+ *
+ * If the observable emits a value and then errors, `data` retains the last emitted value and
+ * `status` changes to `'error'`. There is no automatic retry path once an error occurs.
+ */
 export function useObservable<T = unknown>(observableId: string, source: Observable<T>, config: ReactFireOptions = {}): ObservableStatus<T> {
   if (!observableId) {
     throw new Error('cannot call useObservable without an observableId');
@@ -105,9 +115,8 @@ export function useObservable<T = unknown>(observableId: string, source: Observa
         next: () => {
           onStoreChange();
         },
-        error: (e) => {
+        error: () => {
           onStoreChange();
-          throw e;
         },
         complete: () => {
           onStoreChange();
@@ -145,9 +154,9 @@ export function useObservable<T = unknown>(observableId: string, source: Observa
     } as ObservableStatus<T>;
   }
 
-  // throw an error if there is an error
-  // TODO(jhuleatt) this is the current, tested-for, behavior. But do we actually want it?
-  if (update.error) {
+  // In suspense mode, throw errors so React Error Boundaries can catch them.
+  // In non-suspense mode, surface errors via status so consumers can handle them locally.
+  if (suspenseEnabled && update.error) {
     throw update.error;
   }
 

test/storage.test.tsx

@@ -58,6 +58,13 @@ describe('Storage', () => {
   });
 
   describe('useStorageDownloadURL', () => {
+    it('surfaces storage/object-not-found as status: error for a nonexistent file', async () => {
+      const missingRef = ref(storage, `nonexistent/${randomString()}.txt`);
+      const { result } = renderHook(() => useStorageDownloadURL(missingRef), { wrapper: Provider });
+      await waitFor(() => expect(result.current.status).toEqual('error'));
+      expect((result.current.error as any)?.code).toEqual('storage/object-not-found');
+    });
+
     it('returns the same value as getDownloadURL', async () => {
       const someBytes = Uint8Array.from(Buffer.from(new ArrayBuffer(500_000)));
       const testFileRef = ref(storage, `${randomString()}/${randomString()}.txt`);

test/useObservable.test.tsx

@@ -2,7 +2,9 @@ import '@testing-library/jest-dom/extend-expect';
 import { act, cleanup, render, renderHook, waitFor } from '@testing-library/react';
 import * as React from 'react';
 import { of, Subject, BehaviorSubject, throwError } from 'rxjs';
-import { useObservable } from '../src/index';
+import { useObservable, FirebaseAppProvider } from '../src/index';
+import { initializeApp } from 'firebase/app';
+import { baseConfig } from './appConfig';
 
 describe('useObservable', () => {
   afterEach(cleanup);
@@ -124,10 +126,46 @@ describe('useObservable', () => {
 
       act(() => observable$.next('val'));
       expect(result.current.isComplete).toEqual(false);
-      
+
       act(() => observable$.complete());
       await waitFor(() => expect(result.current.isComplete).toEqual(true));
     });
+
+    it('surfaces errors via status in non-suspense mode', async () => {
+      const error = new Error('I am an error');
+      const observable$ = throwError(error);
+
+      const { result } = renderHook(() => useObservable('test-error-non-suspense', observable$, { suspense: false }));
+
+      await waitFor(() => expect(result.current.status).toEqual('error'));
+      expect(result.current.error).toEqual(error);
+    });
+
+    it('surfaces errors via status when no suspense option is provided', async () => {
+      const error = new Error('default mode error');
+      const observable$ = throwError(error);
+
+      const { result } = renderHook(() => useObservable('test-error-default-mode', observable$));
+
+      await waitFor(() => expect(result.current.status).toEqual('error'));
+      expect(result.current.error).toEqual(error);
+    });
+
+    it('retains last emitted data when observable errors after emitting', async () => {
+      const subject$ = new Subject<string>();
+      const error = new Error('late error');
+
+      const { result } = renderHook(() => useObservable('test-late-error', subject$, { suspense: false }));
+
+      act(() => subject$.next('good value'));
+      await waitFor(() => expect(result.current.status).toEqual('success'));
+      expect(result.current.data).toEqual('good value');
+
+      act(() => subject$.error(error));
+      await waitFor(() => expect(result.current.status).toEqual('error'));
+      expect(result.current.error).toEqual(error);
+      expect(result.current.data).toEqual('good value');
+    });
   });
 
   describe('Suspense Mode', () => {
@@ -328,5 +366,29 @@ describe('useObservable', () => {
       // if useObservable doesn't re-emit, the value here will still be "Jeff"
       expect(refreshedComp).toHaveTextContent('James');
     });
+    it('throws an error via FirebaseAppProvider suspense context path', () => {
+      const spy = vi.spyOn(console, 'error');
+      spy.mockImplementation(() => {});
+
+      const onError = (e: ErrorEvent) => e.preventDefault();
+      window.addEventListener('error', onError);
+
+      const app = initializeApp(baseConfig, 'suspense-context-test');
+      const error = new Error('context-path error');
+      const observable$ = throwError(error);
+
+      const wrapper = ({ children }: { children: React.ReactNode }) => (
+        <FirebaseAppProvider firebaseApp={app} suspense={true}>
+          {children}
+        </FirebaseAppProvider>
+      );
+
+      expect(() => renderHook(() => useObservable('test-context-suspense-error', observable$), { wrapper })).toThrow(
+        expect.objectContaining({ message: 'context-path error' })
+      );
+
+      spy.mockRestore();
+      window.removeEventListener('error', onError);
+    });
   });
 });