Skip to content

fix(ext/web): fire FileReader progress events as tasks#34763

Merged
crowlKats merged 1 commit into
mainfrom
fix/filereader-event-task-ordering
Jun 8, 2026
Merged

fix(ext/web): fire FileReader progress events as tasks#34763
crowlKats merged 1 commit into
mainfrom
fix/filereader-event-task-ordering

Conversation

@crowlbot

@crowlbot crowlbot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

What

FileReader's read operation fired its progress events — loadstart,
progress, load/error, loadend — using queueMicrotask. The
File API read operation
specifies that each of these is dispatched by "queue a task".

This switches those dispatches to a task (setTimeout) and queues
loadend as its own task so the preceding load/error event can be
observed before loadend.

Why

The task-vs-microtask distinction is observable. A single read fires
several events in sequence, and consumers rely on the microtask queue
draining between them — so a listener (or an await) can react to one
event before the next is dispatched. The WPT EventWatcher is exactly
such a consumer: it re-arms for the next event in a microtask after the
previous one resolves. With queueMicrotask, the events collapsed into a
single turn and arrived back-to-back, so EventWatcher reported
"Not expecting event, but got <event>".

The existing code already carried TODOs noting that queueMicrotask was
wrong and should be "queue a task".

Spec

File API §read operation,
steps that queue tasks to fire loadstart, progress, load, error,
and loadend.

WPT subtests now passing

FileAPI/reading-data-section/filereader_events.any.html and
.any.worker.html:

  • events are dispatched in the correct order for an empty blob
  • events are dispatched in the correct order for a non-empty blob

tests/wpt/runner/expectations/FileAPI.json is flipped from false to
true for both files.

Root cause

FileReader#readOperation in ext/web/10_filereader.js dispatched all
progress events via queueMicrotask instead of as tasks, and fired
load+loadend (and error+loadend) within a single task.

The read operation in FileReader fired its progress events (loadstart,
progress, load/error, loadend) via queueMicrotask. The File API spec
requires these to be dispatched by "queue a task". The distinction is
observable: a single read fires several events in sequence, and consumers
(notably the WPT EventWatcher) rely on the microtask queue draining
between them so a listener can react to one event before the next is
dispatched. Using microtasks collapsed the events into a single turn, so
they arrived back-to-back.

Switch the dispatches to a task (setTimeout) and queue loadend as its own
task so the preceding load/error event can be observed before loadend.

Fixes WPT FileAPI/reading-data-section/filereader_events.any.{html,worker.html}
("events are dispatched in the correct order for an empty blob" and
"... for a non-empty blob").
@crowlbot

crowlbot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

@crowlKats ready for review

The WPT job is green — FileAPI/reading-data-section/filereader_events.any.{html,worker.html} now pass (both "empty blob" and "non-empty blob" subtests). The few red checks in the first run were unrelated CI flakes (Chromium/Playwright download failures in test specs on macos/windows-aarch64, and the flaky node_compat sequential/test-cluster-inspect-brk.js on windows-aarch64); I've re-run the failed jobs.

@crowlKats

Copy link
Copy Markdown
Member

@crowlbot shouldnt we have a proper implementation of a task rather than settimeout? furthern investigate this spec language.

@crowlbot

crowlbot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review — I looked into both points.

Spec language. The read operation says "queue a task" for each of loadstart / progress / load / loadend, so these are macrotasks — that's what the old queueMicrotask got wrong. The events are consumed one at a time, with the consumer re-arming for the next event in a microtask in between (the WPT EventWatcher does exactly this), so consecutive events have to land in separate event-loop turns for that re-arm to run before the next event fires.

Why setTimeout and not queueMicrotask/defer. The microtask-based deferrals don't give that separation. Two callbacks deferred in the same turn run back-to-back as microtasks — this is also true for op_defer/defer, since its .then reaction is a microtask — so a microtask scheduled by the first runs after the second:

setTimeout:     t1 → (microtask checkpoint) → t2     // ✓
queueMicrotask: t1 → t2   (no checkpoint between)    // ✗

Among the primitives we expose, setTimeout(fn, 0) is the one that queues each event as a real separate task. If you'd prefer a dedicated lightweight "queue a task" op in deno_core I'm happy to look into adding one, but I wanted to flag that the existing defer won't work here.

load + loadend. Good catch that the spec fires both within a single task. In the spec/browser model that task isn't author script, so a microtask checkpoint happens between the load and loadend listener invocations (the JS stack empties after each), which is what lets EventWatcher re-arm between them. We fire from a JS callback, so two synchronous dispatchEvents in one callback have no checkpoint between them. Rather than splitting loadend into its own task, I can keep them in one task (matching the spec wording) by awaiting a microtask between the two dispatches — I verified that also passes the WPT test. Happy to switch to whichever you prefer.

@crowlKats
crowlKats merged commit d5a81a5 into main Jun 8, 2026
267 of 271 checks passed
@crowlKats
crowlKats deleted the fix/filereader-event-task-ordering branch June 8, 2026 07:43
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.

2 participants