From a2dd3caaf31097a1850322248337aaf34ac3a0da Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 8 Oct 2026 14:38:08 +0000 Subject: [PATCH] :tada: Unify the renderer cancel around one injected check Both drivers take the same sys-first shape and read the job fate from a single zero-arg check: cooperative checkpoints on the browser, checkpoints plus a self-abort of the leased worker on wasm, whose signal buffer is born in the lease itself. The facade takes sys plus a closed, validated task map, so a bad task never touches a pool. Sys stays task-free infrastructure. AI-assisted-by: muse-spark-1.3-contributor --- .serena/memories/exporter/core.md | 5 +- exporter/src/exporter/renderer.cljs | 86 +++++----- exporter/src/exporter/renderer/browser.cljs | 70 +++++--- exporter/src/exporter/renderer/wasm.cljs | 102 ++++++----- exporter/src/exporter/wasm/pool.cljs | 14 +- .../exporter_tests/renderer_browser_test.cljs | 60 ++++++- .../test/exporter_tests/renderer_test.cljs | 120 ++++++++----- .../exporter_tests/renderer_wasm_test.cljs | 160 +++++++++--------- .../wasm_pool_service_test.cljs | 4 +- 9 files changed, 376 insertions(+), 245 deletions(-) diff --git a/.serena/memories/exporter/core.md b/.serena/memories/exporter/core.md index 5b42bb11cc..6bbe196bea 100644 --- a/.serena/memories/exporter/core.md +++ b/.serena/memories/exporter/core.md @@ -43,8 +43,9 @@ on this side: the row of the job, its cancel and its result are the backend's. - Service config maps take plain (non-namespaced) param keys (`{:exporter.browser/pool {:max 2}}`): the top-level key already scopes the component, so namespacing the params only adds noise. Component keys and error reasons stay namespaced. - `exporter.wasm.pool` mirrors the browser service for `worker_threads`: the running instance is `{:pool :timeout-ms}` (the pool plus the silence timeout its renders await), `with-worker`/`render-on`/`terminate` take the pool first, the worker factory is injected through config, and the creation handshake (`await-ready`) is its own unit with `:worker-not-ready`/`boot-error` paths. - `exporter.renderer.browser` merges the bitmap/pdf/svg flows into one `render [ctx params on-object]` (branching by type, `on-object` always awaited, unknown types raise); `ctx` carries what the job params do not freeze (`:browser-pool`, `:base-uri`, `:public-uri`, `:svgo?`). Dead xml predicates from the old tree did not survive the merge. -- `exporter.renderer.wasm` drives headless renders off the wasm pool instance (`{:pool :timeout-ms}`): `with-scope` leases one worker and serializes its renders, and the cancel source arrives injected (`:cancel-signal`/`:cancelled?`/`:on-cancel` fns) instead of reaching the jobs registry. -- `exporter.renderer` is the malli-validated facade over both drivers: `render [env params on-object]` dispatches by `:is-wasm`, `with-scope [env exports f]` leases one headless worker for the whole call. `env` is flat (`:browser-pool :base-uri :public-uri :svgo? :wasm-pool :cancel-source`); the browser ctx is derived by `select-keys`. Params validation raises `:data-validation`. +- `exporter.renderer.wasm` drives headless renders off the wasm pool instance (`{:pool :timeout-ms}`): `with-scope [sys lease]` leases one worker and serializes its renders, where the lease map carries `on-scope` (fn of the leased render fn, firing every headless export through it), `check-cancelled` and `watch-interval` (tests only). Cancel is fully driver-owned: the injected zero-arg check (raising `:job-cancelled`) runs before each render, and a watchdog interval polls it mid-render — on cancel the leased worker aborts (per-lease SharedArrayBuffer flipped, thread terminated) and the render rejects. The buffer is born in the lease; nobody outside knows what it is. A nil check arms no interval and wires no cancel. +- `exporter.renderer` is the malli-validated facade with one entry point: `render [sys & task]` takes a task map (`:exports :on-object :check-cancelled`) and fires the whole batch at once, the headless exports sharing one leased worker, the browser ones on their own checkouts. `sys` is task-free infrastructure with process lifetime (pools, uris, flags); everything per-task rides in the validated task map. One closed schema validates data plus injections, so a bad task — or a misspelled key — never touches a pool; task validation raises `:data-validation`. Each driver takes the same `[sys params on-object check]` shape and implements cancel internally: cooperative checkpoints on the browser, checkpoints plus a mid-render abort of its own leased worker on wasm. The scope fn survives one level down only, as the lease primitive inside `exporter.renderer.wasm`. +- The runner stays backend-blind: plan → render batch → settle, with beats/watchdog/settle untouched. Its `on-object` calls the same injected check the drivers use. When `exporter.jobs` is ported, its only cancel surface toward renderers is one zero-arg check constructor per job; the old `cancel-fns`/terminate-registration apparatus does not survive (abort ownership moved to the lease holder). - `exporter.main` wires both pools (`:exporter.browser/pool` and `:exporter.wasm.pool/pool`, each sized to the worker concurrency). No test boots the production wiring: the wasm pool warms one worker eagerly and the worker script in a test process would be the test bundle itself; the wiring is asserted as data and the lifecycle rides fake configs. - Async test vars prove their chain only when really awaited: a test passing with 0 assertions means the chain broke upstream. diff --git a/exporter/src/exporter/renderer.cljs b/exporter/src/exporter/renderer.cljs index fb2c8a196d..e3b134f94e 100644 --- a/exporter/src/exporter/renderer.cljs +++ b/exporter/src/exporter/renderer.cljs @@ -7,13 +7,27 @@ (ns exporter.renderer "Common renderer interface, validated with malli from day one. - Dispatches one render at a time to the browser or the headless - driver by the `:is-wasm` flag of the params. `env` carries - everything the params do not freeze: the browser context - (`:browser-pool`, `:base-uri`, `:public-uri`, `:svgo?`), the wasm - pool instance (`:wasm-pool`, the `{:pool :timeout-ms}` map the pool - service owns) and the injected cancel source (`:cancel-source`, - the `:cancel-signal`/`:cancelled?`/`:on-cancel` fns)." + One entry point: `render` takes `sys` plus a task map + (`:exports`, `:on-object`, `:check-cancelled`) and fires the whole + batch at once, dispatching each export to the browser or the + headless driver by its `:is-wasm` flag. A single export is a batch + of one. The headless ones share one leased worker (serialized on + it); the browser ones check out their own browser each (bounded by + the pool). Rendered objects travel through `on-object`, so the + batch itself resolves a vector of nils. + + `sys` is task-free infrastructure with process lifetime — the + browser pool and render uris, the wasm pool instance, flags — what + could one day come straight from the running system map. The task + map holds everything per-task: the `exports` data plus the two + injected indirections, `on-object` (the per-object collector) and + `check-cancelled` (the zero-arg cancel check raising + `:job-cancelled`). One closed schema validates the whole task, so a + bad batch — or a non-fn injection — never touches a pool. Each + driver takes the same `[sys params on-object check]` shape and + implements cancel internally: cooperative checkpoints on the + browser, checkpoints plus a mid-render abort of its own leased + worker on wasm." (:require [app.common.schema :as sm] [exporter.renderer.browser :as browser] @@ -44,15 +58,17 @@ [:is-wasm {:optional true} ::sm/boolean] [:job-id {:optional true} ::sm/uuid]]) -(def ^:private check-params - (sm/check-fn schema:render-params :hint "invalid render params")) +(def schema:exports + [:vector {:min 1} schema:render-params]) -(def ^:private check-callback - (sm/check-fn ::sm/fn :hint "on-object must be a fn")) +(def schema:task + [:map {:closed true} + [:exports schema:exports] + [:on-object ::sm/fn] + [:check-cancelled ::sm/fn]]) -(defn- browser-ctx - [env] - (select-keys env [:browser-pool :base-uri :public-uri :svgo?])) +(def ^:private check-task + (sm/check-fn schema:task :hint "invalid render task")) (defn headless? "Whether `params` renders with render-wasm rather than a browser." @@ -60,28 +76,22 @@ (boolean is-wasm)) (defn ^:async render - [env params on-object] - (check-params params) - (check-callback on-object) - (if (headless? params) - (await (wasm/render (:wasm-pool env) (:cancel-source env) params on-object)) - (await (browser/render (browser-ctx env) params on-object)))) + "Renders every export, firing them all at once. The whole task is + validated before anything is leased, so a bad task never touches a + pool. Rejects on the first failure." + [sys & {:as task}] + (let [{:keys [exports on-object check-cancelled] :as task} + (check-task task) -(defn ^:async with-scope - "Runs `f`, a fn of a render fn with the same signature as `render`. - Exports that render headless share one worker for the whole call - instead of acquiring one per render; the browser backend keeps - rendering them one checkout at a time." - [env exports f] - (if (some headless? exports) - (await (wasm/with-scope (:wasm-pool env) (:cancel-source env) - (:job-id (first exports)) - (fn [leased] - (f (fn [params on-object] - (check-params params) - (check-callback on-object) - (if (headless? params) - (leased params on-object) - (browser/render (browser-ctx env) params on-object))))))) - (await (f (fn [params on-object] - (render env params on-object)))))) + do-render + (fn [wasm-render] + (js/Promise.all + (mapv (fn [params] + (if (and (headless? params) (fn? wasm-render)) + (wasm-render params on-object) + (browser/render sys params on-object check-cancelled))) + exports)))] + + (if (some headless? exports) + (await (wasm/with-scope sys (assoc task :on-scope do-render))) + (await (do-render nil))))) diff --git a/exporter/src/exporter/renderer/browser.cljs b/exporter/src/exporter/renderer/browser.cljs index bb347edf5c..b28ba49d17 100644 --- a/exporter/src/exporter/renderer/browser.cljs +++ b/exporter/src/exporter/renderer/browser.cljs @@ -12,9 +12,10 @@ with each object carrying its `:path`. `on-object` may return a plain value or a promise; it is always awaited. - `ctx` carries the environment the job params do not freeze: the - `:browser-pool`, the render `:base-uri` and `:public-uri`, and the - `:svgo?` flag. There are no promesa chains in this namespace." + `sys` is task-free infrastructure: the `:browser-pool`, the + render `:base-uri` and `:public-uri`, and the `:svgo?` flag. The + per-task injections (`on-object`, `check`) ride positional. There + are no promesa chains in this namespace." (:require ["@penpot/svgo" :as svgo] ["xml-js" :as xml] @@ -33,6 +34,15 @@ (l/set-level! :trace) +(defn- check-cancelled + "Cooperative checkpoint over the injected zero-arg check. This + backend never blocks indefinitely (every step yields), so + yielding-point checks are the whole cancel story here; a nil check + wires no cancel." + [check] + (when check + (check))) + (defn- prepare-options [uri token scale] #js {:screen #js {:width browser/default-viewport-width @@ -81,15 +91,15 @@ nil) (defn- ^:async render-bitmap - [ctx {:keys [file-id page-id share-id token scale type objects skip-children]} on-object] + [sys {:keys [file-id page-id share-id token scale type objects skip-children]} on-object] (let [query {:file-id file-id :page-id page-id :share-id share-id :object-id (mapv :id objects) :route "objects" :skip-children skip-children} - uri (render-uri (:base-uri ctx) query)] - (await (browser/exec (:browser-pool ctx) + uri (render-uri (:base-uri sys) query)] + (await (browser/exec (:browser-pool sys) (prepare-options uri token scale) (fn [page] (render-bitmap-page uri page type objects on-object)))))) @@ -141,19 +151,20 @@ (await (on-object (assoc object :path path)))))) (defn- ^:async render-pdf-page - [base-uri page params objects on-object] + [base-uri page params objects on-object check] (doseq [object objects] + (check-cancelled check) (await (render-pdf-object page base-uri params object on-object))) nil) (defn- ^:async render-pdf - [ctx {:keys [token scale] :as params} on-object] - (let [base-uri (-> (:base-uri ctx) + [sys {:keys [token scale] :as params} on-object check] + (let [base-uri (-> (:base-uri sys) (u/ensure-path-slash))] - (await (browser/exec (:browser-pool ctx) + (await (browser/exec (:browser-pool sys) (prepare-options base-uri token scale) (fn [page] - (render-pdf-page base-uri page params (:objects params) on-object)))))) + (render-pdf-page base-uri page params (:objects params) on-object check)))))) ;; --- SVG @@ -429,7 +440,7 @@ (str/replace content " " " ")) (defn- ^:async render-svg-object - [ctx page type object on-object] + [sys page type object on-object] (let [path (await (sh/tempfile :prefix "penpot.tmp.render.svg." :suffix (mime/get-extension type))) node (browser/select page (str/concat "#screenshot-" (:id object)))] (await (browser/wait-for node)) @@ -438,49 +449,56 @@ result (replace-text-nodes xmldata txtdata) result (sanitize-svg-content result) - result (if (:svgo? ctx) + result (if (:svgo? sys) (svgo/optimize result svgo/defaultOptions) result) - result (replace-internal-uris result (:base-uri ctx) (:public-uri ctx))] + result (replace-internal-uris result (:base-uri sys) (:public-uri sys))] (await (sh/write-file path result)) (await (on-object (assoc object :path path)))))) (defn- ^:async render-svg-page - [ctx uri page type objects on-object] + [sys uri page type objects on-object] (l/info :uri uri) ;; navigate to the page and perform basic setup (await (browser/nav page (str uri))) (await (browser/sleep page 1000)) ; the good old fix with sleep (await (browser/wait-for-fonts page)) ;; take the screnshot of requested objects, one by one - (await (js/Promise.all (mapv (fn [object] (render-svg-object ctx page type object on-object)) + (await (js/Promise.all (mapv (fn [object] (render-svg-object sys page type object on-object)) objects))) nil) (defn- ^:async render-svg - [ctx {:keys [file-id page-id share-id token scale type objects]} on-object] + [sys {:keys [file-id page-id share-id token scale type objects]} on-object] (let [query {:file-id file-id :page-id page-id :share-id share-id :render-embed true :object-id (mapv :id objects) :route "objects"} - uri (render-uri (:base-uri ctx) query)] - (await (browser/exec (:browser-pool ctx) + uri (render-uri (:base-uri sys) query)] + (await (browser/exec (:browser-pool sys) (prepare-options uri token scale) - (fn [page] (render-svg-page ctx uri page type objects on-object)))))) + (fn [page] (render-svg-page sys uri page type objects on-object)))))) (defn ^:async render "Renders one partition of objects through a browser page: bitmap (png, jpeg, webp), pdf or svg depending on the type in `params`. - Calls `on-object` with each object carrying its `:path`, awaiting it - every time, and resolves nil." - [ctx {:keys [type] :as params} on-object] + Same shape as `exporter.renderer.wasm/render`: `sys` is task-free + infrastructure (pools, uris, flags), while `on-object` and `check` + ride positional as the per-task injections — the per-object + collector and the zero-arg cancel check raising `:job-cancelled`. + The check runs here at fail-fast entry plus the sequential pdf + loop; the concurrent flows rely on the entry check and the caller's + `on-object`. Calls `on-object` with each object carrying its + `:path`, awaiting it every time, and resolves nil." + [sys {:keys [type] :as params} on-object check] + (check-cancelled check) (case type - (:png :jpeg :webp) (await (render-bitmap ctx params on-object)) - :pdf (await (render-pdf ctx params on-object)) - :svg (await (render-svg ctx params on-object)) + (:png :jpeg :webp) (await (render-bitmap sys params on-object)) + :pdf (await (render-pdf sys params on-object check)) + :svg (await (render-svg sys params on-object)) (ex/raise :type :validation :code :unknown-render-type :hint (str "unknown render type: " type)))) diff --git a/exporter/src/exporter/renderer/wasm.cljs b/exporter/src/exporter/renderer/wasm.cljs index 3c39f33305..cae8480ca1 100644 --- a/exporter/src/exporter/renderer/wasm.cljs +++ b/exporter/src/exporter/renderer/wasm.cljs @@ -8,18 +8,21 @@ "Main-thread side of the headless renderer. Renders run on pooled workers because Skia calls are synchronous and - would block the event loop for every other export. Each job keeps one - worker for all its renders, sharing its caches and pool slot. + would block the event loop for every other export. Each batch keeps + one worker for all its renders, sharing its caches and pool slot. - The worker pool arrives as the instance map the pool service owns - (`{:pool :timeout-ms}`); the cancel source arrives as plain fns, so - production wires the jobs registry and tests wire atoms: - - ```clojure - {:cancel-signal (fn [job-id] ...) - :cancelled? (fn [job-id] ...) - :on-cancel (fn [job-id f] ...)} - ```" + `sys` is task-free infrastructure (the wasm pool instance, the + `{:pool :timeout-ms}` map the pool service owns — hence unwrapping + one level to reach the raw pool). The lease map holds the rest: + `on-scope`, a fn receiving the leased 2-arg render fn and firing + every headless export through it, and `check-cancelled`, the + zero-arg cancel check raising `:job-cancelled` once the job ended. + Cancel is fully driver-owned: the check runs before each render, + and a watchdog interval polls it mid-render — on cancel the leased + worker aborts (shared signal flipped, thread terminated) and the + render rejects. The buffer the worker polls is born in the lease + itself; nobody outside knows what it is. A nil check arms no + interval and wires no cancel." (:require [app.common.logging :as l] [exporter.wasm.pool :as pool])) @@ -35,36 +38,51 @@ result)))) (defn ^:async with-scope - "Runs `f`, a fn of a 2-arg render fn. Every render goes to the same - worker, one at a time, so the cancel check runs as each render's turn - comes up. Without a `job-id` nothing is wired for cancel." - [wpool cancel-source job-id f] - (let [{:keys [cancel-signal cancelled? on-cancel]} cancel-source] - (await - (pool/with-worker (:pool wpool) - (fn [worker] - ((^:async fn [] - (let [chain (serializer) - live (volatile! worker) - signal (when job-id (cancel-signal job-id)) - opts {:cancel-buffer (some-> signal (.-buffer)) - :cancelled? (when job-id (fn [] (cancelled? job-id))) - :timeout-ms (:timeout-ms wpool)}] - (when job-id - ;; Between objects the worker sees the flag; inside a - ;; render only terminating the thread stops it. Cleared - ;; on the way out so a later cancel cannot terminate a - ;; worker that is by then somebody else's. - (on-cancel job-id (fn [] (pool/terminate @live)))) - (try - (await (f (fn [params on-object] - (chain (fn [] (pool/render-on worker params on-object opts)))))) - (finally - (vreset! live nil))))))))))) + "Leases one worker for the batch: `on-scope` fires every headless + export through the leased render fn, one at a time on the same + worker, so the check runs as each render's turn comes up. The + interval dies with the lease; a truly stuck thread is backstopped + by the pool's own silence watchdog." + [sys {:keys [check-cancelled on-scope watch-interval] :or {watch-interval 1000}}] + (await + (pool/with-worker (:pool (:wasm-pool sys)) + (fn [worker] + ((^:async fn [] + (let [chain (serializer) + live (volatile! worker) + signal (js/Int32Array. (js/SharedArrayBuffer. 4)) + timer (volatile! nil) + opts {:cancel-buffer (.-buffer signal) + :check-cancelled check-cancelled + :timeout-ms (:timeout-ms (:wasm-pool sys))}] + (vreset! timer + (when check-cancelled + (js/setInterval + (fn [] + (try + (check-cancelled) + (catch :default _cause + ;; One-shot: the lease is doomed anyway. + ;; Between objects the worker sees the + ;; flag; inside a render only terminating + ;; the thread stops it. + (some-> @timer js/clearInterval) + (js/Atomics.store signal 0 1) + (some-> @live pool/terminate)))) + watch-interval))) + (try + (await (on-scope (fn [params on-object] + (chain (fn [] (pool/render-on worker params on-object opts)))))) + (finally + (some-> @timer js/clearInterval) + (vreset! live nil)))))))))) (defn ^:async render - [wpool cancel-source params on-object] - (let [job-id (:job-id params)] - (l/info :hint "render" :type (:type params) :backend "wasm") - (await (with-scope wpool cancel-source job-id - (fn [render*] (render* params on-object)))))) + "Same shape as `exporter.renderer.browser/render`: one export as a + batch of one." + [sys params on-object check] + (l/info :hint "render" :type (:type params) :backend "wasm") + (await (with-scope sys {:exports [params] + :on-object on-object + :check-cancelled check + :on-scope (fn [render-wasm] (render-wasm params on-object))}))) diff --git a/exporter/src/exporter/wasm/pool.cljs b/exporter/src/exporter/wasm/pool.cljs index 66653fed01..cacb3b69ff 100644 --- a/exporter/src/exporter/wasm/pool.cljs +++ b/exporter/src/exporter/wasm/pool.cljs @@ -241,13 +241,13 @@ :cancel cancel-buffer})))))) (defn ^:async render-on - "Renders `params` on an already acquired worker." - [worker params on-object {:keys [cancel-buffer cancelled? timeout-ms]}] - (if (and cancelled? (cancelled?)) - (throw (ex/error :type :internal - :code :job-cancelled - :hint "export job was cancelled")) - (await (render-on-worker worker params cancel-buffer on-object timeout-ms)))) + "Renders `params` on an already acquired worker. The check runs + before posting: a render whose turn comes up cancelled rejects + without touching the worker." + [worker params on-object {:keys [cancel-buffer check-cancelled timeout-ms]}] + (when check-cancelled + (check-cancelled)) + (await (render-on-worker worker params cancel-buffer on-object timeout-ms))) (defn terminate [worker] diff --git a/exporter/test/exporter_tests/renderer_browser_test.cljs b/exporter/test/exporter_tests/renderer_browser_test.cljs index 4f4fd0d0bf..0bf64296dd 100644 --- a/exporter/test/exporter_tests/renderer_browser_test.cljs +++ b/exporter/test/exporter_tests/renderer_browser_test.cljs @@ -62,13 +62,28 @@ :validate (fn [_] true)} #js {:max 1 :min 0})) -(defn- test-ctx +(defn- test-sys [pool] {:browser-pool pool :base-uri (cf/get-internal-uri) :public-uri (cf/get :public-uri) :svgo? false}) +(defn- never-cancelled + [] + (fn [] nil)) + +(defn- raising-check + [calls raise-on-call] + (let [seen (atom 0)] + (fn [] + (swap! calls conj :check) + (when (= raise-on-call (swap! seen inc)) + (throw (ex-info "export job was cancelled" + {:type :internal + :code :job-cancelled + :hint "export job was cancelled"})))))) + (defn- test-object [id] {:id id :name id}) @@ -94,7 +109,7 @@ page (stub-page calls (stub-locator calls (fn [_] nil))) pool (test-pool calls page) {:keys [seen fn]} (collect-on-object)] - (await (render/render (test-ctx pool) (test-params :png ["a" "b"]) fn)) + (await (render/render (test-sys pool) (test-params :png ["a" "b"]) fn (never-cancelled))) (t/is (= 2 (count @seen))) (t/is (every? #(str/ends-with? (:path %) ".png") @seen)) (t/is (= ["a" "b"] (mapv :id @seen))) @@ -113,7 +128,7 @@ page (stub-page calls (stub-locator calls (fn [_] nil))) pool (test-pool calls page) {:keys [seen fn]} (collect-on-object)] - (await (render/render (test-ctx pool) (test-params :pdf ["a" "b"]) fn)) + (await (render/render (test-sys pool) (test-params :pdf ["a" "b"]) fn (never-cancelled))) (t/is (= 2 (count @seen))) (t/is (every? #(str/ends-with? (:path %) ".pdf") @seen)) (t/testing "one navigation per object" @@ -122,6 +137,43 @@ (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) +(t/deftest ^:async cancelled-render-rejects-before-navigating + (try + (let [calls (atom []) + page (stub-page calls (stub-locator calls (fn [_] nil))) + pool (test-pool calls page) + sys (test-sys pool)] + (try + (await (render/render sys (test-params :png ["a"]) + (fn [_] (swap! calls conj :object)) + (raising-check calls 1))) + (t/is false "render should have rejected a cancelled render") + (catch :default cause + (t/is (= :job-cancelled (-> cause ex-data :code))) + (t/is (not-any? #(and (vector? %) (= :goto (first %))) @calls)) + (t/is (not-any? #{:object} @calls))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async cancel-mid-batch-stops-the-pdf-loop + (try + (let [calls (atom []) + page (stub-page calls (stub-locator calls (fn [_] nil))) + pool (test-pool calls page) + ;; entry check is the first call, then one per object: + ;; the cancel lands as the second object comes up + sys (test-sys pool) + {:keys [seen fn]} (collect-on-object)] + (try + (await (render/render sys (test-params :pdf ["a" "b"]) fn (raising-check calls 3))) + (t/is false "render should have rejected a cancelled batch") + (catch :default cause + (t/is (= :job-cancelled (-> cause ex-data :code))) + (t/is (= ["a"] (mapv :id @seen))) + (t/is (= 1 (count (filter #(and (vector? %) (= :goto (first %))) @calls))))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + (t/deftest ^:async renders-svg-reassembling-the-document (try (let [calls (atom []) @@ -130,7 +182,7 @@ page (stub-page calls locator) pool (test-pool calls page) {:keys [seen fn]} (collect-on-object) - _ (await (render/render (test-ctx pool) (test-params :svg ["a"]) fn)) + _ (await (render/render (test-sys pool) (test-params :svg ["a"]) fn (never-cancelled))) path (:path (first @seen)) content (await (sh/read-file path))] (t/is (= 1 (count @seen))) diff --git a/exporter/test/exporter_tests/renderer_test.cljs b/exporter/test/exporter_tests/renderer_test.cljs index 232c74e778..7ad56b0a3e 100644 --- a/exporter/test/exporter_tests/renderer_test.cljs +++ b/exporter/test/exporter_tests/renderer_test.cljs @@ -84,21 +84,23 @@ :release (fn [_] (swap! calls conj :release) (js/Promise.resolve nil)) :destroy (fn [_] (swap! calls conj :destroy) (js/Promise.resolve nil))}) -(defn- stub-cancel-source +(defn- stub-check [calls] - {:cancel-signal (fn [job-id] (swap! calls conj [:cancel-signal job-id]) nil) - :cancelled? (fn [job-id] (swap! calls conj [:cancelled?-asked job-id]) false) - :on-cancel (fn [job-id f] (swap! calls conj [:on-cancel job-id f]) nil)}) + (fn [] + (swap! calls conj :check))) -(defn- test-env +(defn- test-sys [calls] (let [worker (stub-worker calls)] - {:browser-pool (stub-browser-pool calls) - :base-uri (cf/get-internal-uri) - :public-uri (cf/get :public-uri) - :svgo? false - :wasm-pool {:pool (stub-wasm-pool calls worker) :timeout-ms 5000} - :cancel-source (stub-cancel-source calls)})) + {:browser-pool (stub-browser-pool calls) + :base-uri (cf/get-internal-uri) + :public-uri (cf/get :public-uri) + :svgo? false + :wasm-pool {:pool (stub-wasm-pool calls worker) :timeout-ms 5000}})) + +(defn- never-cancelled + [calls] + (stub-check calls)) (defn- test-params [type opts] @@ -119,9 +121,10 @@ (try (let [calls (atom []) seen (atom [])] - (await (renderer/render (test-env calls) - (test-params :png nil) - (fn [object] (swap! seen conj object) nil))) + (await (renderer/render (test-sys calls) + :exports [(test-params :png nil)] + :on-object (fn [object] (swap! seen conj object) nil) + :check-cancelled (never-cancelled calls))) (t/is (= ["a"] (mapv :id @seen))) (t/is (some #(and (vector? %) (= :goto (first %))) @calls)) (t/is (not-any? #(and (vector? %) (= :post (first %))) @calls))) @@ -133,12 +136,13 @@ (let [calls (atom []) job-id (uuid/next) seen (atom [])] - (await (renderer/render (test-env calls) - (test-params :png {:is-wasm true :job-id job-id}) - (fn [object] (swap! seen conj object) nil))) + (await (renderer/render (test-sys calls) + :exports [(test-params :png {:is-wasm true :job-id job-id})] + :on-object (fn [object] (swap! seen conj object) nil) + :check-cancelled (never-cancelled calls))) (t/is (= [{:id "w1" :path "/tmp/w1.png"}] @seen)) (t/is (some #{[:post "render"]} @calls)) - (t/is (some #{[:cancel-signal job-id]} @calls)) + (t/is (some #{:check} @calls)) (t/is (not-any? #(and (vector? %) (= :goto (first %))) @calls))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) @@ -147,9 +151,10 @@ (try (let [calls (atom [])] (try - (await (renderer/render (test-env calls) - (dissoc (test-params :png nil) :objects) - (fn [_] nil))) + (await (renderer/render (test-sys calls) + :exports [(dissoc (test-params :png nil) :objects)] + :on-object (fn [_] nil) + :check-cancelled (never-cancelled calls))) (t/is false "render should have rejected params without objects") (catch :default cause (t/is (= :data-validation (-> cause ex-data :code)))))) @@ -160,47 +165,78 @@ (try (let [calls (atom [])] (try - (await (renderer/render (test-env calls) (test-params :png nil) :not-a-fn)) + (await (renderer/render (test-sys calls) + :exports [(test-params :png nil)] + :on-object :not-a-fn + :check-cancelled (never-cancelled calls))) (t/is false "render should have rejected a non-fn on-object") (catch :default cause (t/is (= :data-validation (-> cause ex-data :code)))))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) -(t/deftest ^:async with-scope-shares-one-worker-across-headless-renders +(t/deftest ^:async render-shares-one-worker-across-headless-renders (try (let [calls (atom []) job-id (uuid/next) - env (test-env calls) + sys (test-sys calls) seen (atom [])] - (await (renderer/with-scope env - [(test-params :png {:is-wasm true :job-id job-id}) - (test-params :png {:is-wasm true :job-id job-id}) - (test-params :png {:job-id job-id})] - (^:async fn [render*] - (await (render* (test-params :png {:is-wasm true :job-id job-id}) - (fn [object] (swap! seen conj object) nil))) - (await (render* (test-params :png {:is-wasm true :job-id job-id}) - (fn [object] (swap! seen conj object) nil))) - (await (render* (test-params :png {:job-id job-id}) - (fn [object] (swap! seen conj object) nil)))))) + (await (renderer/render sys + :exports [(test-params :png {:is-wasm true :job-id job-id}) + (test-params :png {:is-wasm true :job-id job-id}) + (test-params :png {:job-id job-id})] + :on-object (fn [object] (swap! seen conj object) nil) + :check-cancelled (never-cancelled calls))) (t/is (= 1 (count (filter #{:acquire} @calls)))) (t/is (= 1 (count (filter #{:release} @calls)))) (t/is (= 3 (count @seen)))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) -(t/deftest ^:async with-scope-without-headless-renders-directly +(t/deftest ^:async render-without-headless-renders-directly (try (let [calls (atom []) - env (test-env calls) + sys (test-sys calls) seen (atom [])] - (await (renderer/with-scope env - [(test-params :png nil)] - (fn [render*] - (render* (test-params :png nil) - (fn [object] (swap! seen conj object) nil))))) + (await (renderer/render sys + :exports [(test-params :png nil)] + :on-object (fn [object] (swap! seen conj object) nil) + :check-cancelled (never-cancelled calls))) (t/is (= ["a"] (mapv :id @seen))) (t/is (not-any? #{:acquire} @calls))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async render-validates-before-leasing + (try + (let [calls (atom []) + job-id (uuid/next) + sys (test-sys calls)] + (try + (await (renderer/render sys + :exports [(test-params :png {:is-wasm true :job-id job-id}) + (dissoc (test-params :png nil) :objects)] + :on-object (fn [_] nil) + :check-cancelled (never-cancelled calls))) + (t/is false "render should have rejected the bad batch") + (catch :default cause + (t/is (= :data-validation (-> cause ex-data :code))) + (t/testing "no worker was leased for a batch that never renders" + (t/is (not-any? #{:acquire} @calls)))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async render-rejects-unknown-task-keys + (try + (let [calls (atom [])] + (try + (await (renderer/render (test-sys calls) + :exports [(test-params :png nil)] + :on-objec (fn [_] nil) + :check-cancelled (never-cancelled calls))) + (t/is false "render should have rejected the misspelled key") + (catch :default cause + (t/is (= :data-validation (-> cause ex-data :code))) + (t/is (not-any? #(and (vector? %) (= :goto (first %))) @calls))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) diff --git a/exporter/test/exporter_tests/renderer_wasm_test.cljs b/exporter/test/exporter_tests/renderer_wasm_test.cljs index 4831f1f59a..a1ef0825c8 100644 --- a/exporter/test/exporter_tests/renderer_wasm_test.cljs +++ b/exporter/test/exporter_tests/renderer_wasm_test.cljs @@ -6,8 +6,8 @@ (ns exporter-tests.renderer-wasm-test "The wasm renderer driver: leased workers, sequential renders, - failure isolation and cancel wiring, against a stub pool, stub - workers and an injected cancel source." + failure isolation and driver-owned cancel, against a stub pool, + stub workers and an injected zero-arg check." (:require ["node:events" :as events] [app.common.transit :as transit] @@ -43,21 +43,23 @@ :release (fn [_] (swap! calls conj :release) (js/Promise.resolve nil)) :destroy (fn [_] (swap! calls conj :destroy) (js/Promise.resolve nil))}) -(defn- stub-cancel-source +(defn- stub-check [calls cancelled?] - {:cancel-signal (fn [job-id] (swap! calls conj [:cancel-signal job-id]) nil) - :cancelled? (fn [job-id] (swap! calls conj [:cancelled?-asked job-id]) @cancelled?) - :on-cancel (fn [job-id f] (swap! calls conj [:on-cancel job-id f]) nil)}) + (fn [] + (swap! calls conj :check) + (when @cancelled? + (throw (ex-info "export job was cancelled" + {:type :internal + :code :job-cancelled + :hint "export job was cancelled"}))))) -(defn- test-instance +(defn- test-sys [calls worker] - {:pool (stub-pool calls worker) - :timeout-ms 5000}) + {:wasm-pool {:pool (stub-pool calls worker) :timeout-ms 5000}}) (defn- test-params - [job-id] - {:job-id job-id - :type :png + [] + {:type :png :objects [{:id "o1"}]}) (defn- posted? @@ -66,44 +68,36 @@ (t/deftest ^:async with-scope-runs-renders-on-the-leased-worker (try - (let [calls (atom []) + (let [calls (atom []) worker (stub-worker calls (render-behavior {:id "o1" :path "/tmp/o1.png"})) - seen (atom [])] - (await (render/with-scope (test-instance calls worker) - (stub-cancel-source calls (atom false)) - :job-1 - (^:async fn [render*] - (await (render* (test-params :job-1) - (fn [object] (swap! seen conj object) nil))) - (await (render* (test-params :job-1) - (fn [object] (swap! seen conj object) nil)))))) + seen (atom [])] + (await (render/with-scope (test-sys calls worker) + {:check-cancelled (stub-check calls (atom false)) + :on-scope (^:async fn [render*] + (await (render* (test-params) + (fn [object] (swap! seen conj object) nil))) + (await (render* (test-params) + (fn [object] (swap! seen conj object) nil))))})) (t/is (= [{:id "o1" :path "/tmp/o1.png"} {:id "o1" :path "/tmp/o1.png"}] @seen)) (t/is (= 1 (count (filter #{:release} @calls)))) - (t/testing "the job was wired for cancel exactly once" - (t/is (= [[:cancel-signal :job-1] - [:cancelled?-asked :job-1] - [:cancelled?-asked :job-1]] - (filter #(and (vector? %) (not (contains? #{:post :on-cancel} (first %)))) - @calls))) - (let [registrations (filter #(and (vector? %) (= :on-cancel (first %))) @calls)] - (t/is (= 1 (count registrations))) - (t/is (= :job-1 (second (first registrations))))))) + (t/testing "the check runs as each render's turn comes up" + (t/is (= 2 (count (filter #{:check} @calls))))) + (t/testing "a quiet lease leaves its worker alone" + (t/is (not-any? #{:terminate} @calls)))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) (t/deftest ^:async cancelled-render-rejects-before-posting (try (let [calls (atom []) - worker (stub-worker calls (render-behavior {:id "o1"})) - cancelled (atom true)] + worker (stub-worker calls (render-behavior {:id "o1"}))] (try - (await (render/with-scope (test-instance calls worker) - (stub-cancel-source calls cancelled) - :job-1 - (fn [render*] - (render* (test-params :job-1) (fn [_] nil))))) + (await (render/with-scope (test-sys calls worker) + {:check-cancelled (stub-check calls (atom true)) + :on-scope (fn [render*] + (render* (test-params) (fn [_] nil)))})) (t/is false "with-scope should have rejected a cancelled render") (catch :default cause (t/is (= :job-cancelled (-> cause ex-data :code))) @@ -111,70 +105,70 @@ (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) -(t/deftest ^:async cancel-terminates-the-leased-worker +(t/deftest ^:async cancel-aborts-the-leased-worker-mid-render (try - (let [calls (atom []) - worker (stub-worker calls (fn [_ _])) - handlers (atom [])] + (let [calls (atom []) + worker (stub-worker calls (fn [_ _])) + cancelled (atom false)] (try - (await (render/with-scope (test-instance calls worker) - {:cancel-signal (fn [_] nil) - :cancelled? (fn [_] false) - :on-cancel (fn [_ f] (swap! handlers conj f) nil)} - :job-1 - (fn [render*] - (let [pending (render* (test-params :job-1) (fn [_] nil))] - ;; the cancel lands mid-render, once - ;; the worker is listening - (js/setTimeout (fn [] ((first @handlers))) 10) - pending)))) - (t/is false "with-scope should have rejected a terminated render") + (await (render/with-scope (test-sys calls worker) + {:check-cancelled (stub-check calls cancelled) + :watch-interval 20 + :on-scope (fn [render*] + ;; the cancel lands mid-render, once + ;; the worker is listening + (js/setTimeout #(reset! cancelled true) 10) + (render* (test-params) (fn [_] nil)))})) + (t/is false "with-scope should have rejected an aborted render") (catch :default cause (t/is (= :worker-exited (-> cause ex-data :code))) (t/is (some #{:terminate} @calls))))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) -(t/deftest ^:async render-takes-the-job-id-from-params +(t/deftest ^:async render-leases-and-releases-for-one-render (try - (let [calls (atom []) - worker (stub-worker calls (render-behavior {:id "o1"}))] - (await (render/render (test-instance calls worker) - (stub-cancel-source calls (atom false)) - (test-params :job-9) - (fn [_] nil))) - (t/is (some #{[:cancel-signal :job-9]} @calls))) + (let [calls (atom []) + worker (stub-worker calls (render-behavior {:id "o1"})) + seen (atom [])] + (await (render/render (test-sys calls worker) + (test-params) + (fn [object] (swap! seen conj object) nil) + (stub-check calls (atom false)))) + (t/is (= [{:id "o1"}] @seen)) + (t/is (= 1 (count (filter #{:release} @calls))))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) -(t/deftest ^:async render-without-job-id-wires-no-cancel +(t/deftest ^:async render-without-check-wires-no-cancel (try - (let [calls (atom []) - worker (stub-worker calls (render-behavior {:id "o1"}))] - (await (render/render (test-instance calls worker) - (stub-cancel-source calls (atom false)) - (dissoc (test-params :job-9) :job-id) - (fn [_] nil))) - (t/is (not-any? #(and (vector? %) (not= :post (first %))) @calls))) + (let [calls (atom []) + worker (stub-worker calls (render-behavior {:id "o1"})) + seen (atom [])] + (await (render/render (test-sys calls worker) + (test-params) + (fn [object] (swap! seen conj object) nil) + nil)) + (t/is (= [{:id "o1"}] @seen)) + (t/is (not-any? #{:terminate} @calls))) (catch :default cause (t/is false (str "unexpected failure: " (ex-message cause)))))) (t/deftest ^:async failed-render-does-not-break-the-chain (try - (let [calls (atom []) + (let [calls (atom []) worker (stub-worker calls (render-behavior {:id "o1"})) - seen (atom [])] - (await (render/with-scope (test-instance calls worker) - (stub-cancel-source calls (atom false)) - :job-1 - (^:async fn [render*] - (try - (await (render* (test-params :job-1) - (fn [_] (throw (ex-info "on-object broke" {}))))) - (catch :default _cause - (swap! calls conj :first-failed))) - (await (render* (test-params :job-1) - (fn [object] (swap! seen conj object) nil)))))) + seen (atom [])] + (await (render/with-scope (test-sys calls worker) + {:check-cancelled (stub-check calls (atom false)) + :on-scope (^:async fn [render*] + (try + (await (render* (test-params) + (fn [_] (throw (ex-info "on-object broke" {}))))) + (catch :default _cause + (swap! calls conj :first-failed))) + (await (render* (test-params) + (fn [object] (swap! seen conj object) nil))))})) (t/is (some #{:first-failed} @calls)) (t/is (= [{:id "o1"}] @seen))) (catch :default cause diff --git a/exporter/test/exporter_tests/wasm_pool_service_test.cljs b/exporter/test/exporter_tests/wasm_pool_service_test.cljs index 6b28add398..01aca5a01b 100644 --- a/exporter/test/exporter_tests/wasm_pool_service_test.cljs +++ b/exporter/test/exporter_tests/wasm_pool_service_test.cljs @@ -111,7 +111,9 @@ (pool/render-on worker {} (fn [_]) - {:cancelled? (constantly true) + {:check-cancelled (fn [] + (throw (ex-info "export job was cancelled" + {:code :job-cancelled}))) :timeout-ms 5000})))) (t/is false "render-on should have rejected a cancelled render") (catch :default cause