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

.gitignore

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

docs/use.md

@@ -6,6 +6,10 @@
   * [Initialize product SDKs and register them with ReactFire](#initialize-product-sdks-and-register-them-with-reactfire)
   * [Connect to the Firebase Local Emulator Suite](#connect-to-the-firebase-local-emulator-suite)
   * [Set up App Check](#set-up-app-check)
+- [Error Handling](#error-handling)
+  * [Non-suspense mode (default)](#non-suspense-mode-default)
+  * [Suspense mode](#suspense-mode)
+  * [No automatic retry](#no-automatic-retry)
 - [Auth](#auth)
   * [Display the current signed-in user](#display-the-current-signed-in-user)
   * [Only render a component if a user is signed in](#only-render-a-component-if-a-user-is-signed-in)
@@ -167,6 +171,51 @@ function FirebaseComponents({ children }) {
 
 See the [App Check setup guide in the Firebase docs](https://firebase.google.com/docs/app-check/web/recaptcha-provider#project-setup) for more detailed instructions.
 
+## Error Handling
+
+ReactFire hooks report errors from the underlying Firebase observable, and how an error surfaces depends on whether suspense mode is enabled.
+
+### Non-suspense mode (default)
+
+By default (or with `suspense: false`), an error is returned through the hook's `status` so you can handle it where the data is used:
+
+```tsx
+function CatImage() {
+  const storage = useStorage();
+  const catRef = ref(storage, 'cats/newspaper');
+
+  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" />;
+}
+```
+
+If a hook emits a value and then errors, `status` becomes `'error'` while `data` keeps its last emitted value.
+
+### Suspense mode
+
+When suspense is enabled (`<FirebaseAppProvider suspense={true}>`, or `suspense: true` on the hook), errors are thrown instead so the nearest React [Error Boundary](https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary) can catch them. In this mode the hook never returns `status: 'error'`:
+
+```tsx
+<ErrorBoundary fallback={<span>Something went wrong</span>}>
+  <React.Suspense fallback={<span>loading...</span>}>
+    <CatImage />
+  </React.Suspense>
+</ErrorBoundary>
+```
+
+### No automatic retry
+
+Once an observable errors, there is no automatic retry. The errored observable stays in ReactFire's global cache under its `observableId`, so unmounting and remounting the same component rejoins the same errored state. The only workaround today is to use a different `observableId`. A retry mechanism is tracked in [#742](https://github.com/FirebaseExtended/reactfire/issues/742).
+
 ## Auth
 
 The following samples assume that `FirebaseAppProvider` and `AuthProvider` components exist higher up the component tree (see [setup instructions](#setup) for more detail).
@@ -426,12 +475,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" />;
 }
 ```

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

@@ -126,9 +126,12 @@ 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 a React Error Boundary can catch them.
+  // In non-suspense mode (the default), surface the error via `status: 'error'` so
+  // the consumer can handle it locally. `update` already carries the error, so once
+  // an observable errors `data` retains its last emitted value. There is no automatic
+  // retry path once an error occurs (see #742).
+  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);
+    });
   });
 });