Skip to content

Pre-download PDFs to file:// URI to avoid stale blob: URIs on iOS#260

Merged
nyomanjyotisa merged 3 commits into
mainfrom
claude/fix-stale-blob-uri-ios-pdf-1v
Jun 5, 2026
Merged

Pre-download PDFs to file:// URI to avoid stale blob: URIs on iOS#260
nyomanjyotisa merged 3 commits into
mainfrom
claude/fix-stale-blob-uri-ios-pdf-1v

Conversation

@sentry

@sentry sentry Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

What

Pre-download PDFs to a local file:// URI using expo-file-system before passing to the Pdf component, preventing react-native-pdf from creating blob: URIs internally on iOS.

Changes to app/pdf-viewer.tsx:

  • Add download state (cachedUri, downloadError, isDownloading) with a useEffect that calls File.downloadFileAsync on mount, mirroring the existing pattern in pdf-navigation-sheet.tsx
  • Show LoadingSpinner while the download is in progress
  • Pass the local file:// cachedUri to the Pdf component's source prop
  • Pass cachedUri ?? uri to PdfNavigationSheet so it reuses the already-downloaded file instead of downloading redundantly
  • Filter stale-blob errors ('Unable to resolve data for blob:') from Sentry.captureException in the onError callback
  • Handle download failure by showing the existing error/retry UI

Changes to tests/app/pdf-viewer.test.tsx:

  • Update existing tests to await waitFor for the async download to complete before asserting
  • Add test: loading spinner displays while download is pending
  • Add test: error view displays when download fails

Why

On iOS with Hermes, blob: URIs created by react-native-blob-util (used internally by react-native-pdf) become unresolvable after JS runtime resets. The native RCTBlobManager blob store is cleared on restart, but blob: URI references can persist in TanStack Query cache, navigation state, or background tab mounts via Expo Router. This causes 'Unable to resolve data for blob: UUID' crashes when the app is reopened.

By pre-downloading to a file:// path, react-native-pdf receives a local file and never invokes react-native-blob-util to create a blob: URI, eliminating the stale reference entirely.

Test results

All 28 test suites pass (197 tests), including 2 new tests for the download lifecycle:

Test Suites: 28 passed, 28 total
Tests:       197 passed, 197 total

TypeScript compiles cleanly (tsc --noEmit — no errors).


Built with Claude Sonnet 4. Prompted with the root cause analysis and proposed solution from the issue description.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Low Risk
Scoped to PDF viewing and caching; no auth or payment changes, with added tests for download and retry edge cases.

Overview
The PDF viewer now downloads remote PDFs to cache with expo-file-system before mounting react-native-pdf, shows a loading spinner until the local file:// path is ready, and feeds that path into both the main viewer and the navigation sheet (cachedUri ?? uri) so thumbnails do not re-fetch the same remote URL.

Error handling treats download failures like render failures (shared retry UI), cancels in-flight downloads on unmount/retry so late failures do not overwrite a successful retry, and suppresses Sentry for known stale blob: resolution errors while still surfacing other PDF errors.

PdfNavigationSheet skips downloadFileAsync when the URI is already file://, matching the viewer’s cached file. Tests cover the async download lifecycle, retry races, and navigation-sheet download vs local behavior.

Reviewed by Cursor Bugbot for commit d0e811d. Bugbot is set up for automated code reviews on this repo. Configure here.

Fixes GUMROAD-MOBILE-1V

On iOS with Hermes, blob: URIs created by react-native-blob-util become
unresolvable after JS runtime resets because the native blob store is
cleared but URI references persist. This caused 'Unable to resolve data
for blob: UUID' errors when reopening the app with cached PDF state.

Pre-download PDFs using expo-file-system before passing to the Pdf
component, so react-native-pdf receives a local file:// path and never
creates blob: URIs internally.

Changes:
- Add download state (cachedUri, downloadError, isDownloading) to
  PdfViewerScreen with a useEffect that downloads the PDF on mount
- Show LoadingSpinner while downloading instead of the Pdf component
- Pass the local file:// cachedUri to the Pdf component source
- Pass cachedUri to PdfNavigationSheet to reuse the downloaded file
- Filter stale-blob errors from Sentry captures in onError callback
- Update tests to handle async download lifecycle

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 25ffff1. Configure here.

Comment thread app/pdf-viewer.tsx Outdated
@greptile-apps

greptile-apps Bot commented Jun 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR pre-downloads PDFs to a local file:// path using expo-file-system before mounting the react-native-pdf component, eliminating the stale blob: URI crash on iOS Hermes runtimes where the RCTBlobManager store is cleared after JS resets.

  • app/pdf-viewer.tsx: Adds a downloadPdf callback with cancelled-flag cancellation stored in a cancelDownloadRef, a loading-spinner state, download-error UI, sharing falls back to a fresh download only when cachedUri is null, and PdfNavigationSheet receives cachedUri ?? uri to avoid redundant network fetches.
  • components/pdf-navigation-sheet.tsx: Short-circuits downloadFileAsync when the uri is already file://, preventing the re-download error state described in the previous review thread.
  • Tests: All existing tests updated to await waitFor for the async download before asserting, plus new tests covering the loading state, download failure, stale-retry race, and the share-using-cached-URI path.

Confidence Score: 5/5

Safe to merge — change is scoped to PDF loading and does not touch auth, payments, or any shared infrastructure.

All state transitions (download → loading → render, download failure → retry, stale retry cancellation) are correctly guarded by the cancelled-flag pattern. The PdfNavigationSheet re-download regression from the previous review is fixed with the isLocalFile guard. The share handler correctly prefers the already-cached URI. New tests cover every new code path including the deferred-promise race.

No files require special attention.

Important Files Changed

Filename Overview
app/pdf-viewer.tsx Core change: pre-download loop with cancellation ref, loading/error states, stale-blob Sentry filter, share using cachedUri. Logic is correct — cancellation, retry, and state transitions all handled properly.
components/pdf-navigation-sheet.tsx Adds isLocalFile guard to skip downloadFileAsync for file:// URIs and pass them directly to cachedUri — correctly addresses the previous review finding about re-triggering the download with a local source.
tests/app/pdf-viewer.test.tsx Tests updated to await the async download before asserting, with new coverage for loading spinner, download failure, stale-retry race (deferred promises), and share-using-cached-URI.
tests/components/pdf-navigation-sheet.test.tsx New test file covering remote-download-then-thumbnail and local-file-direct-use paths — provides full coverage for the isLocalFile guard.

Reviews (3): Last reviewed commit: "Use cached PDF for sharing" | Re-trigger Greptile

Comment thread app/pdf-viewer.tsx
Comment thread tests/app/pdf-viewer.test.tsx Outdated
Comment thread app/pdf-viewer.tsx
Comment thread tests/app/pdf-viewer.test.tsx
@gumclaw gumclaw self-assigned this Jun 4, 2026
@gumclaw

gumclaw commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Fixed the bot review findings in d0e811d: retry downloads now cancel prior attempts, PdfNavigationSheet uses file:// URIs directly instead of re-downloading them, and the PDF viewer tests cover loading, scoped download failures, stale retry races, and local/remote thumbnail download behavior.\n\nVerification:\n- yarn test --runInBand\n- yarn typecheck\n- yarn lint\n- yarn prettier --check app/pdf-viewer.tsx components/pdf-navigation-sheet.tsx tests/app/pdf-viewer.test.tsx tests/components/pdf-navigation-sheet.test.tsx

@gumclaw

gumclaw commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Follow-up fixed the remaining Greptile summary P1 in cf676f1: the share button now uses the already-cached file:// PDF when available instead of re-downloading the original remote URL, with regression coverage that sharing only performs the initial cache download.\n\nVerification after cf676f1:\n- yarn test --runInBand (201 tests passed)\n- yarn typecheck\n- yarn lint\n- yarn prettier --check app/pdf-viewer.tsx tests/app/pdf-viewer.test.tsx\n- GitHub checks: test + Cursor Bugbot passing

@gumclaw gumclaw assigned nyomanjyotisa and unassigned gumclaw Jun 4, 2026
@gumclaw

gumclaw commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Assigning @nyomanjyotisa per Gianfranco.

@nyomanjyotisa nyomanjyotisa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assigned-to-me.mp4

@nyomanjyotisa nyomanjyotisa merged commit 8d517b9 into main Jun 5, 2026
2 checks passed
@nyomanjyotisa nyomanjyotisa deleted the claude/fix-stale-blob-uri-ios-pdf-1v branch June 5, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants