diff --git a/.serena/memories/exporter/core.md b/.serena/memories/exporter/core.md index 06445a6f2d..d8f8ecf0fa 100644 --- a/.serena/memories/exporter/core.md +++ b/.serena/memories/exporter/core.md @@ -39,6 +39,8 @@ on this side: the row of the job, its cancel and its result are the backend's. - New code grows under `exporter.*`, in parallel to the legacy `app.*` tree: `exporter.main` will replace `app.core` once every piece is ported; do not extend `app.*`. - Lifecycle is `exporter.utils.system`: async-aware Integrant-style init/halt (`init-key`/`halt-key` multimethods, `ref`s, `weavejester.dependency` ordering, `^:async` + `cljs.core/await`, no promesa chains); tests are `^:async deftest` with `await` directly in the body. - First ported service is `exporter.browser` (whole `app.browser` at once): the running instance is the raw pool, the public API takes it first (`exec` fails fast on a nil pool), the browser factory is injected through config for hermetic tests, and the new tree carries no promesa. +- `exporter.main` mirrors the backend's `app.main`: a `system-config` def reading env at the wiring layer, a public `system` atom with the running map, `start`/`stop`/`restart` (`^:async`, resolving `:started`/`:stopped`/`:restarted`), `start-custom` as the test seam, idempotent `install-process-handlers` for the `uncaughtException`/SIGTERM/SIGINT wiring (runs on `start`, never on namespace load), `^:dev/before-load-async`/`^:dev/after-load-async` hooks bridging the promise lifecycle to shadow's `done` callbacks (a hook failure logs, never sticks the reload), and `main` as the entry point: shadow boots `exporter.main/main`, and the old `app.core` reload path (`stop` and the `:devtools` wiring) sits commented out until `app.*` is fully ported. The entry leaves info logging enabled for the new tree (`setup!` on the `exporter` logger, mirroring the old `{:app :info}`); without it the default level filters everything below fatal. +- 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. - Async test vars prove their chain only when really awaited: a test passing with 0 assertions means the chain broke upstream. ## Render details diff --git a/exporter/shadow-cljs.edn b/exporter/shadow-cljs.edn index 076e6a10f6..6c4987ce9a 100644 --- a/exporter/shadow-cljs.edn +++ b/exporter/shadow-cljs.edn @@ -6,13 +6,15 @@ {:target :esm :runtime :node :output-dir "target/" - :devtools {:before-load-async app.core/stop - :after-load app.core/start} + ;; Old reload hooks, superseded by the ^:dev metadata on the + ;; exporter.main vars. Listing both would run every hook twice. + ;; :devtools {:before-load-async app.core/stop + ;; :after-load app.core/start} :modules {:app {:entries [] - :init-fn app.core/main}} + :init-fn exporter.main/main}} :js-options {:entry-keys ["module" "browser" "main"] diff --git a/exporter/src/app/core.cljs b/exporter/src/app/core.cljs index 25977ded0c..776de3cafd 100644 --- a/exporter/src/app/core.cljs +++ b/exporter/src/app/core.cljs @@ -46,41 +46,44 @@ (def main start) +;; Old hot reload path, superseded by exporter.main: the shadow entry +;; point moved to exporter.main/main and the reload hooks ride its +;; ^:dev metadata. Kept visible until app.* is fully ported. ;; Draining a pool waits for every checked-out resource to come back, which an ;; export in flight can hold for as long as its own timeout. On a hot reload ;; that would block `start` from ever running again, leaving a drained pool that ;; fails every later job. -(def ^:private shutdown-step-timeout 3000) +#_(def ^:private shutdown-step-timeout 3000) -(defn- shutdown-step - [label f] - (-> (p/race [(p/do (f)) - (p/fmap (constantly ::timeout) (p/delay shutdown-step-timeout))]) - (p/handle (fn [result cause] - (when (or (some? cause) (= ::timeout result)) - (l/warn :hint "shutdown step did not finish cleanly" - :step label - :cause cause)) - nil)))) +#_(defn- shutdown-step + [label f] + (-> (p/race [(p/do (f)) + (p/fmap (constantly ::timeout) (p/delay shutdown-step-timeout))]) + (p/handle (fn [result cause] + (when (or (some? cause) (= ::timeout result)) + (l/warn :hint "shutdown step did not finish cleanly" + :step label + :cause cause)) + nil)))) -(defn stop - [done] - ;; an empty line for visual feedback of restart - (js/console.log "") +#_(defn stop + [done] + ;; an empty line for visual feedback of restart + (js/console.log "") - (if-not ^boolean wt/isMainThread - ;; A render worker owns no server, pools or connections; nothing to unwind. - (done) - (do - (l/info :msg "stopping") - (p/do - (shutdown-step "browser-pool" bwr/stop) - (shutdown-step "wasm-worker-pool" wasm.pool/stop) - ;; the pollers own their connections and may sit in a blocking - ;; pop for the whole poll timeout; draining the pools first lets - ;; the exports in flight unwind against their own resources - (shutdown-step "consumer-worker" worker/stop) - (done))))) + (if-not ^boolean wt/isMainThread + ;; A render worker owns no server, pools or connections; nothing to unwind. + (done) + (do + (l/info :msg "stopping") + (p/do + (shutdown-step "browser-pool" bwr/stop) + (shutdown-step "wasm-worker-pool" wasm.pool/stop) + ;; the pollers own their connections and may sit in a blocking + ;; pop for the whole poll timeout; draining the pools first lets + ;; the exports in flight unwind against their own resources + (shutdown-step "consumer-worker" worker/stop) + (done))))) (.on proc/default "uncaughtException" (fn [cause] diff --git a/exporter/src/exporter/browser.cljs b/exporter/src/exporter/browser.cljs index c572b08384..27f4fa7247 100644 --- a/exporter/src/exporter/browser.cljs +++ b/exporter/src/exporter/browser.cljs @@ -144,12 +144,12 @@ ;; --- POOL SERVICE (def defaults - "Static pool defaults. The sizing (`::max`) mirrors the worker + "Static pool defaults. The sizing (`:max`) mirrors the worker concurrency default; the wiring layer passes the env-derived value." - {::max 2 - ::acquire-timeout 10000 - ::idle-timeout 10000 - ::launch-args ["--allow-insecure-localhost" "--font-render-hinting=none"]}) + {:max 2 + :acquire-timeout 10000 + :idle-timeout 10000 + :launch-args ["--allow-insecure-localhost" "--font-render-hinting=none"]}) (defn- default-create-browser [launch-args] @@ -186,17 +186,17 @@ (defmethod system/init-key ::pool [_ cfg] (let [opts (merge defaults (d/without-nils cfg)) - create-browser (or (::create-browser opts) - (default-create-browser (::launch-args opts)))] - (l/info :hint "initializing browser pool" :max (::max opts)) + create-browser (or (:create-browser opts) + (default-create-browser (:launch-args opts)))] + (l/info :hint "initializing browser pool" :max (:max opts)) (gp/createPool (make-factory create-browser) - #js {:max (::max opts) + #js {:max (:max opts) :min 0 :testOnBorrow true :evictionRunIntervalMillis 5000 :numTestsPerEvictionRun 5 - :acquireTimeoutMillis (::acquire-timeout opts) - :idleTimeoutMillis (::idle-timeout opts)}))) + :acquireTimeoutMillis (:acquire-timeout opts) + :idleTimeoutMillis (:idle-timeout opts)}))) (defn- ^:async drain-pool [pool] diff --git a/exporter/src/exporter/main.cljs b/exporter/src/exporter/main.cljs index 4f0d55f115..000b161461 100644 --- a/exporter/src/exporter/main.cljs +++ b/exporter/src/exporter/main.cljs @@ -5,8 +5,119 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns exporter.main - "Entry point of the new exporter tree. + "Entry point of the new exporter tree, modeled on the backend's + `app.main`: a `system-config` map wired with refs, and `start`, + `stop` and `restart` managing the running system. This namespace will replace `app.core` once every piece of `app.*` - has been ported over. New code grows under `exporter.*`, in parallel - to the legacy `app.*` tree, until the old one can be removed.") + has been ported over. Until then the shadow build still boots + `app.core`; switching the entry point comes last." + (:require + ["node:process" :as proc] + ["node:worker_threads" :as wt] + [app.common.logging :as l] + [app.config :as cf] + [app.consumer.config :as ccfg] + ;; Loaded for its init-key/halt-key methods, which is what makes the + ;; browser pool part of the system below. + [exporter.browser] + [exporter.utils.system :as system])) + +(l/setup! {:exporter :info}) + +(def system-config + "The production wiring. Env-derived values are read here, at the + wiring layer; the components themselves take plain data." + {:exporter.browser/pool {:max (ccfg/concurrency)}}) + +;; The running system map, or nil when nothing is started. +;; Counterpart of the backend's `app.system/system` var. +(defonce system + (atom nil)) + +;; The config the running system booted with. `restart` reboots it, +;; so tests can cycle a fake config without touching production. +(defonce ^:private config-used + (atom nil)) + +(defn ^:async start-custom + "Boot the given config, halting the running system first, like the + backend's `start-custom`. Resolves to `:started`." + [config] + (when-let [sys @system] + (await (system/halt sys))) + (swap! config-used (constantly config)) + (reset! system nil) + (let [sys (await (system/init config))] + (reset! system sys) + :started)) + +;; Install-once guard: `start` runs on every hot reload, and piling +;; process listeners would end in MaxListenersExceeded warnings. +(defonce ^:private handlers-installed? (atom false)) + +(defn install-process-handlers + "Register the process-level handlers: log uncaught exceptions instead + of dying silently, and exit on SIGTERM/SIGINT. Signals only reach the + main thread; exiting in a render worker would take down that worker + rather than the process. Runs on `start`, never on namespace load, + and only the first call takes effect." + [] + (when (compare-and-set! handlers-installed? false true) + (.on proc/default "uncaughtException" + (fn [cause] + (js/console.error cause))) + (when ^boolean wt/isMainThread + (.on proc/default "SIGTERM" (fn [] (proc/exit 0))) + (.on proc/default "SIGINT" (fn [] (proc/exit 0)))))) + +(defn ^:async start + "Boot the production wiring. Resolves to `:started`." + [] + (install-process-handlers) + (l/info :msg "starting exporter" + :workers (ccfg/concurrency) + :version (:full cf/version)) + (await (start-custom system-config))) + +(defn ^:async stop + "Halt the running system, if any. Resolves to `:stopped`." + [] + (when-let [sys @system] + (await (system/halt sys))) + (reset! system nil) + :stopped) + +(defn ^:async restart + "Halt and reboot whatever config is running (or the production + wiring when nothing ever started). Resolves to `:restarted`." + [] + (await (stop)) + (await (start-custom (or @config-used system-config))) + :restarted) + +(defn ^:dev/before-load-async on-before-load + "Hot reload hook: halt the running system before shadow swaps the + code. `done` always runs, so a halt failure is logged, never a + stuck reload." + [done] + (-> (stop) + (.then (fn [_] (done))) + (.catch (fn [cause] + (l/warn :hint "hot reload stop failed" + :cause (ex-message cause)) + (done))))) + +(defn ^:dev/after-load-async on-after-load + "Hot reload hook: boot the production wiring after shadow swaps the + code. `done` always runs, so a boot failure is logged, never a + stuck reload." + [done] + (-> (start) + (.then (fn [_] (done))) + (.catch (fn [cause] + (l/warn :hint "hot reload start failed" + :cause (ex-message cause)) + (done))))) + +(def main start) diff --git a/exporter/test/exporter_tests/browser_test.cljs b/exporter/test/exporter_tests/browser_test.cljs index e286c4e192..5da5998f6b 100644 --- a/exporter/test/exporter_tests/browser_test.cljs +++ b/exporter/test/exporter_tests/browser_test.cljs @@ -41,8 +41,8 @@ (defn- test-config [calls] - {:exporter.browser/pool {:exporter.browser/max 1 - :exporter.browser/create-browser (fake-create-browser calls)}}) + {:exporter.browser/pool {:max 1 + :create-browser (fake-create-browser calls)}}) (t/deftest ^:async pool-starts-and-stops-through-the-system (try diff --git a/exporter/test/exporter_tests/main_test.cljs b/exporter/test/exporter_tests/main_test.cljs index 1bf136bf2d..edba45701a 100644 --- a/exporter/test/exporter_tests/main_test.cljs +++ b/exporter/test/exporter_tests/main_test.cljs @@ -5,12 +5,119 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns exporter-tests.main-test - "The new `exporter.*` tree loads: requiring `exporter.main` resolves - its namespace." + "The entry point of the new tree: the namespace loads, and + start/stop/restart manage the running system." (:require + ["node:process" :as proc] + [app.common.logging :as l] [cljs.test :as t :include-macros true] - [exporter.main])) + [exporter.browser :as browser] + [exporter.main :as main])) (t/deftest new-tree-loads (t/testing "exporter.main resolves once required" (t/is (some? (find-ns 'exporter.main))))) + +(t/deftest entry-logger-allows-info + (t/testing "the entry point leaves info logging enabled for the new tree" + (t/is (true? (l/enabled? "exporter.main" :info))))) + +(defn- stub-context + [calls] + #js {:newPage (fn [] (swap! calls conj :new-page) (js/Promise.resolve #js {})) + :close (fn [] (swap! calls conj :close-context) (js/Promise.resolve nil))}) + +(defn- stub-browser + [calls] + #js {:newContext (fn [_opts] + (swap! calls conj :new-context) + (js/Promise.resolve (stub-context calls))) + :isConnected (fn [] true) + :close (fn [] + (swap! calls conj :close-browser) + (js/Promise.resolve nil))}) + +(defn- fake-config + [calls] + {:exporter.browser/pool {:max 1 + :create-browser (fn [] + (swap! calls conj :create-browser) + (js/Promise.resolve (stub-browser calls)))}}) + +(defn- pool-of + [] + (:exporter.browser/pool @main/system)) + +(t/deftest ^:async install-process-handlers-registers-handlers + (try + (let [listeners (fn [event] (.listenerCount proc/default event)) + current (fn [] (mapv listeners ["uncaughtException" "SIGTERM" "SIGINT"])) + before (current)] + (main/install-process-handlers) + (t/is (= (mapv inc before) (current))) + (t/testing "a second install adds nothing: reloads must not pile listeners" + (main/install-process-handlers) + (t/is (= (mapv inc before) (current))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async hot-reload-stop-halts-through-done + (try + (let [calls (atom [])] + (await (main/start-custom (fake-config calls))) + (await (browser/exec (pool-of) {} (fn [_] :ok))) + (let [stopped (js/Promise. (fn [resolve _] (main/on-before-load (fn [] (resolve :done)))))] + (t/is (= :done (await stopped)))) + (t/is (nil? @main/system)) + (t/is (some #{:close-browser} @calls)) + (t/is (= :stopped (await (main/stop))))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async hot-reload-start-boots-production-wiring + (try + ;; No browser launches: the pool starts empty (min 0) and nothing + ;; checks one out, so this boots the real config with no Chromium. + (let [started (js/Promise. (fn [resolve _] (main/on-after-load (fn [] (resolve :done)))))] + (t/is (= :done (await started)))) + (t/is (some? (pool-of))) + (t/is (= :stopped (await (main/stop)))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async start-boots-and-stop-halts + (try + (let [calls (atom [])] + (t/is (= :started (await (main/start-custom (fake-config calls))))) + (t/is (some? (pool-of))) + (await (browser/exec (pool-of) {} (fn [_] :ok))) + (t/is (= :stopped (await (main/stop)))) + (t/is (nil? @main/system)) + (t/is (some #{:close-browser} @calls))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async restart-reboots-the-same-config + (try + (let [calls (atom [])] + (await (main/start-custom (fake-config calls))) + (await (browser/exec (pool-of) {} (fn [_] :ok))) + (t/is (= :restarted (await (main/restart)))) + (t/is (some #{:close-browser} @calls)) + (t/is (= :ok (await (browser/exec (pool-of) {} (fn [_] :ok))))) + (t/is (= 2 (count (filter #{:create-browser} @calls)))) + (await (main/stop))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause)))))) + +(t/deftest ^:async second-start-replaces-the-running-system + (try + (let [calls (atom [])] + (await (main/start-custom (fake-config calls))) + (await (browser/exec (pool-of) {} (fn [_] :ok))) + (t/is (= :started (await (main/start-custom (fake-config calls))))) + (t/is (some #{:close-browser} @calls)) + (t/is (= :ok (await (browser/exec (pool-of) {} (fn [_] :ok))))) + (await (main/stop))) + (catch :default cause + (t/is false (str "unexpected failure: " (ex-message cause))))))