Progress bar broken

[adamziel]

This is a high priority issue.

The progress bar foes from 0% to 50% in one go without any visible increments. The problem seems to be with the requests routed through the service worker – the fetch() in web worker isn't getting any progress updates from the readable stream, as if the service worker was buffering the response body stream before passing the Response object to the app. This also means we're not stream-loading the PHP WASM file anymore.

Here's one workaround I found on the internet:

https://github.com/anthumchris/fetch-progress-indicators/blob/master/sw-basic/sw-simple.js

I almost refuse to believe it's this complicated, though. Maybe we're missing something obvious around here:

wordpress-playground/packages/playground/remote/service-worker.ts

Lines 92 to 95 in 1f28d77

	event.respondWith(
	cachePromise.then((cache) => cache.cachedFetch(event.request))
	);
	});

cc @bgrgicak @brandonpayton

[adamziel]

Actually, I was wrong. This change in boot-playground-remote.ts logs the progress events in the first event listener, and the second one is only registered after all the progress events have been emitted:

	phpApi.onDownloadProgress(function () {
		console.log('progress', arguments);
	})
	const wpFrame = document.querySelector('#wp') as HTMLIFrameElement;
	const webApi: WebClientMixin = {
		async onDownloadProgress(fn) {
			console.log("Registering progress 2")
			phpApi.onDownloadProgress(function () {
				console.log('progress2', arguments);
			})
			return phpApi.onDownloadProgress(fn);
		},

That's likely related to the recent boot sequence updates, and perhaps to registering the service worker before the web worker.

Edit: Actually, this is weird. I'm getting seemingly non-deterministic results. Sometimes it's this, sometimes I'm getting the events.

[brandonpayton]

@adamziel in my tests so far, I am seeing progress events in emscripten-download-monitor for the sqlite-database-integration and WP zips before any updates to the progress tracker.

Here's the console log from one of my runs:
https://gist.github.com/brandonpayton/4cb2673aa3bbc57c58a6a01f696cfe63#file-console-log

[brandonpayton]

When testing with the #1668 PR, I am seeing the same for the first load except now the empscripten-download-monitor logs include the php-wasm download.

[brandonpayton]

@adamziel I think this must be happening because the worker-thread API, which exposes the onDownloadProgress method, is not marked as ready until after the sqlite-database-integration and WP zip downloads have completed.

How:

• PHPWorker has the onDownloadProgress() method
• the worker-thread declares the PlaygroundWorkerEndpoint API which inherits from PHPWorker
• worker-thread exposes an instance of the PlaygroundWorkerEndpoint API
• worker-thread awaits the sqlite-database-integration ZIP download and awaits bootWordPress() which downloads the WordPress ZIP
• worker-thread only marks the PlaygroundWorkerEndpoint API as ready after those downloads have resolved

Consumers of the PlaygroundWorkerEndpoint API hook into onDownloadProgress(callback), but they can only do it after the API is marked as ready. And the API is only marked as ready after the ZIP downloads complete.

[brandonpayton]

Maybe we should:

1. Mark the PlaygroundWorkerEndpoint API as ready earlier so consumers can receive progress updates
2. Expose an isBooted(): Promise method on the PlaygroundWorkerEndpoint API and have consumers wait on that before acting as if WordPress/Playground is ready to run.

As an aside, if we ever want to consider allowing PHP subrequests during WordPress install (see #1644), we could use a similar approach so PHP request routing can be set up before WP install.

[adamziel]

@brandonpayton setAPIReady() resolves the client.isReady() promise on the other end, but onDownloadProgress() still works earlier on once client.isConnected() resolves. onDownloadProgress() is first called before .isReady():

wordpress-playground/packages/playground/client/src/index.ts

Lines 180 to 181 in 51e5502

	await playground.onDownloadProgress(downloadPHPandWP.loadingListener);
	await playground.isReady();

[brandonpayton]

@adamziel Thank you for correcting my mental model. 🙇

Accepting that my previous understanding is incorrect, I still wonder why we are seeing progress events in the worker-thread while the progress tracker is not. When I'm back from AFK (and if this issue still exists), I should probably instrument the rest of the progress consumers and observe what is happening.

There at least seems to be a break in the chain leading to the progress-tracker because emscripten-download-monitor in worker-thread is seeing steps of progress during download.

[brandonpayton]

Actually, I was wrong. This change in boot-playground-remote.ts logs the progress events in the first event listener, and the second one is only registered after all the progress events have been emitted:

Sorry, @adamziel. You have been saying this all along. I'm not sure why I didn't notice yesterday as I did read your comment. Probably good that I'm (mostly) take a break today.

[brandonpayton]

After merging the boot changes in #1669, this is still broken. Planning to look at this after merging #1679.

[brandonpayton]

After merging the boot changes in #1669, this is still broken. Planning to look at this after merging #1679.

This has been a bit confusing as sometimes the progress bar seems to work fine, and other times it seems to stall for quite a while before finishing. I wonder if this is a problem with something else causing slow initialization or somehow blocking progress reporting.

[adamziel]

Update: The progress bar stays empty for a while, but then it starts gradually filling up. I don't believe it reflects the actual download progress, but at least it does something. Let's hold on with this work until #1731 lands as it profoundly changes the state management logic and I'd rather avoid fixing this issue twice.