Skip to content

Keep FileReader in DONE state after abort - #57692

Closed
fallintoplace wants to merge 1 commit into
react:mainfrom
fallintoplace:fix-filereader-abort-state
Closed

Keep FileReader in DONE state after abort#57692
fallintoplace wants to merge 1 commit into
react:mainfrom
fallintoplace:fix-filereader-abort-state

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary:

Before #57375, reads never entered LOADING, so the active abort path was effectively unreachable. Now that reads use LOADING, that path calls _reset() before and after transitioning to DONE, leaving the reader in EMPTY after abort() returns.

This differs from the File API abort algorithm, which keeps an active reader in DONE. The final reset also overwrites a replacement read started by an abort handler, and that new read clears the shared _aborted flag before the canceled native promise settles.

This change:

  • keeps an aborted active reader in DONE with a null result
  • skips the old loadend if the abort handler starts another read
  • gives each read an ID so a canceled native promise cannot complete over a newer read

Changelog:

[GENERAL] [FIXED] - Keep FileReader in the correct state after aborting a read.

Test Plan:

  • yarn jest packages/react-native/Libraries/Blob/__tests__/FileReader-test.js --runInBand
  • ./node_modules/.bin/prettier --check packages/react-native/Libraries/Blob/FileReader.js packages/react-native/Libraries/Blob/__tests__/FileReader-test.js
  • ./node_modules/.bin/eslint --max-warnings 0 packages/react-native/Libraries/Blob/FileReader.js packages/react-native/Libraries/Blob/__tests__/FileReader-test.js
  • yarn flow-check

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 25, 2026
@fallintoplace
fallintoplace force-pushed the fix-filereader-abort-state branch from beb7b66 to 71067af Compare July 25, 2026 20:26
@fallintoplace fallintoplace changed the title Fix FileReader abort state handling Keep FileReader in DONE state after abort Jul 25, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 25, 2026

@Abbondanzo Abbondanzo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One small nit while we're here to honor the spec. If possible, could you also add some tests to ensure that the result and error are never stale? I'll reimport with those changes

Comment thread packages/react-native/Libraries/Blob/FileReader.js Outdated
@meta-codesync

meta-codesync Bot commented Jul 27, 2026

Copy link
Copy Markdown

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D113802292.

@fallintoplace
fallintoplace force-pushed the fix-filereader-abort-state branch from 71067af to 23dca56 Compare July 27, 2026 18:55
@meta-codesync meta-codesync Bot closed this in e92816c Jul 28, 2026
@meta-codesync meta-codesync Bot added the Merged This PR has been merged. label Jul 28, 2026
@meta-codesync

meta-codesync Bot commented Jul 28, 2026

Copy link
Copy Markdown

@Abbondanzo merged this pull request in e92816c.

meta-codesync Bot pushed a commit that referenced this pull request Jul 29, 2026
Summary:
Pull Request resolved: #57745

Builds on [#57692](#57692), which fixed the `abort()` state machine and reset `result`/`error` at the start of each read via the `_startRead()` helper. A few gaps remain versus the [File API spec](https://w3c.github.io/FileAPI/): a read that starts while the reader is `LOADING` is silently superseded instead of throwing, `loadstart` is never dispatched, and `error` is a plain `Error` rather than a `DOMException`.

This change:

- throws an `InvalidStateError` `DOMException` when a read starts while the reader is `LOADING`, by extending the `_startRead()` helper
- fires `loadstart` when a read begins
- exposes `error` as a `DOMException` (`NotReadableError`), matching the spec typing

No `progress` event is synthesized: `NativeFileReaderModule` resolves the whole payload in a single promise, so there is no incremental read to observe. The spec fires `progress` opportunistically as bytes stream in (it is not a required event), and emitting one post-completion `progress` with `loaded === total` would report data the reader never actually measured — a handler would also see `reader.result === null` at that point. The lifecycle is covered by `loadstart`/`load`/`error`/`abort`/`loadend`. The legacy `readystatechange` event is left untouched, and the deprecated `readAsBinaryString()` is out of scope.

## Changelog:

[GENERAL] [FIXED] - Fire `loadstart`, throw `InvalidStateError` on overlapping reads, and expose `FileReader.error` as a `DOMException`.

Reviewed By: christophpurrer

Differential Revision: D113819663

fbshipit-source-id: c7c37b3b578d825cdccf73840b2c0742b6a4eaf9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants