Repository navigation
fix(studio-server): sub-composition previews build off the request thread within a deadline - #7857
miguel-heygen wants to merge 4 commits into
Conversation
…used off the request thread
…read within a deadline
jrusso1020
left a comment
There was a problem hiding this comment.
Request changes at b4fb520d. Moving the sub-composition build onto a worker under a deadline is the right shape, and the deadline/replace logic reads correctly. But the head doesn't build, and the worker can't start from source, so the new tests and the existing sub-composition route tests fail.
Blocking
-
Typecheck/build fails.
src/helpers/subCompositionBuild.ts:73:started.on("error", (error) => replace(started, error)). Theerrorparameter isunknown, which givesTS2345: Argument of type 'unknown' is not assignable to parameter of type 'Error | undefined'. This failsBuild,Typecheck, and every job that builds the workspace first (Test (studio/cli/runtime/support), CLI smoke, Producer, SDK). Fix:(error) => replace(started, error instanceof Error ? error : new Error(String(error))). -
The worker can't load from source, so every sub-composition preview returns 500 there.
subCompositionWorker.tsimports./subComposition.js. When the.tsentry runs in a Node worker, that relative.jsdoesn't map to the.tsfile. Node 22.18+ strips types itself, so the worker file loads, but its import doesn't resolve. Adding--import tsx/esmto the worker'sexecArgvdoesn't fix it either. I tried it directly on Node 22.22.3 and gotERR_MODULE_NOT_FOUND …/src/helpers/subComposition.js. The precedent,shaderTransitionWorker, only works because it has no relative imports.- Locally at this head:
vitest run src/routes/preview.compDeadline.test.ts src/routes/preview.compPath.test.tsfails 4/4. Each request returns 500 with that error in stderr, including the two existingpreview.compPathtests. - The same path affects any source-mode host, e.g. Studio dev loading studio-server from
src. - Possible fixes: in source mode, start a small
.mjsbootstrap that callsregister()fromtsx/esm/apiand thenimport()s the.tsworker. Or keep the worker free of relative imports. Then re-run both test files.
- Locally at this head:
-
Commentscheck:createStudioApi.tscomment share rose (the new doc line onsubCompositionDeadlineMs). Cut it or move it, per the ratchet's message.
Non-blocking
- A deadline on a queued job kills the running one. When a job times out while another job is running (e.g. one request's
idsjob behind a different request's slowbuild),replace()ends the worker. The running build is resent and starts over from scratch. The comment "after a deadline the culprit is gone" doesn't hold in that case. It's bounded by each job's own deadline, so it isn't a correctness bug. Tracking the in-flight id, and only replacing when the timed-out job is the one running, would avoid it. - The remaining request-thread passes aren't bounded. After the worker returns, the route still runs
recordPreviewReferences(an element walk),transformPreviewHtml,injectStudioPreviewAugmentations(which does aparseHTMLatpreview.ts:218) and the codec-map scan. All of these run synchronously on the request thread, over a page the worker took up to 30 s to build.withinDeadlinecan't interrupt synchronous work. Either a test with a page that builds just inside the deadline, or a note on why those passes are cheap relative to the build, would close this. - The test plan's "fails without the fix" run and the existing
preview.compPathrun are still pending in the description. Point 2 above is what they would have shown.
Need: real (a project's own page could hold the request thread with no bound) · Size: right-sized · Reuse: follows the existing worker-entry pattern, but that pattern doesn't carry over to a worker with relative imports · Simpler: fine · Structure: good split (route / pool / worker)
Risk: as posted, every sub-composition preview 500s in source mode and the workspace doesn't build. Once 1–3 are fixed, the risk is low, with the packaged-CLI dist worker path still unexercised.
— Rames
What
GET /projects/:id/preview/comp/*now builds the sub-composition page on a worker thread, within a deadline (30 s by default). A page still building at the deadline is answered503with " took too long to build", and its worker is ended and replaced.Why
A sub-composition page is the project's own HTML, and a shared or imported project can carry any page. The route built it on the server's request thread with no time limit, in three steps: the hf-id pin on the raw file,
buildSubCompositionHtml, then the hf-id pass on the transformed page. A page big enough to take seconds held every other request to the server for that long. One that never finished held the server for good. The media-codec probes for the page's media had no limit either.Related work
The root preview build already runs through the host's
adapter.bundle, which a host can run off its main thread. This gives the sub-composition build the same separation inside studio-server itself, for every host.How
helpers/subCompositionWorker.ts: the worker. Abuildjob pins hf-ids on the raw file, aspinSubCompHfIdsdid, and runsbuildSubCompositionHtml. Anidsjob pins hf-ids on the page the host transformed.helpers/subCompositionBuild.ts: one long-lived worker, so builds queue one at a time. Its entry is found beside the module that starts it: the built.jsindist, else the.tssource throughtsx, asshaderTransitionWorkerPooldoes.runOffThread(job, deadline, compPath)refuses withSubCompositionDeadlineErroronce the deadline passes. The worker is then ended, and the builds queued behind the refused one go to a fresh worker.withinDeadlineputs the same deadline on the media-codec probes. They are child processes, so nothing blocks; the probes themselves finish and fill the cache.createStudioApi(adapter, { subCompositionDeadlineMs })sets the deadline; tests use a short one.tsup.config.tsin studio-server and the CLI emitsubCompositionWorker.jsbeside the chunks that start it, as the CLI already does forshaderTransitionWorker.Test plan
src/routes/preview.compDeadline.test.ts(Linux run of both commits pending: the box is full; I add its result here.)
preview.compPath.test.ts(existing sub-composition route tests): run pending with the above.Not exercised: a packaged CLI build starting the worker from
dist(the CI build compiles it; nohyperframes previewrun was made), and Windows.