diff --git a/.gitignore b/.gitignore index df34806e..b2ebc657 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +CLAUDE.local* .DS_Store npm-debug.log diff --git a/docs/reference/functions/StorageImage.md b/docs/reference/functions/StorageImage.md index 24370e02..ea871ad4 100644 --- a/docs/reference/functions/StorageImage.md +++ b/docs/reference/functions/StorageImage.md @@ -8,7 +8,7 @@ > **StorageImage**(`props`): `Element` -Defined in: [src/storage.tsx:78](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L78) +Defined in: [src/storage.tsx:79](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L79) ## Parameters diff --git a/docs/reference/functions/useStorageDownloadURL.md b/docs/reference/functions/useStorageDownloadURL.md index 898c4931..ced03bbf 100644 --- a/docs/reference/functions/useStorageDownloadURL.md +++ b/docs/reference/functions/useStorageDownloadURL.md @@ -8,7 +8,7 @@ > **useStorageDownloadURL**\<`T`\>(`ref`, `options?`): [`ObservableStatus`](../interfaces/ObservableStatus.md)\<`string` \| `T`\> -Defined in: [src/storage.tsx:29](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L29) +Defined in: [src/storage.tsx:30](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L30) Subscribe to a storage ref's download URL diff --git a/docs/reference/functions/useStorageTask.md b/docs/reference/functions/useStorageTask.md index ccda3ddf..f6cbc13d 100644 --- a/docs/reference/functions/useStorageTask.md +++ b/docs/reference/functions/useStorageTask.md @@ -8,7 +8,7 @@ > **useStorageTask**\<`T`\>(`task`, `ref`, `options?`): [`ObservableStatus`](../interfaces/ObservableStatus.md)\<`T` \| `UploadTaskSnapshot`\> -Defined in: [src/storage.tsx:16](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L16) +Defined in: [src/storage.tsx:17](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L17) Subscribe to the progress of a storage task diff --git a/docs/reference/type-aliases/StorageImageProps.md b/docs/reference/type-aliases/StorageImageProps.md index b75fa337..f1206d9b 100644 --- a/docs/reference/type-aliases/StorageImageProps.md +++ b/docs/reference/type-aliases/StorageImageProps.md @@ -8,7 +8,7 @@ > **StorageImageProps** = `object` -Defined in: [src/storage.tsx:36](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L36) +Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L37) ## Properties @@ -16,7 +16,7 @@ Defined in: [src/storage.tsx:36](https://github.com/FirebaseExtended/reactfire/b > `optional` **placeHolder?**: `React.ReactNode` -Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L40) +Defined in: [src/storage.tsx:41](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L41) *** @@ -24,7 +24,7 @@ Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/b > `optional` **storage?**: `FirebaseStorage` -Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L38) +Defined in: [src/storage.tsx:39](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L39) *** @@ -32,7 +32,7 @@ Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/b > **storagePath**: `string` -Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L37) +Defined in: [src/storage.tsx:38](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L38) *** @@ -40,4 +40,4 @@ Defined in: [src/storage.tsx:37](https://github.com/FirebaseExtended/reactfire/b > `optional` **suspense?**: `boolean` -Defined in: [src/storage.tsx:39](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L39) +Defined in: [src/storage.tsx:40](https://github.com/FirebaseExtended/reactfire/blob/main/src/storage.tsx#L40) diff --git a/docs/use.md b/docs/use.md index 5686a368..2d840fd5 100644 --- a/docs/use.md +++ b/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 loading...; + } + + if (status === 'error') { + return Error: {error.message}; + } + + return 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 (``, 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 +Something went wrong}> + loading...}> + + + +``` + +### 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 loading...; } + if (status === 'error') { + return Error: {error.message}; + } + return cat reading the newspaper; } ``` diff --git a/src/storage.tsx b/src/storage.tsx index 5a0299da..ea4e483f 100644 --- a/src/storage.tsx +++ b/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(task: UploadTask, ref: StorageRefere */ export function useStorageDownloadURL(ref: StorageReference, options?: ReactFireOptions): ObservableStatus { const observableId = `storage:downloadUrl:${ref.toString()}`; - const observable$ = getDownloadURL(ref); + const observable$ = defer(() => getDownloadURL(ref)); return useObservable(observableId, observable$, options); } diff --git a/src/useObservable.ts b/src/useObservable.ts index 9e6173a8..aae1dac8 100644 --- a/src/useObservable.ts +++ b/src/useObservable.ts @@ -126,9 +126,12 @@ export function useObservable(observableId: string, source: Observa } as ObservableStatus; } - // 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; } diff --git a/test/storage.test.tsx b/test/storage.test.tsx index 043457b4..6c9a5139 100644 --- a/test/storage.test.tsx +++ b/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`); diff --git a/test/useObservable.test.tsx b/test/useObservable.test.tsx index f16d327a..fd7250a9 100644 --- a/test/useObservable.test.tsx +++ b/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(); + 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 }) => ( + + {children} + + ); + + expect(() => renderHook(() => useObservable('test-context-suspense-error', observable$), { wrapper })).toThrow( + expect.objectContaining({ message: 'context-path error' }) + ); + + spy.mockRestore(); + window.removeEventListener('error', onError); + }); }); });