close
Skip to content

[Website] Support streamed PHP responses in the service worker#3361

Merged
adamziel merged 7 commits intotrunkfrom
stream-php-responses-through-service-worker
Mar 10, 2026
Merged

[Website] Support streamed PHP responses in the service worker#3361
adamziel merged 7 commits intotrunkfrom
stream-php-responses-through-service-worker

Conversation

@adamziel
Copy link
Collaborator

@adamziel adamziel commented Mar 9, 2026

Summary

The service worker used to buffer the entire PHP response in memory before sending it to the browser. When a long-running PHP task (such as a site import) exceeded the hard 25-second timeout on waiting for postMessage() response, the service worker assumed the response is not coming and communicated a network error.

This PR streams the response body instead of buffering it. The main thread calls requestStreamed(), awaits just the response headers (fast, well within the 25s timeout), then bridges the body stream to the service worker via a MessagePort. The service worker constructs a streaming Response and returns it immediately — the body flows through as PHP produces it, with no timeout on the body.

The change also fixes a latent bug in StreamedPHPResponse.getHeadersStream(): the cookie handler in PHPRequestHandler consumes the headers stream before Comlink gets to serialize it, causing a "ReadableStream is locked" error. The fix synthesizes a fresh stream from already-parsed header data when the original has been consumed.

A backwards-compatible fallback checks for the legacy buffered bytes property, so older main-thread code continues to work.

Test plan

  • npx nx build php-wasm-web-service-worker and npx nx build playground-remote pass
  • npx nx test php-wasm-web-service-worker and npx nx test php-wasm-universal pass
  • npm run dev → open http://127.0.0.1:5400/website-server/ → WordPress front page loads
  • Navigate to wp-admin → Dashboard loads with no console errors
  • Load the streaming site importer blueprint → import should no longer fail after 25 seconds

The service worker's convertFetchEventToPHPRequest() used to buffer the
entire PHP response in memory before sending it to the browser. A hard
25-second timeout (DEFAULT_RESPONSE_TIMEOUT) killed any PHP request that
took longer than that, breaking long-running scripts like the streaming
site importer.

With this change, the main thread calls requestStreamed() instead of
request(), awaits just the response headers (fast, well within the 25s
timeout), then bridges the body stream to the service worker via a
MessagePort using the existing streamToPort/portToStream utilities.
The service worker constructs a streaming Response and returns it
immediately. The 25-second timeout now only applies to receiving
headers, not the entire body.

To make this work, getHeadersStream() on StreamedPHPResponse was fixed
to synthesize a fresh stream from already-parsed header data when the
original stream has been consumed (e.g. by the cookie handler in
PHPRequestHandler). Without this, Comlink's transfer handler hit
"ReadableStream is locked" when serializing the response across the
worker boundary.

A buffered fallback (checking for phpResponse.bytes) preserves backwards
compatibility with older main-thread code.
Copilot AI review requested due to automatic review settings March 9, 2026 22:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Streams PHP response bodies through the service worker to avoid buffering entire responses in memory and to prevent the existing 25s timeout from killing long-running PHP requests.

Changes:

  • Added a streamed-response path for request that forwards headers immediately and bridges the body via MessagePort.
  • Updated the service worker to prefer streamed bodies (via bodyPort) with a legacy fallback to buffered bytes.
  • Fixed StreamedPHPResponse.getHeadersStream() to synthesize a new headers stream from parsed headers when the original stream has been consumed.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
packages/playground/remote/src/lib/boot-playground-remote.ts Uses requestStreamed() and bridges the body stream to the service worker via MessagePort.
packages/php-wasm/web-service-worker/src/utils.ts Constructs Response using a streamed body when provided, falling back to legacy buffered bytes.
packages/php-wasm/universal/src/lib/php-response.ts Synthesizes a serializable headers stream from already-parsed headers to avoid “stream locked” errors.
packages/php-wasm/universal/src/lib/api.ts Exposes stream/port bridging utilities (streamToPort, portToStream) and feature detection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@adamziel adamziel changed the title Stream PHP responses through the service worker [Website] Support streamed PHP responses in the service worker Mar 9, 2026
@adamziel adamziel marked this pull request as draft March 9, 2026 22:25
Make supportsTransferableStreams() private again since nothing
outside api.ts needs it. Fix the detection to actually include
the stream in the transfer list (the old code always returned
false because ReadableStream can't be cloned, only transferred).
Cache the result so we only probe once.

Remove the unused stdout case from the service worker — we always
use the MessagePort bridge because ServiceWorker.postMessage()
doesn't support transferable streams.
…serializer

The cookie handler in PHPRequestHandler consumes the headers stream
before Comlink serializes the response. Previously, getHeadersStream()
detected this and synthesized a fresh stream — but that put transport
concerns inside the response class.

Now getHeadersStream() just returns the raw stream. A new
headersStreamConsumed getter tells the Comlink transfer handler
whether it needs to build a fresh stream from the parsed headers
and status code. The re-synthesis logic moves to a headersToStream()
helper in the serialization layer where it belongs.
The cookie handler in PHPRequestHandler reads the headers before
Comlink serializes the response. Instead of detecting this and
re-building the stream from parsed data (either in the response
class or the serializer), just tee the headers stream upfront:
one branch for internal parsing, one for transport. No conditional
logic, no headersStreamConsumed flag, no headersToStream helper.
Two changes:

Tee the headers stream in the StreamedPHPResponse constructor so
the cookie handler and Comlink serializer each get their own branch.
This removes the headersToStream/headersStreamConsumed workarounds.

Document why we use a MessagePort bridge instead of direct
ReadableStream transfer for the service worker channel. Tested in
Chrome: ServiceWorker.postMessage() silently drops the entire message
when the transfer list contains a ReadableStream. The call succeeds
and the stream detaches from the sender, but the message never
arrives. Service workers live in a different agent cluster than their
clients, which limits what can be transferred.
The directory trailing-slash redirect in PHPRequestHandler used
'Location' (capital L), but StreamedPHPResponse normalizes all
header names to lowercase when parsing the headers stream. Use
lowercase 'location' to match the normalized form, and update the
test assertions accordingly.
@adamziel adamziel marked this pull request as ready for review March 10, 2026 00:13
@adamziel adamziel merged commit 0714477 into trunk Mar 10, 2026
42 checks passed
@adamziel adamziel deleted the stream-php-responses-through-service-worker branch March 10, 2026 08:03
adamziel added a commit that referenced this pull request Mar 10, 2026
…tion and re-scoping redirect URLs (#3365)

## Motivation for the change, related issues

#3361 introduced streamed PHP responses in the service worker. This
changed the cookie store population from synchronous (awaited via
`PHPResponse.fromStreamedResponse()`) to a floating `.then()` that could
resolve after the next request arrived. This caused
`playground_auto_login` to redirect infinitely on Safari because the
`playground_auto_login_already_happened` cookie was never stored in
time. `Response.redirect()` drops `Set-Cookie` headers, and the internal
cookie store was the only mechanism preserving them across requests.

A secondary issue was that `Response.redirect(new URL('/wp-admin/',
scopedUrl))` resolves absolute paths against the origin, stripping the
`/scope:…/` prefix.

cc @bgrgicak 

## Implementation details

- `php-request-handler.ts`: Replace the floating `.then()` with await
response.headers before returning from `requestStreamed()`, ensuring the
cookie store is populated before the next request can arrive. Headers
are available as soon as PHP flushes them, well before the body finishes
streaming, so this does not hurt throughput.
- `utils.ts`: Re-scope the Location URL before calling
`Response.redirect()`. If the resolved URL lacks the `/scope:…/` prefix,
add it back using `setURLScope`.

## Testing Instructions (or ideally a Blueprint)

run `npm run dev` and go to `http://127.0.0.1:5400/website-server/` on
safari

It will crash on `trunk` and run correctly from this branch.

---------

Co-authored-by: Adam Zieliński <adam@adamziel.com>
adamziel added a commit that referenced this pull request Mar 13, 2026
PR #3361 added streaming response support to the service worker to avoid
postMessage timeouts. However, streaming partially-rendered HTML to the
browser can cause broken markup flashes while PHP is still producing
output. This buffers the full response body when the content-type is
text/html, while leaving all other responses (JSON, images, downloads)
streaming as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants