diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index a8ae7deedc..390c40d53a 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -158,3 +158,5 @@ {"area": "cmd/mxcli", "date": "2026-10-03", "symptom": "`mxcli report` could not score a project's own modules: `lint` has --modules, `report` had only --exclude", "cause": "Feature gap; and the LintContext module filter alone would not make the score exact, because project-level findings (CONV008 role mappings, project security) carry no module and are reported regardless", "file": "`cmd/mxcli/cmd_report.go`, `mdl/linter/report.go` (`ScopeToModules`, Report.Modules)", "insight": "Filter the scored violations to those located in a selected module, and print the selection in every format so a module score is not mistaken for the project's", "refs": ["ako/mxcli#953"]} {"area": "cmd/mxcli/docker", "date": "2026-10-03", "symptom": "`mxcli docker build` (and docker run / reload, which call it) rewrote an MPRv1 project's .mpr, rewrote every MPRv2 .mxunit (restored with new mtimes), and wrote theme-cache/, deployment/, javasource/ proxies, the .launch file, .classpath and .project into the project; the TUI checker and `mxcli eval`'s mx_check wrote theme-cache/web/ and deployment/sass/ on every run", "cause": "update-widgets ran on the project under a snapshot that restored only v2 storage, and mx check / MxBuild ran on the project itself; the TUI and eval runner ran a bare `mx check `", "file": "`cmd/mxcli/docker/build.go` (`buildOnCopy`), `cmd/mxcli/docker/check.go` (`MxCheckOnCopy`), `cmd/mxcli/tui/checker.go` (`runMxCheck`), `cmd/mxcli/evalrunner/checks.go` (`checkMxCheck`)", "insight": "Run update-widgets, mx check and MxBuild on one temporary copy (copyProjectToTemp) and write only the output directory. Measured on the 11.14 testapp: the PAD from the copy differs from the in-place build in the same 9 files as two in-place builds of identical copies differ (cache-bust stamps, operation ids, the native metro paths), so the output is equivalent; rebuild time unchanged (58s vs 59s), because a PAD build gains nothing from the project's deployment/. 11.14's blank app needs JDK 25, so the real-mx build test skips without it; run it with 11.13's mx.", "refs": ["ako/mxcli#961", "ako/mxcli#951", "mendixlabs/mxcli#808"]} {"area": "cmd/mxcli", "date": "2026-10-03", "symptom": "ako/mxcli#962 item 3: in VS Code, two calls to a stored void JavaScript action with the same output name (Studio Pro's `$ReturnValueName`/`$RefreshEntity` shape) are squiggled MDL063, while `mxcli check -p` passes them", "cause": "runSemanticValidation called executor.ValidateMicroflow/ValidateNanoflow, which pass a nil void-action resolver: without the project every call output counts as a declaration", "file": "`cmd/mxcli/lsp_diagnostics.go` (runSemanticValidation); `mdl/executor/validate_void_code_calls.go` (FlowRules, CodeActionCache)", "fix": "executor.NewFlowRules(prog, s.findMprPath(), s.codeActions): resolves actions through the script and the workspace project (opened lazily), treats an unresolvable action as possibly void (unknownIsVoid) for MDL063 but never for MDL093, and shares project answers across keystrokes for 30s since one action read costs ~300ms on PedApp", "insight": "An entry point that wraps a richer internal API with nil arguments (ValidateMicroflow = validateMicroflowWith(stmt, nil)) silently gives every caller the weakest behaviour; a fix landed in the richer API (#958) does not reach them. Grep the exported wrapper's callers when the internal one gains a parameter. Measure the cost before putting project I/O on a keystroke path", "test": "`cmd/mxcli/lsp_void_calls_test.go` (PedApp: void pair clean with and without project, Boolean pair control, MDL093 only with project); `mdl/executor/validate_void_code_calls_test.go` (TestCodeActionCache_SharesProjectAnswersAcrossRuns)"} +{"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch` (Mendix <= 11.13, rollup bundler): after a while every page change fails with `web client rebuild failed: web client watcher exited` (or `client bundle not served after apply: web client re-bundle: web client build timed out after 5m0s`) and nothing reaches the browser until `run --local` is restarted; after adding an entity the next changes fail with `ENOTDIR: not a directory, stat '.../web/pages/.js/package.json'`", "cause": "watchAndApply held a bare *WebClientWatcher: (1) once it exited nothing restarted it (only recoverMissingPages did), so WaitForRebuild failed every later change; (2) a failed incremental build (rollup's commonjs resolver hitting web/pages mid-rewrite by the serve build) left the watcher erroring and the change was dropped; (3) ensureClientServed's recovery ran a one-shot NODE_ENV=production BuildWebClient in the same web/ dir while the watcher was still running — two rollups on web/dist; (4) the 5m limit was hard-coded and a timeout printed nothing about why", "file": "cmd/mxcli/docker/webclient_supervisor.go, cmd/mxcli/docker/runlocal.go (watchAndApply, ensureClientServed), cmd/mxcli/docker/webclient.go (webClientTimeout, webClientBuildLogTail)", "fix": "bundlerSupervisor owns the bundler: EnsureAlive restarts an exited one (backoff 2s..60s after failed starts), AwaitRebuild retries a failed/aborted incremental rebuild once with a fresh bundler, Rebundle = stop+reap then start (never two). ensureClientServed takes the rebundle func; clientRebundler hands it the supervisor under --watch and the one-shot otherwise. --web-client-timeout / MXCLI_WEB_CLIENT_TIMEOUT; timeout errors append the last 30 lines of deployment/log/web-client-build.log. sessionNotice prints that a restart dropped sessions", "insight": "Killing the runner (`kill `) reproduces the dead-watcher state in seconds — no need to wait for it to die on its own. A fresh bundler is the universal recovery under --watch: its first build is a full bundle of the current source, so it covers missing pages, dangling chunks, a dist/ deleted by Gradle, and a transient incremental failure alike, without a second process on web/dist. Note: exec.Cmd.Wait called a second time concurrently with the reaper did block until exit on this Go version, so the old Stop was not the overlap — the one-shot in ensureClientServed was", "test": "cmd/mxcli/docker/webclient_supervisor_test.go (TestBundlerSupervisor_RestartsExitedBundler, _AwaitRebuildRecovers, _RebundleNeverOverlaps, TestEnsureClientServed_WatchModeRebundleIsExclusive, TestBuildWebClient_TimeoutShowsLogTail); live: 11.13 scratch app, 7 consecutive changes incl. a killed runner"} +{"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch`: an `mxcli exec` (or save) made while a change is still building/applying is never built — no `Change detected` follows, the app keeps the previous model, and re-running the script writes nothing (byte-idempotent) so nothing re-triggers", "cause": "watchAndApply set `last = sourceMTime(...)` after every successful apply, under a comment claiming it kept mid-build edits; it did the opposite — the edit's mtime was folded into the baseline, so the next tick saw nothing newer", "file": "cmd/mxcli/docker/runlocal.go (watchAndApply)", "fix": "keep `last` at the settled mtime the build was taken from; the build writes nothing under the watched model/theme source (checked with find -newer during a live run), so this cannot self-trigger", "insight": "Found while reproducing #971 with a script that waited for the first output line of a change instead of its `applied` line — the next exec landed during a 2-minute restart-apply and vanished. Any test of a watch loop should include an edit made DURING a build, not only between builds", "test": "live only (11.13 scratch app, hsqldb): exec an entity add, exec a page change 15s into its build; fixed binary builds it as the next build, the binary with the refresh restored shows no further build after 45s"} diff --git a/.claude/skills/mendix/run-local/SKILL.md b/.claude/skills/mendix/run-local/SKILL.md index 847c09fb89..64501fdec5 100644 --- a/.claude/skills/mendix/run-local/SKILL.md +++ b/.claude/skills/mendix/run-local/SKILL.md @@ -220,6 +220,7 @@ Launch `run --local` as the **sole** command in its invocation (don't chain a tr | `--hub-secret` | — | Shared auth (`user:pass`) matching an **open** hub's `--secret` | | *(hub API key)* | — | For an **authenticated** hub: get one from `https:///cli`, set `MXCLI_HUB_KEY` (see below) | | `--watch` | off | Rebuild + hot-apply on each change | +| `--web-client-timeout` | `$MXCLI_WEB_CLIENT_TIMEOUT`, else `5m` | Limit for one web client bundle build; on timeout the tail of `deployment/log/web-client-build.log` is printed | | `--ensure-db` | off | Provision local Postgres + app database if missing | | `--setup` | off | Cache MxBuild+runtime + ensure DB, then exit (SessionStart bring-up) | | `--screenshot` | off | Playwright PNG after boot + each change | @@ -301,6 +302,15 @@ Playwright + the devcontainer's Chromium). `mxbuild --serve`): a page/widget edit re-bundles in ~3–4 s; a microflow/entity edit skips the bundle and just hot-reloads. It uses `CHOKIDAR_USEPOLLING` because inotify is silent on container filesystems. + The bundler is supervised: one that **exits** is restarted on the next change + (`web client bundler exited unexpectedly; restarting it...`, with backoff when it + cannot start), and an incremental rebuild that **fails** is retried once with a + fresh bundler before the change is reported as failed. A recovery re-bundle + (missing page, dangling chunk, `/dist/index.js` gone after a restart) replaces the + bundler — two rollups never write `web/dist` at once. +- **A structural change restarts the runtime and drops every browser session.** The + loop prints `runtime restarted for this change — browser sessions were dropped; + log in again`; a browser test that sees it must log in before the next step. - Without `--watch`, a single one-shot bundle (~7 s) runs before boot. - **The bundle is re-checked after the boot**, because bundling before it is not enough: the runtime's boot runs Gradle `clean-custom-classes compile package`, diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a83d6a4e7..faff9c5dcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`run --local --watch` no longer loses an edit made while a change is being applied** — after each apply the loop moved its change baseline to the source's current mtime, so an `exec` or save that landed during the build (easily, while a structural change restarts the runtime) was never rebuilt: the app kept serving the previous model with nothing reported. The baseline now stays at the mtime the build was taken from, and the edit is built on the next poll. +- **`run --local --watch` keeps its web client bundler alive and never runs two** (ako/mxcli#971) — a bundler that exited stayed dead, so every later page change failed with `web client watcher exited` and only restarting `run --local` recovered (reproduced on Mendix 11.13 by killing the runner: two changes in a row failed). It is now restarted on the next change, with backoff when it cannot start. An incremental rebuild that fails — measured after adding an entity: `ENOTDIR … web/pages/.js/package.json`, and the next change failed the same way — is retried once with a fresh bundler. A recovery re-bundle (a missing page, a dangling chunk, or `/dist/index.js` gone after a runtime restart) replaces the bundler instead of running a one-shot production bundle beside it in the same `web/` directory. The bundle-build limit is configurable (`--web-client-timeout`, or `MXCLI_WEB_CLIENT_TIMEOUT`; default 5m), and a timeout prints the tail of `deployment/log/web-client-build.log`. A change that restarts the runtime now says that browser sessions were dropped. - **`list workflows` and `show structure` count every activity of a workflow** (ako/mxcli#963) — including those on a boundary-event path and in an event sub-process, as the catalog's `workflows_data` has since #937. TestApp `Workflow1` listed 5 activities where the catalog counted 8; all three now share one walk (`wfnames.WalkActivities`). - **`docker build`, the TUI checker and `mxcli eval` no longer modify the project** (ako/mxcli#961) — `docker build` (and `docker run` / `docker reload`, which use it) now runs `mx update-widgets`, `mx check` and MxBuild on one temporary copy and writes only its output directory (`.docker/build/` or `-o`). Before, it rewrote an MPR v1 project's `.mpr`, rewrote every MPR v2 `.mxunit` (restored with new mtimes), and wrote `theme-cache/`, `deployment/`, `javasource/` proxies, the `.launch` file, `.classpath` and `.project` into the project. The build still uses the widget-normalised model, from the copy, and the package it produces is the same; `mxcli fix widgets` applies the normalisation to the project. **The project's `deployment/` is no longer refreshed by `docker build`** — `run --local` builds its own. The TUI auto-check and eval's `mx_check` ran a plain `mx check` on the project, which wrote `theme-cache/web/` and `deployment/sass/` every time; they now check a copy too. - **MPR010 no longer tells a native page to use a layout grid** (ako/mxcli#962) — the "wrap the form in a layoutgrid" advice is about Bootstrap columns, which a native page does not have: a form DataView directly on an `Atlas_Core.NativePhone_Default` page builds clean in mxbuild 11.13.0, and following the advice there is CE6858 ("update Atlas UI … to use Layout Grid on Native pages"). `lint` skips pages on a native layout and snippets of type Native (MPR010 now needs the full catalog, which a default `lint` builds anyway); `check -p` skips pages whose layout the project says is native. diff --git a/cmd/mxcli/cmd_run.go b/cmd/mxcli/cmd_run.go index aec8758042..cd9e63be92 100644 --- a/cmd/mxcli/cmd_run.go +++ b/cmd/mxcli/cmd_run.go @@ -143,6 +143,7 @@ Examples: } watch, _ := cmd.Flags().GetBool("watch") + webClientTimeout, _ := cmd.Flags().GetDuration("web-client-timeout") testEndpoint, _ := cmd.Flags().GetBool("test-endpoint") ensureDB, _ := cmd.Flags().GetBool("ensure-db") setupOnly, _ := cmd.Flags().GetBool("setup") @@ -208,6 +209,7 @@ Examples: ServePort: servePort, MxBuildPath: mxbuildPath, Watch: watch, + WebClientTimeout: webClientTimeout, EnsureDB: ensureDB, SetupOnly: setupOnly, Screenshot: screenshot, @@ -313,6 +315,7 @@ func init() { runCmd.Flags().String("hub-worktree", "", "Worktree label to distinguish multiple worktrees of one branch") runCmd.Flags().String("hub-session", "", "Session id to group this preview under in the hub overview (default: CLAUDE_CODE_REMOTE_SESSION_ID / MXCLI_HUB_SESSION)") runCmd.Flags().Bool("watch", false, "Rebuild and hot-apply on every project change") + runCmd.Flags().Duration("web-client-timeout", 0, "Limit for one web client bundle build, e.g. 10m (default $MXCLI_WEB_CLIENT_TIMEOUT, else 5m); on timeout the tail of deployment/log/web-client-build.log is printed") runCmd.Flags().Bool("test-endpoint", false, "Host mxcli's token-guarded test endpoint so 'mxcli test --attach' can run tests against this app without booting its own runtime (removed on exit)") runCmd.Flags().Bool("ensure-db", false, "Provision the local Postgres + app database if missing (fresh-session bootstrap)") runCmd.Flags().Bool("setup", false, "Prepare prerequisites (cache MxBuild+runtime, ensure DB) and exit without booting — for a SessionStart hook") diff --git a/cmd/mxcli/docker/clientgate_test.go b/cmd/mxcli/docker/clientgate_test.go index 75a570f280..42b7398d78 100644 --- a/cmd/mxcli/docker/clientgate_test.go +++ b/cmd/mxcli/docker/clientgate_test.go @@ -73,7 +73,7 @@ func TestEnsureClientServed_NoRecoveryWhenServed(t *testing.T) { })) defer srv.Close() - if err := ensureClientServed(dir, srv.URL+"/", "/nonexistent/mxbuild", io.Discard); err != nil { + if err := ensureClientServed(dir, srv.URL+"/", bogusOneShot(dir), io.Discard); err != nil { t.Fatalf("a present+served bundle should need no recovery, got: %v", err) } } @@ -93,7 +93,7 @@ func TestEnsureClientServed_RecoversWhenNotServed(t *testing.T) { })) defer srv.Close() - err := ensureClientServed(dir, srv.URL+"/", "/nonexistent/mxbuild", io.Discard) + err := ensureClientServed(dir, srv.URL+"/", bogusOneShot(dir), io.Discard) if err == nil { t.Fatal("expected recovery to run and fail (no rollup.config.mjs)") } diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index 143bfff469..4490daec43 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -99,6 +99,9 @@ type LocalRunOptions struct { SetupOnly bool // PollInterval is how often Watch checks for changes (default 1s). PollInterval time.Duration + // WebClientTimeout bounds one web client bundle build. Zero falls back to + // $MXCLI_WEB_CLIENT_TIMEOUT, then 5m (see webClientTimeout). + WebClientTimeout time.Duration // Screenshot, when set, captures a PNG of the app after boot and after each // applied change (requires the Playwright CLI + a browser). Screenshot bool @@ -685,22 +688,26 @@ func RunLocal(opts LocalRunOptions) error { // /dist/index.js and renders blank. In --watch we keep an incremental bundler // hot (warm rollup graph + polling file detection -> ~3-4s re-bundles); // otherwise a single one-shot build (~7s cold) suffices. - var watcher *WebClientWatcher + var bundler *bundlerSupervisor if opts.Watch { fmt.Fprintln(w, "Starting incremental web client bundler...") // A nil watcher is not a failure: on Mendix 11.14+ mxbuild's serve build - // writes web/dist itself, so there is no bundler to keep hot. Every - // watcher method is nil-safe, so the watch loop needs no branch. - watcher, err = StartWebClientWatch(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Stdout: w}) + // writes web/dist itself, so there is no bundler to keep hot. The + // supervisor is a no-op then, so the watch loop needs no branch. + watcher, err := StartWebClientWatch(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: w}) if err != nil { return fmt.Errorf("starting web client bundler: %w", err) } - // Stop whichever watcher is current at exit: watchAndApply replaces it - // when a newly added page needs a fresh bundler (see missingPageChunks). - defer func() { _ = watcher.Stop() }() + // From here on the supervisor owns the bundler: it restarts one that + // exits and replaces it for a recovery re-bundle (#971), so stop + // whichever one is current at exit. + bundler = newBundlerSupervisor(watcher, func() (*WebClientWatcher, error) { + return StartWebClientWatch(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: io.Discard}) + }, w) + defer bundler.Stop() } else { fmt.Fprintln(w, "Bundling web client...") - if err := BuildWebClient(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Stdout: w}); err != nil { + if err := BuildWebClient(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: w}); err != nil { return fmt.Errorf("bundling web client: %w", err) } } @@ -797,7 +804,7 @@ func RunLocal(opts LocalRunOptions) error { // wrote seconds ago. Verify after the boot, because before it proves nothing // (mxcli-formula1 §35). Costs a stat when the bundle survived. if _, err := EnsureWebClientBundle(WebClientOptions{ - DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Stdout: w, + DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: w, }); err != nil { // The app is up and its services answer; only the browser is broken. Say so // and keep running rather than tearing down a working runtime. @@ -927,7 +934,7 @@ func RunLocal(opts LocalRunOptions) error { // 7. Stay up until interrupted. With --watch, rebuild + hot-apply on every // project change; otherwise just keep the runtime serving. if opts.Watch { - return watchAndApply(opts, serve, rt, &watcher, mxbuildPath) + return watchAndApply(opts, serve, rt, bundler, mxbuildPath) } fmt.Fprintln(w, "(run with --watch to rebuild and hot-apply on changes; Ctrl-C to stop)") if waitForInterruptOrExit(rt.Exited()) { @@ -1170,7 +1177,11 @@ func clientBundleServedWithin(appURL string, window time.Duration) bool { // present+served, fall back to the reliable synchronous one-shot bundle (the same // path the non-watch boot uses) and re-probe. A no-op when the bundle is already // served (a pure model reload never touches web/dist). -func ensureClientServed(deployDir, appURL, mxbuildPath string, out io.Writer) error { +// +// rebundle performs the recovery bundle. Under --watch it must be the bundler +// supervisor's (see clientRebundler): a one-shot here ran a second rollup on +// web/dist alongside the incremental watcher (#971). +func ensureClientServed(deployDir, appURL string, rebundle func() error, out io.Writer) error { // Nothing below applies to the classic (Dojo) client: it has no bundle, no // chunks, and never serves /dist/index.js. Fixing only the boot path would // have moved this failure to every applied change under --watch rather than @@ -1185,7 +1196,7 @@ func ensureClientServed(deployDir, appURL, mxbuildPath string, out io.Writer) er if dangling := danglingClientChunks(deployDir); len(dangling) > 0 { fmt.Fprintf(out, " client bundle inconsistent — index.js imports %d chunk(s) that are not on disk (%s); re-bundling web client...\n", len(dangling), strings.Join(dangling, ", ")) - if err := BuildWebClient(WebClientOptions{DeployDir: deployDir, MxBuildPath: mxbuildPath, Stdout: out}); err != nil { + if err := rebundle(); err != nil { return fmt.Errorf("web client re-bundle: %w", err) } if still := danglingClientChunks(deployDir); len(still) > 0 { @@ -1196,16 +1207,14 @@ func ensureClientServed(deployDir, appURL, mxbuildPath string, out io.Writer) er // A page module with no bundled chunk is likewise invisible to the index.js // probe: pages are loaded by dynamic import. A one-shot bundle re-globs // web/pages and emits it. See missingPageChunks. - if _, err := recoverMissingPages(deployDir, func() error { - return BuildWebClient(WebClientOptions{DeployDir: deployDir, MxBuildPath: mxbuildPath, Stdout: out}) - }, out); err != nil { + if _, err := recoverMissingPages(deployDir, rebundle, out); err != nil { return fmt.Errorf("web client re-bundle: %w", err) } if clientBundlePresent(deployDir) && clientBundleServedWithin(appURL, clientProbeWindow) { return nil } fmt.Fprintln(out, " /dist/index.js not served after apply; re-bundling web client...") - if err := BuildWebClient(WebClientOptions{DeployDir: deployDir, MxBuildPath: mxbuildPath, Stdout: out}); err != nil { + if err := rebundle(); err != nil { return fmt.Errorf("web client re-bundle: %w", err) } if !clientBundleServedWithin(appURL, clientProbeWindow) { @@ -1263,19 +1272,13 @@ func settleSourceWith(projectPath string, seen time.Time, sigCh <-chan os.Signal // watchAndApply polls the project for changes and applies each rebuild until the // user interrupts (Ctrl-C). StartLocalRuntime already resolved the JVM; here we // only rebuild via serve and let the RuntimeController decide reload vs restart. -func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, watcherRef **WebClientWatcher, mxbuildPath string) error { +func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, bundler *bundlerSupervisor, mxbuildPath string) error { w := opts.Stdout - watcher := *watcherRef - // restartWatcher replaces the incremental bundler with a fresh one, whose - // first build re-globs web/pages. The caller's reference is updated so it - // stops the live watcher, not the one replaced here. - restartWatcher := func() error { - _ = watcher.Stop() - next, err := StartWebClientWatch(WebClientOptions{DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Stdout: io.Discard}) - watcher = next - *watcherRef = next - return err - } + // Every recovery re-bundle goes through the supervisor under --watch, so a + // one-shot never runs alongside the incremental bundler (#971). + rebundle := clientRebundler(bundler, WebClientOptions{ + DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: opts.Stdout, + }) sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) @@ -1328,10 +1331,20 @@ func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, w fmt.Fprintf(w, "Change detected, rebuilding (build #%d)...\n", gen) start := time.Now() + // A bundler that exited since the last change is restarted here + // rather than waited on: waiting on a dead one failed every later + // change with "watcher exited" until `run --local` was restarted + // (#971). Its first build bundles the current source, so whatever it + // missed while down is in it. + bundled, err := bundler.EnsureAlive() + if err != nil { + fmt.Fprintf(opts.Stderr, " %v\n", err) + } + // Capture the client-bundle generation and the web-source mtime before // the serve build, so we can tell whether the change plausibly touched // client source and, if so, wait for the incremental re-bundle. - genBefore := watcher.Generation() + genBefore := bundler.Generation() webBefore := webClientSourceMTime(opts.DeployDir) build, err := serve.Build(BuildRequest{Target: TargetDeploy, ProjectFilePath: opts.ProjectPath}) @@ -1360,24 +1373,24 @@ func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, w // no rebuild materializes (the touched file isn't a rollup input — e.g. a // microflow edit that rewrites a web metadata file but no page/widget), so // this never hangs. A pure model change skips the wait entirely. - bundled := false if webClientSourceMTime(opts.DeployDir).After(webBefore) { // Detection is a reliable ~1s with polling, so a 2.5s settle is ample // margin to catch a rebuild that's going to start, while keeping the // no-rebuild case (a model edit that only grazed web/) snappy. - bundled, err = watcher.WaitForRebuild(genBefore, 2500*time.Millisecond, 90*time.Second) + rebuilt, err := bundler.AwaitRebuild(genBefore, 2500*time.Millisecond, 90*time.Second, opts.Stderr) if err != nil { fmt.Fprintf(opts.Stderr, " web client rebuild failed: %v\n", err) continue } + bundled = bundled || rebuilt } // The incremental bundler never picks up a page added after it // started (mxbuild's pages plugin globs once), so a new page would // 404 in the browser while everything above reports success. A fresh // bundler re-globs. Only meaningful with a live watcher; without one, // ensureClientServed's one-shot covers the same check. - if watcher != nil { - restarted, err := recoverMissingPages(opts.DeployDir, restartWatcher, w) + if bundler.Wanted() { + restarted, err := recoverMissingPages(opts.DeployDir, bundler.Restart, w) if err != nil { fmt.Fprintf(opts.Stderr, " %v\n", err) continue @@ -1390,27 +1403,44 @@ func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, w fmt.Fprintf(opts.Stderr, " apply (%s) failed: %v\n", action, err) continue } + if note := sessionNotice(action); note != "" { + fmt.Fprintln(w, note) + } // Gate the "applied" report on the browser bundle actually being served: // a structural change can leave the restarted runtime 404ing // /dist/index.js (see ensureClientServed). This recovers before we tell // the user the build is live. - if err := ensureClientServed(opts.DeployDir, rt.AppURL(), mxbuildPath, opts.Stdout); err != nil { + if err := ensureClientServed(opts.DeployDir, rt.AppURL(), rebundle, opts.Stdout); err != nil { fmt.Fprintf(opts.Stderr, " client bundle not served after apply: %v\n", err) continue } client := "" if bundled { - client = fmt.Sprintf(", client re-bundled (gen %d)", watcher.Generation()) + client = fmt.Sprintf(", client re-bundled (gen %d)", bundler.Generation()) } fmt.Fprintf(w, " build #%d applied via %s in %s%s -> %s\n", gen, action, time.Since(start).Round(time.Millisecond), client, rt.AppURL()) maybeScreenshot(opts, rt) - // Refresh the baseline AFTER the apply so an edit made mid-build is - // still caught on the next tick. - last = sourceMTime(opts.ProjectPath) + // The baseline stays at the mtime this build settled on. Moving it to + // "now" here — as this loop used to, under a comment claiming the + // opposite — swallowed any edit made while the build was applying: the + // next tick saw nothing newer than the refreshed baseline, so the edit + // was never built (measured: an exec during a restart-apply on 11.13, + // no rebuild in 45s). The build writes nothing under the watched model + // and theme source, so keeping the settled baseline cannot loop. } } } +// sessionNotice is the line printed after an apply that restarted the runtime: +// a restart drops every browser session, which a reload does not, and a browser +// test that is not told so fails on a login page it did not expect (#971). +func sessionNotice(action ApplyAction) string { + if action != ActionRestart { + return "" + } + return " runtime restarted for this change — browser sessions were dropped; log in again" +} + // declaredJarDependencies collects the managed Java dependency coordinates the // model declares, across every module. func declaredJarDependencies(reader backend.FullBackend) []JarDependencyRef { diff --git a/cmd/mxcli/docker/webclient.go b/cmd/mxcli/docker/webclient.go index 4cf7f078f7..5bb7b2c18a 100644 --- a/cmd/mxcli/docker/webclient.go +++ b/cmd/mxcli/docker/webclient.go @@ -9,6 +9,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "time" ) @@ -32,7 +33,8 @@ type WebClientOptions struct { // MxBuildPath is /modeler/mxbuild; the node tooling is resolved from // its sibling tools/node directory. MxBuildPath string - // Timeout bounds the bundle build (default 5m). + // Timeout bounds the bundle build. Zero means webClientTimeout's default: + // $MXCLI_WEB_CLIENT_TIMEOUT when set, else 5m. Timeout time.Duration // Stdout receives a short progress line (default discarded). Stdout io.Writer @@ -123,10 +125,7 @@ func BuildWebClient(opts WebClientOptions) error { if err != nil { return err } - timeout := opts.Timeout - if timeout == 0 { - timeout = 5 * time.Minute - } + timeout := webClientTimeout(opts.Timeout) start := time.Now() cmd := exec.Command(nodeBin, runner) @@ -152,7 +151,10 @@ func BuildWebClient(opts WebClientOptions) error { case <-time.After(timeout): _ = cmd.Process.Kill() <-done - return fmt.Errorf("web client build timed out after %s", timeout) + // A bare "timed out" left the user nothing to act on (#971): say how to + // raise the limit and show what the bundler was doing when it was cut off. + return fmt.Errorf("web client build timed out after %s (raise it with --web-client-timeout or %s)%s", + timeout, webClientTimeoutEnv, webClientBuildLogTail(opts.DeployDir)) } if !WebClientBundled(opts.DeployDir) { @@ -163,6 +165,52 @@ func BuildWebClient(opts WebClientOptions) error { return nil } +// defaultWebClientTimeout bounds one bundle build when nothing overrides it. +const defaultWebClientTimeout = 5 * time.Minute + +// webClientTimeoutEnv overrides defaultWebClientTimeout (a Go duration, e.g. +// "10m"). The --web-client-timeout flag of `run --local` wins over it. +const webClientTimeoutEnv = "MXCLI_WEB_CLIENT_TIMEOUT" + +// webClientTimeout resolves the bundle-build limit: an explicit value first, +// then $MXCLI_WEB_CLIENT_TIMEOUT, then 5m. An unparsable or non-positive +// environment value is ignored rather than fatal — it bounds a build, and the +// default is a working bound. +func webClientTimeout(explicit time.Duration) time.Duration { + if explicit > 0 { + return explicit + } + if v := strings.TrimSpace(os.Getenv(webClientTimeoutEnv)); v != "" { + if d, err := time.ParseDuration(v); err == nil && d > 0 { + return d + } + } + return defaultWebClientTimeout +} + +// webClientLogTailLines is how much of web-client-build.log a timeout shows. +const webClientLogTailLines = 30 + +// webClientBuildLogTail returns the last lines of the bundler's own log +// (deployment/log/web-client-build.log), formatted to append to an error, or "" +// when there is no log. The runner writes its progress there, not to stdout, so +// it is where a stalled bundle shows which phase it stalled in. +func webClientBuildLogTail(deployDir string) string { + path := filepath.Join(deployDir, "log", "web-client-build.log") + data, err := os.ReadFile(path) + if err != nil { + return "" + } + lines := strings.Split(strings.TrimRight(string(data), "\n"), "\n") + if len(lines) == 1 && lines[0] == "" { + return "" + } + if len(lines) > webClientLogTailLines { + lines = lines[len(lines)-webClientLogTailLines:] + } + return fmt.Sprintf("\n last %d line(s) of %s:\n %s", len(lines), path, strings.Join(lines, "\n ")) +} + // webClientBundlePath is the one file whose absence is the black screen: the // shell loads, paints the theme's background, and never starts the client. func webClientBundlePath(deployDir string) string { diff --git a/cmd/mxcli/docker/webclient_classic_test.go b/cmd/mxcli/docker/webclient_classic_test.go index 18c1e94eec..4d5fdd9d70 100644 --- a/cmd/mxcli/docker/webclient_classic_test.go +++ b/cmd/mxcli/docker/webclient_classic_test.go @@ -167,7 +167,7 @@ func TestEnsureWebClientBundleSkipsClassic(t *testing.T) { // rather than removed it. appURL is deliberately unreachable: reaching the probe // at all is the defect. func TestEnsureClientServedSkipsClassic(t *testing.T) { - if err := ensureClientServed(classicDeployment(t), "http://127.0.0.1:1", "", io.Discard); err != nil { + if err := ensureClientServed(classicDeployment(t), "http://127.0.0.1:1", bogusOneShot(""), io.Discard); err != nil { t.Fatalf("classic deployment must not be probed for a bundle it has no concept of: %v", err) } } diff --git a/cmd/mxcli/docker/webclient_pages_test.go b/cmd/mxcli/docker/webclient_pages_test.go index bd580ca9e5..1746315a3c 100644 --- a/cmd/mxcli/docker/webclient_pages_test.go +++ b/cmd/mxcli/docker/webclient_pages_test.go @@ -163,7 +163,7 @@ func TestEnsureClientServed_RecoversWhenAPageIsMissing(t *testing.T) { })) defer srv.Close() - err := ensureClientServed(deploy, srv.URL+"/", "/nonexistent/mxbuild", io.Discard) + err := ensureClientServed(deploy, srv.URL+"/", bogusOneShot(deploy), io.Discard) if err == nil || !strings.Contains(err.Error(), "re-bundle") { t.Fatalf("expected the re-bundle branch to run for a missing page chunk, got: %v", err) } diff --git a/cmd/mxcli/docker/webclient_supervisor.go b/cmd/mxcli/docker/webclient_supervisor.go new file mode 100644 index 0000000000..d45fb9257a --- /dev/null +++ b/cmd/mxcli/docker/webclient_supervisor.go @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "fmt" + "io" + "strings" + "time" +) + +// webclient_supervisor.go owns the incremental web client bundler for the +// lifetime of a `run --local --watch` loop (#971). Two things went wrong while the +// loop held a bare *WebClientWatcher: +// +// - A bundler that exited stayed dead. Every later change waited on it, got +// "watcher exited", and was skipped — the edit never reached the browser and +// only restarting `run --local` recovered. +// - A recovery re-bundle (a dangling chunk, or /dist/index.js gone after a +// runtime restart) ran a one-shot production bundle in the same web/ +// directory while the watcher was still running, so two rollup processes +// wrote web/dist at once. +// +// The supervisor is the one place a bundler is started or stopped, so both are +// structural: Rebundle replaces the watcher (stop, then start — never both +// running), and EnsureAlive restarts one that has exited, with backoff. + +// clientBundler is the part of *WebClientWatcher the watch loop drives — a seam +// so the lifecycle is testable with fakes. +type clientBundler interface { + Generation() int + WaitForRebuild(sinceGen int, settle, buildTimeout time.Duration) (bool, error) + Exited() bool + Log() string + Stop() error +} + +// bundlerSupervisor keeps one incremental bundler alive. +type bundlerSupervisor struct { + // start launches a fresh bundler and blocks until its first bundle. A nil + // bundler with a nil error means the deployment needs none. + start func() (clientBundler, error) + out io.Writer + + cur clientBundler // the live bundler; nil when none is running + want bool // the deployment has an incremental bundler to keep alive + + failures int // consecutive failed starts + nextTry time.Time // no restart attempt before this (backoff) + now func() time.Time +} + +// newBundlerSupervisor wraps the bundler the boot started. initial may be nil +// (Mendix 11.14+ or the classic client): the supervisor then has nothing to keep +// alive and every method is a no-op. +func newBundlerSupervisor(initial *WebClientWatcher, start func() (*WebClientWatcher, error), out io.Writer) *bundlerSupervisor { + s := &bundlerSupervisor{ + start: func() (clientBundler, error) { + wc, err := start() + if wc == nil { + return nil, err // a typed nil must not become a non-nil interface + } + return wc, err + }, + out: out, + now: time.Now, + } + if initial != nil { + s.cur, s.want = initial, true + } + return s +} + +// Generation is the live bundler's success count, 0 when none is running. +func (s *bundlerSupervisor) Generation() int { + if s == nil || s.cur == nil { + return 0 + } + return s.cur.Generation() +} + +// WaitForRebuild waits on the live bundler; with none it has nothing to wait for. +func (s *bundlerSupervisor) WaitForRebuild(sinceGen int, settle, buildTimeout time.Duration) (bool, error) { + if s == nil || s.cur == nil { + return false, nil + } + return s.cur.WaitForRebuild(sinceGen, settle, buildTimeout) +} + +// Wanted reports whether this deployment has an incremental bundler at all. +func (s *bundlerSupervisor) Wanted() bool { return s != nil && s.want } + +// Restart replaces the bundler: the old one is stopped and reaped BEFORE the new +// one starts, so two bundlers never write web/dist at once. The new bundler's +// first build is a full bundle of the current source (re-globbing web/pages), so +// it doubles as the recovery re-bundle. Returns once that bundle has landed. +func (s *bundlerSupervisor) Restart() error { + if s == nil || !s.want { + return nil + } + if s.cur != nil { + _ = s.cur.Stop() + s.cur = nil + } + next, err := s.start() + if err != nil { + if next != nil { + _ = next.Stop() + } + s.failures++ + s.nextTry = s.now().Add(bundlerRestartBackoff(s.failures)) + return err + } + if next == nil { + // The deployment no longer has a bundler to keep (it changed shape, e.g. + // an upgrade to a Mendix that bundles itself). Nothing to supervise. + s.want = false + return nil + } + s.cur = next + s.failures = 0 + s.nextTry = time.Time{} + return nil +} + +// Rebundle is the recovery re-bundle under --watch: a fresh bundler, never a +// second one alongside the first. +func (s *bundlerSupervisor) Rebundle() error { return s.Restart() } + +// EnsureAlive restarts the bundler when it has exited (or a previous start +// failed). It reports whether it restarted one. Within the backoff window after +// a failed start it does not retry and returns an error saying when it will. +func (s *bundlerSupervisor) EnsureAlive() (bool, error) { + if s == nil || !s.want { + return false, nil + } + if s.cur != nil && !s.cur.Exited() { + return false, nil + } + if wait := s.nextTry.Sub(s.now()); wait > 0 { + return false, fmt.Errorf("web client bundler is down after %d failed restart(s); next attempt in %s", + s.failures, wait.Round(time.Second)) + } + if s.cur != nil { + fmt.Fprintf(s.out, " web client bundler exited unexpectedly; restarting it...%s\n", indentTail(s.cur.Log(), 10)) + } else { + fmt.Fprintf(s.out, " restarting web client bundler (attempt %d)...\n", s.failures+1) + } + if err := s.Restart(); err != nil { + return false, fmt.Errorf("restarting web client bundler (next attempt in %s): %w", + bundlerRestartBackoff(s.failures), err) + } + if s.want { + fmt.Fprintln(s.out, " web client bundler restarted") + } + return s.want, nil +} + +// AwaitRebuild waits for the incremental re-bundle a serve build triggered, and +// recovers when the bundler cannot deliver it. Two failures used to drop the +// change outright (#971): +// +// - the bundler exited (killed, crashed) — every later change then failed with +// "watcher exited" too; +// - the incremental build failed on a transient state of the source the serve +// build was rewriting, e.g. "ENOTDIR: stat …/web/pages/.js/package.json" +// after an entity was added — and the next change failed the same way. +// +// Either way a fresh bundler bundles the source as it now stands, so it is +// restarted once; only when that fails too is the error the caller's. errOut +// receives the incremental failure the recovery is answering. +func (s *bundlerSupervisor) AwaitRebuild(sinceGen int, settle, buildTimeout time.Duration, errOut io.Writer) (bool, error) { + rebuilt, err := s.WaitForRebuild(sinceGen, settle, buildTimeout) + if err == nil || !s.Wanted() { + return rebuilt, err + } + fmt.Fprintf(errOut, " incremental web client rebuild failed: %s\n", firstLine(err.Error())) + fmt.Fprintln(s.out, " re-bundling with a fresh web client bundler...") + if rerr := s.Restart(); rerr != nil { + return false, fmt.Errorf("fresh bundler failed too (next attempt in %s): %w", bundlerRestartBackoff(s.failures), rerr) + } + return s.want, nil +} + +// firstLine is s up to its first newline. +func firstLine(s string) string { + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} + +// Stop stops the live bundler, if any. +func (s *bundlerSupervisor) Stop() { + if s == nil || s.cur == nil { + return + } + _ = s.cur.Stop() + s.cur = nil +} + +// bundlerRestartBackoff is the wait after the n-th consecutive failed start: +// 2s, 4s, 8s ... capped at a minute, so a bundler that cannot start does not +// cost a full start attempt on every rebuild. +func bundlerRestartBackoff(failures int) time.Duration { + if failures <= 0 { + return 0 + } + d := 2 * time.Second + for i := 1; i < failures && d < time.Minute; i++ { + d *= 2 + } + if d > time.Minute { + d = time.Minute + } + return d +} + +// clientRebundler picks the recovery re-bundle for ensureClientServed. Under +// --watch with a live incremental bundler it is the supervisor's Rebundle — a +// one-shot BuildWebClient there would run a second rollup on web/dist alongside +// the watcher (#971). Otherwise it is the one-shot build. +func clientRebundler(sup *bundlerSupervisor, opts WebClientOptions) func() error { + if sup.Wanted() { + return sup.Rebundle + } + return func() error { return oneShotWebClientBuild(opts) } +} + +// oneShotWebClientBuild is BuildWebClient, as a var so a test can observe when a +// one-shot bundle runs. +var oneShotWebClientBuild = BuildWebClient + +// indentTail formats the last n lines of a log for appending to a message. +func indentTail(log string, n int) string { + log = strings.TrimRight(log, "\n") + if log == "" { + return "" + } + lines := strings.Split(log, "\n") + if len(lines) > n { + lines = lines[len(lines)-n:] + } + return "\n " + strings.Join(lines, "\n ") +} diff --git a/cmd/mxcli/docker/webclient_supervisor_test.go b/cmd/mxcli/docker/webclient_supervisor_test.go new file mode 100644 index 0000000000..128405680b --- /dev/null +++ b/cmd/mxcli/docker/webclient_supervisor_test.go @@ -0,0 +1,412 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "bytes" + "errors" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" +) + +// bogusOneShot is a one-shot re-bundle that fails visibly (no mxbuild there), so +// a test can tell that the recovery branch ran. +func bogusOneShot(deployDir string) func() error { + return func() error { + return BuildWebClient(WebClientOptions{DeployDir: deployDir, MxBuildPath: "/nonexistent/mxbuild"}) + } +} + +// bundlerPool tracks fake bundlers, so a test can assert how many ran at once. +type bundlerPool struct { + mu sync.Mutex + live int + maxLive int + started int + // onStart runs when a fake bundler starts — its "first build". + onStart func() +} + +func (p *bundlerPool) start() (clientBundler, error) { + p.mu.Lock() + p.live++ + p.started++ + if p.live > p.maxLive { + p.maxLive = p.live + } + p.mu.Unlock() + if p.onStart != nil { + p.onStart() + } + return &fakeBundler{pool: p, gen: 1}, nil +} + +func (p *bundlerPool) liveCount() int { + p.mu.Lock() + defer p.mu.Unlock() + return p.live +} + +type fakeBundler struct { + pool *bundlerPool + gen int + exited bool + stopped bool + waitErr error // WaitForRebuild's error while the process is alive +} + +func (f *fakeBundler) Generation() int { return f.gen } +func (f *fakeBundler) WaitForRebuild(int, time.Duration, time.Duration) (bool, error) { + if f.exited { + return false, errors.New("web client watcher exited:\nboom") + } + if f.waitErr != nil { + return false, f.waitErr + } + return true, nil +} +func (f *fakeBundler) Exited() bool { return f.exited } +func (f *fakeBundler) Log() string { return "runner log line\n" } + +// die simulates the bundler process exiting on its own. +func (f *fakeBundler) die() { + if !f.exited { + f.exited = true + f.pool.mu.Lock() + f.pool.live-- + f.pool.mu.Unlock() + } +} + +func (f *fakeBundler) Stop() error { + f.stopped = true + f.die() + return nil +} + +func newFakeSupervisor(pool *bundlerPool, out io.Writer) (*bundlerSupervisor, *fakeBundler) { + first, _ := pool.start() + return &bundlerSupervisor{start: pool.start, out: out, cur: first, want: true, now: time.Now}, first.(*fakeBundler) +} + +// The reported symptom: once the bundler had exited, every later change failed +// with "watcher exited" and nothing restarted it. EnsureAlive must start a fresh +// one, and say so. +func TestBundlerSupervisor_RestartsExitedBundler(t *testing.T) { + pool := &bundlerPool{} + var out bytes.Buffer + sup, first := newFakeSupervisor(pool, &out) + + if restarted, err := sup.EnsureAlive(); err != nil || restarted { + t.Fatalf("live bundler: EnsureAlive = (%v, %v), want (false, nil)", restarted, err) + } + first.die() + restarted, err := sup.EnsureAlive() + if err != nil || !restarted { + t.Fatalf("exited bundler: EnsureAlive = (%v, %v), want (true, nil)", restarted, err) + } + if pool.started != 2 || pool.liveCount() != 1 { + t.Fatalf("started=%d live=%d, want a second bundler and exactly one live", pool.started, pool.liveCount()) + } + if sup.cur == clientBundler(first) || sup.cur.Exited() { + t.Fatal("the supervisor still holds the dead bundler") + } + if !strings.Contains(out.String(), "exited unexpectedly") || !strings.Contains(out.String(), "runner log line") { + t.Fatalf("restart not reported with the dead bundler's log:\n%s", out.String()) + } +} + +// A bundler that cannot start must not be retried on every change: after a +// failure the next attempt waits out a growing backoff. +func TestBundlerSupervisor_BackoffAfterFailedStart(t *testing.T) { + now := time.Unix(1000, 0) + attempts := 0 + fail := true + pool := &bundlerPool{} + sup := &bundlerSupervisor{ + start: func() (clientBundler, error) { + attempts++ + if fail { + return nil, errors.New("node crashed") + } + return pool.start() + }, + out: io.Discard, want: true, now: func() time.Time { return now }, + cur: &fakeBundler{pool: pool, exited: true}, + } + + if _, err := sup.EnsureAlive(); err == nil || !strings.Contains(err.Error(), "node crashed") { + t.Fatalf("first restart: want the start error, got %v", err) + } + if _, err := sup.EnsureAlive(); err == nil || !strings.Contains(err.Error(), "next attempt in 2s") { + t.Fatalf("within backoff: want a 'next attempt' error, got %v", err) + } + if attempts != 1 { + t.Fatalf("restart retried inside the backoff window: %d attempts", attempts) + } + now = now.Add(3 * time.Second) + fail = false + if restarted, err := sup.EnsureAlive(); err != nil || !restarted { + t.Fatalf("after backoff: EnsureAlive = (%v, %v), want (true, nil)", restarted, err) + } + if attempts != 2 || sup.failures != 0 { + t.Fatalf("attempts=%d failures=%d, want 2 and a reset counter", attempts, sup.failures) + } +} + +func TestBundlerRestartBackoff(t *testing.T) { + want := map[int]time.Duration{0: 0, 1: 2 * time.Second, 2: 4 * time.Second, 3: 8 * time.Second, 10: time.Minute} + for n, d := range want { + if got := bundlerRestartBackoff(n); got != d { + t.Errorf("bundlerRestartBackoff(%d) = %s, want %s", n, got, d) + } + } +} + +// A recovery re-bundle under --watch must replace the bundler, never run beside it. +func TestBundlerSupervisor_RebundleNeverOverlaps(t *testing.T) { + pool := &bundlerPool{} + sup, _ := newFakeSupervisor(pool, io.Discard) + for i := 0; i < 3; i++ { + if err := sup.Rebundle(); err != nil { + t.Fatal(err) + } + } + if pool.maxLive != 1 { + t.Fatalf("%d bundlers ran at once, want 1", pool.maxLive) + } +} + +// The concurrency defect end to end: a page missing from the bundle under +// --watch must be recovered through the supervisor. A one-shot BuildWebClient +// while the incremental bundler is live puts two rollups on web/dist (#971). +func TestEnsureClientServed_WatchModeRebundleIsExclusive(t *testing.T) { + deploy := pagesDeployment(t, []string{"A.js", "B.js"}, []string{"A.js"}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + pool := &bundlerPool{} + sup, _ := newFakeSupervisor(pool, io.Discard) + // A fresh bundler's first build emits every page. + pool.onStart = func() { + _ = os.WriteFile(filepath.Join(deploy, "web", "dist", "pages", "B.js"), []byte("//"), 0o600) + } + + oneShotWhileLive := 0 + oneShots := 0 + old := oneShotWebClientBuild + oneShotWebClientBuild = func(WebClientOptions) error { + oneShots++ + if pool.liveCount() > 0 { + oneShotWhileLive++ + } + pool.onStart() + return nil + } + defer func() { oneShotWebClientBuild = old }() + + rebundle := clientRebundler(sup, WebClientOptions{DeployDir: deploy}) + if err := ensureClientServed(deploy, srv.URL+"/", rebundle, io.Discard); err != nil { + t.Fatalf("ensureClientServed: %v", err) + } + if oneShotWhileLive > 0 { + t.Fatalf("a one-shot bundle ran %d time(s) while the incremental bundler was live", oneShotWhileLive) + } + if pool.maxLive != 1 || pool.started != 2 { + t.Fatalf("maxLive=%d started=%d, want the bundler replaced (2 starts, 1 at a time)", pool.maxLive, pool.started) + } + + // Control: without an incremental bundler the one-shot is the right tool. + deploy2 := pagesDeployment(t, []string{"A.js", "B.js"}, []string{"A.js"}) + pool.onStart = func() { + _ = os.WriteFile(filepath.Join(deploy2, "web", "dist", "pages", "B.js"), []byte("//"), 0o600) + } + none := newBundlerSupervisor(nil, nil, io.Discard) + if err := ensureClientServed(deploy2, srv.URL+"/", clientRebundler(none, WebClientOptions{DeployDir: deploy2}), io.Discard); err != nil { + t.Fatalf("ensureClientServed without a bundler: %v", err) + } + if oneShots != 1 { + t.Fatalf("without a bundler the one-shot should run once, ran %d", oneShots) + } +} + +// A nil supervisor bundler (Mendix 11.14+, classic client) is inert. +func TestBundlerSupervisor_NoBundlerIsNoop(t *testing.T) { + sup := newBundlerSupervisor(nil, func() (*WebClientWatcher, error) { + t.Fatal("start must not be called") + return nil, nil + }, io.Discard) + if sup.Wanted() || sup.Generation() != 0 { + t.Fatal("a supervisor without a bundler reports one") + } + if r, err := sup.EnsureAlive(); r || err != nil { + t.Fatalf("EnsureAlive = (%v, %v)", r, err) + } + if err := sup.Restart(); err != nil { + t.Fatal(err) + } + sup.Stop() +} + +// Stop must not return while the bundler process is still running: the +// supervisor's restart starts the next bundler as soon as Stop returns. A guard, +// not a regression test — the earlier double-Wait Stop also blocked until exit +// on this Go version, so this passes against it too. +func TestWebClientWatcher_StopWaitsForExit(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("process-group signalling is unix-only") + } + // Takes ~1s to exit on SIGTERM, the way node finishes a bundle in flight. + cmd := exec.Command("sh", "-c", `trap 'sleep 1; exit 0' TERM; echo ready; while :; do sleep 0.05; done`) + setProcessGroup(cmd) + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + wc, err := launchWatcher(cmd, &syncBuffer{}, stdout) + if err != nil { + t.Fatal(err) + } + time.Sleep(200 * time.Millisecond) // let the shell install its trap + if wc.Exited() { + t.Fatal("process exited before Stop") + } + _ = wc.Stop() + if !wc.Exited() { + t.Fatal("Stop returned while the bundler process was still running") + } +} + +func TestWebClientTimeout(t *testing.T) { + t.Setenv(webClientTimeoutEnv, "") + if got := webClientTimeout(0); got != defaultWebClientTimeout { + t.Errorf("default = %s", got) + } + t.Setenv(webClientTimeoutEnv, "12m") + if got := webClientTimeout(0); got != 12*time.Minute { + t.Errorf("env = %s, want 12m", got) + } + if got := webClientTimeout(3 * time.Second); got != 3*time.Second { + t.Errorf("explicit = %s, want it to win over the env", got) + } + t.Setenv(webClientTimeoutEnv, "soon") + if got := webClientTimeout(0); got != defaultWebClientTimeout { + t.Errorf("unparsable env = %s, want the default", got) + } +} + +// A timed-out bundle used to say only "timed out after 5m0s". It must name the +// knob and show the bundler's own log, which is where the stall is visible. +func TestBuildWebClient_TimeoutShowsLogTail(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake node is a shell script") + } + deploy := t.TempDir() + mustWrite := func(p, content string, mode os.FileMode) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), mode); err != nil { + t.Fatal(err) + } + } + mustWrite(filepath.Join(deploy, "web", "rollup.config.mjs"), "export default {};", 0o644) + var log strings.Builder + for i := 1; i <= 40; i++ { + log.WriteString("line ") + log.WriteString(strings.Repeat("x", i%3)) + log.WriteString("\n") + } + log.WriteString("INFO Bundling started\n") + mustWrite(filepath.Join(deploy, "log", "web-client-build.log"), log.String(), 0o644) + + mx := t.TempDir() + mustWrite(filepath.Join(mx, "tools", "node", "rollup-runner.mjs"), "", 0o644) + mustWrite(filepath.Join(mx, "tools", "node", "node"), "#!/bin/sh\nexec sleep 30\n", 0o755) + + err := BuildWebClient(WebClientOptions{ + DeployDir: deploy, MxBuildPath: filepath.Join(mx, "mxbuild"), Timeout: 300 * time.Millisecond, + }) + if err == nil { + t.Fatal("expected a timeout") + } + msg := err.Error() + for _, want := range []string{"timed out after 300ms", "--web-client-timeout", webClientTimeoutEnv, "INFO Bundling started", "last 30 line(s)"} { + if !strings.Contains(msg, want) { + t.Errorf("timeout error lacks %q:\n%s", want, msg) + } + } +} + +func TestSessionNotice(t *testing.T) { + if sessionNotice(ActionReload) != "" { + t.Error("a reload keeps sessions; no notice") + } + if n := sessionNotice(ActionRestart); !strings.Contains(n, "sessions were dropped") { + t.Errorf("restart notice = %q", n) + } +} + +// Both shapes the reported loop died of: an incremental build that failed on the +// source the serve build was rewriting (measured on 11.13 after adding an entity: +// ENOTDIR on web/pages/.js/package.json, then the same on the next change), +// and a bundler that had exited. A fresh bundler must deliver the change. +func TestBundlerSupervisor_AwaitRebuildRecovers(t *testing.T) { + cases := map[string]func(*fakeBundler){ + "failed incremental build": func(f *fakeBundler) { + f.waitErr = errors.New("web client build failed: (plugin commonjs--resolver) Error: ENOTDIR: not a directory\nstack") + }, + "exited bundler": func(f *fakeBundler) { f.die() }, + } + for name, breakIt := range cases { + t.Run(name, func(t *testing.T) { + pool := &bundlerPool{} + sup, first := newFakeSupervisor(pool, io.Discard) + breakIt(first) + var errOut bytes.Buffer + rebuilt, err := sup.AwaitRebuild(0, time.Millisecond, time.Millisecond, &errOut) + if err != nil || !rebuilt { + t.Fatalf("AwaitRebuild = (%v, %v), want the change delivered by a fresh bundler", rebuilt, err) + } + if pool.started != 2 || pool.maxLive != 1 { + t.Fatalf("started=%d maxLive=%d, want one replacement and never two at once", pool.started, pool.maxLive) + } + if !strings.Contains(errOut.String(), "incremental web client rebuild failed") || strings.Contains(errOut.String(), "stack") { + t.Fatalf("the incremental failure is not reported on one line:\n%s", errOut.String()) + } + }) + } +} + +func TestBundlerSupervisor_AwaitRebuildFreshFailsToo(t *testing.T) { + pool := &bundlerPool{} + sup, first := newFakeSupervisor(pool, io.Discard) + first.waitErr = errors.New("web client build failed: syntax error") + sup.start = func() (clientBundler, error) { return nil, errors.New("initial web client build failed: syntax error") } + if _, err := sup.AwaitRebuild(0, time.Millisecond, time.Millisecond, io.Discard); err == nil || !strings.Contains(err.Error(), "fresh bundler failed too") { + t.Fatalf("a genuine build error must surface, got %v", err) + } +} + +func TestBundlerSupervisor_AwaitRebuildHealthyNoRestart(t *testing.T) { + pool := &bundlerPool{} + sup, _ := newFakeSupervisor(pool, io.Discard) + if rebuilt, err := sup.AwaitRebuild(0, time.Millisecond, time.Millisecond, io.Discard); err != nil || !rebuilt { + t.Fatalf("AwaitRebuild = (%v, %v)", rebuilt, err) + } + if pool.started != 1 { + t.Fatalf("a healthy rebuild restarted the bundler (%d starts)", pool.started) + } +} diff --git a/cmd/mxcli/docker/webclient_watch.go b/cmd/mxcli/docker/webclient_watch.go index 163bd6450a..edf791fbe1 100644 --- a/cmd/mxcli/docker/webclient_watch.go +++ b/cmd/mxcli/docker/webclient_watch.go @@ -80,6 +80,12 @@ type WebClientWatcher struct { lastErr string // message of the most recent failed bundle exited bool // the runner process has exited updated chan struct{} // closed+replaced on every state change (broadcast) + + // done is closed once the process has been reaped. Stop waits on it rather + // than calling cmd.Wait a second time beside the reaper goroutine; a restart + // relies on Stop returning only after the old bundler is gone, so two never + // write web/dist at once (#971). + done chan struct{} } // applyStatus folds a status line into the watcher state and broadcasts. @@ -182,7 +188,29 @@ func StartWebClientWatch(opts WebClientOptions) (*WebClientWatcher, error) { } cmd.Stderr = log - wc := &WebClientWatcher{cmd: cmd, log: log, updated: make(chan struct{})} + wc, err := launchWatcher(cmd, log, stdout) + if err != nil { + return nil, err + } + + timeout := webClientTimeout(opts.Timeout) + if err := wc.waitForFirstBuild(timeout); err != nil { + _ = wc.Stop() + return nil, fmt.Errorf("%w%s", err, webClientBuildLogTail(opts.DeployDir)) + } + dist := filepath.Join(webDir, "dist", "index.js") + if _, err := os.Stat(dist); err != nil { + _ = wc.Stop() + return nil, fmt.Errorf("web client watcher reported success but %s is missing:\n%s", dist, wc.log.String()) + } + return wc, nil +} + +// launchWatcher starts cmd and wires the status reader and the reaper. Split from +// StartWebClientWatch so the process lifecycle (Stop, Exited) is testable with +// an ordinary shell process instead of mxbuild's node tooling. +func launchWatcher(cmd *exec.Cmd, log *syncBuffer, stdout io.Reader) (*WebClientWatcher, error) { + wc := &WebClientWatcher{cmd: cmd, log: log, updated: make(chan struct{}), done: make(chan struct{})} if err := cmd.Start(); err != nil { return nil, fmt.Errorf("launching web client watcher: %w", err) } @@ -193,22 +221,20 @@ func StartWebClientWatch(opts WebClientOptions) (*WebClientWatcher, error) { wc.exited = true wc.broadcastLocked() wc.mu.Unlock() + close(wc.done) }() + return wc, nil +} - timeout := opts.Timeout - if timeout == 0 { - timeout = 5 * time.Minute - } - if err := wc.waitForFirstBuild(timeout); err != nil { - _ = wc.Stop() - return nil, err - } - dist := filepath.Join(webDir, "dist", "index.js") - if _, err := os.Stat(dist); err != nil { - _ = wc.Stop() - return nil, fmt.Errorf("web client watcher reported success but %s is missing:\n%s", dist, wc.log.String()) +// Exited reports whether the bundler process has exited. A nil watcher (no +// incremental bundler for this deployment) has nothing to exit. +func (wc *WebClientWatcher) Exited() bool { + if wc == nil { + return false } - return wc, nil + wc.mu.Lock() + defer wc.mu.Unlock() + return wc.exited } // readLoop parses the runner's stdout and folds each status into state. It also @@ -306,19 +332,27 @@ func (wc *WebClientWatcher) Log() string { return wc.log.String() } -// Stop terminates the watcher process. +// Stop terminates the watcher process and returns only once it has been reaped, +// so a caller that starts another bundler next never runs two at once. func (wc *WebClientWatcher) Stop() error { - if wc == nil || wc.cmd == nil || wc.cmd.Process == nil { + if wc == nil || wc.cmd == nil || wc.cmd.Process == nil || wc.done == nil { return nil } + select { + case <-wc.done: + return nil // already gone + default: + } _ = signalProcessGroup(wc.cmd.Process, syscall.SIGTERM) - done := make(chan error, 1) - go func() { done <- wc.cmd.Wait() }() select { - case <-done: - case <-time.After(5 * time.Second): + case <-wc.done: + case <-time.After(watcherStopGrace): _ = killProcessGroup(wc.cmd.Process) - <-done + <-wc.done } return nil } + +// watcherStopGrace is how long Stop lets the bundler exit on SIGTERM before it +// kills the process group. A var so a test can shorten it. +var watcherStopGrace = 5 * time.Second diff --git a/docs-site/src/tools/run-local.md b/docs-site/src/tools/run-local.md index 348a028e4c..8f29baa5fc 100644 --- a/docs-site/src/tools/run-local.md +++ b/docs-site/src/tools/run-local.md @@ -25,7 +25,9 @@ the container on every change (~30–60 s). `run --local` instead: | entity / view entity / association | runtime restart + DDL | ~9 s | The metamodel catalog (entities/associations) is reconciled only at runtime startup, -so structural changes need a restart; behavioural changes do not. +so structural changes need a restart; behavioural changes do not. A restart drops +every browser session — `--watch` prints `browser sessions were dropped; log in +again` when it happens, so a browser test knows to sign in before its next step. ## What it does @@ -72,6 +74,7 @@ so structural changes need a restart; behavioural changes do not. | `--hub-secret` | — | Shared auth (`user:pass`) matching an **open** hub's `--secret` | | *(hub API key)* | — | For a **GitHub-authenticated** hub: get one from `https:///cli`, set `MXCLI_HUB_KEY` (see below) | | `--watch` | off | Rebuild + hot-apply on every project change | +| `--web-client-timeout` | `$MXCLI_WEB_CLIENT_TIMEOUT`, else `5m` | Limit for one web client bundle build (e.g. `10m`); on timeout the tail of `deployment/log/web-client-build.log` is printed | | `--ensure-db` | off | Provision local Postgres + the app database if missing (fresh-session bootstrap) | | `--setup` | off | Prepare prerequisites (cache MxBuild+runtime, ensure DB) and exit without booting — for a SessionStart hook | | `--app-port` | 8080 | App HTTP port |