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

[tyler-reitz]

Problem

useObservable unconditionally re-throws errors from the underlying observable, making status: 'error' unreachable in practice. Users in non-suspense mode who want to handle errors locally have no way to do so:

const { status, error } = useStorageDownloadURL(ref);
if (status === 'error') return <NotFound />; // never reached -- error was already thrown

The original code included a TODO from the author questioning whether this behavior was correct.

Fix

Error handling now matches the mode the consumer opted into:

• Suspense mode (suspense: true): errors still throw, so React Error Boundaries catch them as expected. This preserves the idiomatic Suspense contract.
• Non-suspense mode (default or suspense: false): errors are returned via status: 'error' so consumers can handle them locally.

// non-suspense: handle locally
const { status, error } = useStorageDownloadURL(ref);
if (status === 'error') return <NotFound />;

// suspense: opt in to Error Boundary behavior
const { data } = useStorageDownloadURL(ref); // throws on error, caught by Error Boundary

Breaking change

This affects the default usage pattern. Non-suspense mode is the default — if you don't pass suspense: true or wrap your app in a SuspenseEnabledContext, you are in non-suspense mode.

Before 4.3.0, errors from any reactfire hook would throw unconditionally, regardless of mode. After 4.3.0:

• If you use non-suspense mode (the default) and rely on a React Error Boundary to catch Firebase errors, you must add an explicit check in your component:

  const { status, error } = useStorageDownloadURL(ref);
  if (status === 'error') throw error; // re-throw to reach Error Boundary

• If you use non-suspense mode and already check status, no change needed.
• Suspense mode behavior is unchanged.

Open decisions (need Jeff input)

Semver: This PR is bumped as 4.3.0 (minor). Repo precedent for breaking changes was a major bump (v3 to v4 for the Firebase v9 migration). There is a reasonable argument this is a spec-violation fix (status: 'error' was always documented and typed, just unreachable in practice) rather than a new behavior, which would support staying at a minor. Either way, that call should be made deliberately. Tagging Jeff to decide before merge.

Error recovery: Once a mounted cache entry errors, there is no retry path. The 30s reset timer is cancelled on first subscription, and errored observables do not re-subscribe. This is pre-existing behavior, not introduced by this PR. Surfacing it here because this PR directs users to build status === 'error' UI around a state with no retry path. Punting a proper retry mechanism to a follow-up issue; noting it here so the decision is explicit.

Fixes #535, fixes #540

[gemini-code-assist]

Code Review

This pull request bumps the package version to 4.3.0 and modifies useObservable to surface errors via the status field instead of throwing them. Feedback highlights that completely removing the error throw breaks the React Suspense contract, where errors should be caught by an ErrorBoundary. It is recommended to conditionally throw errors when suspense is enabled and to update the tests accordingly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[armando-navarro]

Verified this end-to-end locally — the fix itself is right.

Requesting changes on three things, all in service of this being the breaking change the description says it is:

Documentation

• This changes default error behavior for every hook, and right now the migration story exists only in this PR description (which becomes a squash-commit message).
• At minimum: a JSDoc update on useObservable/ObservableStatus describing the mode split (which flows into docs/reference — will need a docs regen)
• An error-handling example in docs/use.md (its current storage example checks only loading, so after this merges it silently renders <img src={undefined}> on error — the docs would be teaching the silent-failure pattern)
• An entry in docs/upgrade-guide.md.

Type contract

• ObservableStatusError declares isComplete: true, but runtime is false in every error case I tested — the @ts-expect-error in SuspenseSubject._updateImmutableStatus acknowledges it.
• This was latent while errors always threw; now that status === 'error' is the recommended branch, TypeScript narrows consumers into trusting a value that's wrong at runtime. Either make it isComplete: boolean or set the runtime flag on error.

Tests for what this PR actually promises

• The single added test uses a synthetic throwError with explicit suspense: false.
• Missing:
  • the marquee #535 path (useStorageDownloadURL, nonexistent object, default mode, emulator);
  • bare default (useObservable(id, obs$) with no suspense option — the headline case); the FirebaseAppProvider suspense={true} context path still throwing (distinct code branch, no error test);
  • and the late-error contract (emit-then-error leaves status: 'error' with stale data still readable — real behavior, currently undocumented and untested;
    • if it's intended, a test locks it in; if not, that's a bug to fix). I ran all of these locally and they behave as described — happy to share the exact tests.

Two decisions you might want stated explicitly in the PR (not code changes):

• Semver. The description says breaking; the bump says 4.3.0 (minor). Repo precedent for breaking was a major (3→4, the Firebase v9 change).
  • There's a fair argument this is a spec-violation fix (status: 'error' was always documented and typed, just unreachable) — but that call should be made on purpose, not by the version number in the diff.
  • Note the committed bump only affects exp canary numbering either way; tags drive real releases.
• Error recovery. Once a mounted cache entry errors, nothing un-errors it (the cache reset timer dies on first subscription).
  • That's pre-existing — but this PR directs users to build status === 'error' UI around a state with no retry path.
  • Fine to punt to a follow-up issue; it should be a stated decision.

Non-blocking notes

• StorageImage now swallows errors silently (placeholder forever, error never read) where it used to crash — worth either reading error (log or render slot) or an explicit "known limitation" note;
• the new test sits in the Suspense Mode describe block but tests non-suspense (move it, and don't assert isComplete: true on error — it's false);
• #540's thread proposed an explicit opt-in flag (throwErrorInObservable) rather than coupling to suspense mode — your coupling is defensible and simpler, just worth acknowledging the divergence in the thread when this closes it.

[armando-navarro]

See "Semver" in my review — the description says breaking, this says minor. Worth an explicit 4.3.0-vs-5.0.0 decision (this number only drives exp canary versions either way).

[armando-navarro]

Thanks for the thorough response - nearly everything landed. Take this review with a grain of salt, as there are significant gaps in my knowledge here that could lead to incorrect conclusions.

I'm approving this PR with a couple additional suggestions:

The upgrade guide's retry sentence is wrong

• "If you need retry behavior, unmount and remount the component" is incorrect — remounting doesn't retry.
• The errored subject stays in the globalThis cache under its observableId, and a remount rejoins that same cached subject.
• Verified directly: remounting the same id with a healthy replacement source still returns the original error.
• It also contradicts your own PR body's "errored observables do not re-subscribe."
• Suggested replacement: today the only workaround is a new observableId; a retry mechanism is the follow-up issue you're already planning.

The marquee scenario still has no test

• The #535 path — useStorageDownloadURL on a nonexistent object, default mode, storage emulator → status: 'error' with the storage/object-not-found FirebaseError — is the PR's reason for existing.
• It's verified working, but nothing in the suite pins it.
• One emulator test would lock the headline behavior in.
• Nice-to-have, not blocking: the FirebaseAppProvider suspense={true} context-path error test from my earlier list would also be welcome.

Two minor notes (no action required)

• isComplete on the error branch could be the precise false rather than boolean — error-after-complete can't happen, since completion never reaches the tap once the source errors.
  • Alternatively, just delete the override, since it now matches the base type.
• The subscriber error: callback in useObservable is unreachable (catchError upstream converts errors to completion).
  • That branch could carry a comment or be dropped.
  • The real behavior change in this PR is the render-path condition, which is correct.

[armando-navarro]

The #535 marquee scenario (useStorageDownloadURL on a nonexistent object, default mode, storage emulator) still has no test pinning it — this is the PR's reason for existing. Nice to have before merge, not blocking.

[tyler-reitz]

Added. Had to pair it with a defer(() => getDownloadURL(ref)) fix in storage.tsx since getDownloadURL was firing an eager network request on every render, which leaked an unhandled rejection when no subscriber caught the result. The fix also eliminates redundant requests on re-renders as a bonus.

[armando-navarro]

Minor, not blocking: could be the precise false here rather than boolean — error-after-complete can't happen. Or just delete the override since it now matches the base type.

[tyler-reitz]

Most of this has landed: upgrade guide updated with the correct retry explanation, isComplete override removed, the marquee test added (had to fix an eager-request bug in useStorageDownloadURL first, wrapped with defer, which also eliminates redundant network requests on re-renders as a bonus), and the FirebaseAppProvider suspense={true} context-path error test added. Semver call is @jhuleatt's.