diff --git a/.claude/skills/incremental-build/architecture.md b/.claude/skills/incremental-build/architecture.md index fc9d686b027..1ad440dfbb1 100644 --- a/.claude/skills/incremental-build/architecture.md +++ b/.claude/skills/incremental-build/architecture.md @@ -38,7 +38,11 @@ Use this table to locate source files. ALWAYS read the relevant source file befo | `BuildContext` | `lib/build/helpers/BuildContext.js` | Global build config, project context cache | | `getBuildSignature` | `lib/build/helpers/getBuildSignature.js` | Build signature computation: `BUILD_SIG_VERSION` + build config + project config | | `ProjectBuildContext` | `lib/build/helpers/ProjectBuildContext.js` | Per-project bridge between builder, tasks, and cache | -| `WatchHandler` | `lib/build/helpers/WatchHandler.js` | `@parcel/watcher`-based source path watcher; emits change events to BuildServer. The watcher coalesces its own events with a 50 ms min / 500 ms max wait, so a continuous operation is delivered as batches up to 500 ms apart | +| `WatchHandler` | `lib/build/helpers/WatchHandler.js` | `@parcel/watcher`-based source path watcher; emits `change` events to BuildServer. The watcher coalesces its own events with a 50 ms min / 500 ms max wait, so a continuous operation is delivered as batches up to 500 ms apart. Survives a `git checkout` moving source paths: drops events whose path no longer maps (`getVirtualPath` throws) and skips a path that vanished before `subscribe` resolved, instead of escalating to a fatal error. The `ProjectDefinitionWatcher` then re-inits over the new graph | +| `ProjectDefinitionWatcher` | `lib/graph/ProjectDefinitionWatcher.js` | `@parcel/watcher`-based watcher for project-definition files (`ui5.yaml` / `--config`, `package.json`, workspace config, static dependency-definition file). Emits `definitionChanging` (leading) and `definitionChanged` (trailing, coalesced) to drive a full serving-stack re-init. Owned by `@ui5/server`'s `Supervisor`, not the BuildServer; exported via the `@ui5/project/graph/ProjectDefinitionWatcher` subpath | +| `RecoveryBudget` | `lib/build/helpers/RecoveryBudget.js` | Sliding-window loop protection for watcher recovery (`WATCHER_RECOVERY_MAX_ATTEMPTS` = 5 within `WATCHER_RECOVERY_WINDOW_MS` = 60000). One instance per watcher, so a fault in one does not consume the other's budget | +| `watchSettle` | `lib/build/helpers/watchSettle.js` | Single source of `WATCHER_BURST_SETTLE_MS` = 550 ms, shared by every `@parcel/watcher` consumer (sized above the watcher's 500 ms coalescing cap) | +| `drainSubscriptions` | `lib/build/helpers/watchSubscriptions.js` | Unsubscribes a list of subscriptions in parallel (`Promise.allSettled`), returns the failures. Used by both watchers' `destroy()` and BuildServer's recovery re-subscribe | | `TaskRunner` | `lib/build/TaskRunner.js` | Task composition, execution loop, abort handling | | `Cache` enum | `lib/build/cache/Cache.js` | Cache mode constants: `Default`, `Force`, `ReadOnly`, `Off` (CLI `--cache` option) | | `ProjectBuildCache` | `lib/build/cache/ProjectBuildCache.js` | Cache orchestration per project: index management, stage lookup, result recording | @@ -95,11 +99,15 @@ When a source file changes: 2. `_projectResourceChanged()` walks `traverseDependents()` and calls `ProjectBuildStatus.invalidate({reason, fileAddedOrRemoved})` on the affected project and every dependent. Change is queued in `#resourceChangeQueue` 3. `invalidate()` clears any latched error (lifting the ERRORED gate), aborts the running build via `AbortSignal`, and rotates the `AbortController` 4. `fileAddedOrRemoved=true` (create/delete events) additionally evicts the cached reader on the status. Pure modifies keep the reader so callers already holding its promise still resolve -5. The build loop catches `AbortBuildError`, distinguishes abort from concurrent-change failure, and re-enqueues projects that aren't fresh. Both the source-change-aborted build and a build that *failed* while sources were still changing (the transient branch, `signal.aborted || #resourceChangeQueue.size > 0`) defer their restart until changes settle (`ABORTED_BUILD_RESTART_SETTLE_MS` = 550 ms, reset by each further change) rather than firing on the snappy request debounce — a burst delivered as multiple watcher batches then collapses into one rebuild against the settled tree instead of a build-abort cycle per batch. Both report `SETTLING` for the window's duration: the doomed build no longer parks the banner on `building` (abort) or flips it to `error` (transient failure); it reports "waiting for changes to settle" and retries once the tree is quiet. A reader request supersedes the deferred restart by enqueueing on the normal `BUILD_REQUEST_DEBOUNCE_MS` (10 ms), so serving a request is not delayed. A genuine, non-transient failure still latches ERRORED. +5. The build loop catches `AbortBuildError` and re-enqueues projects that aren't fresh. Two branches defer their restart instead of firing on the request debounce: the source-change-aborted build, and a build that *failed* while sources were still changing (the transient branch, `signal.aborted || #resourceChangeQueue.size > 0`). The restart waits `ABORTED_BUILD_RESTART_SETTLE_MS` (= `WATCHER_BURST_SETTLE_MS` = 550 ms) of quiet, reset by each further change, so a multi-batch burst collapses into one rebuild against the settled tree. The deferral arms the queue timer and sets `#pendingDeferredRestart`. Both branches report `SETTLING` for the window (they no longer park the banner on `building` or flip it to `error`). A genuine, non-transient failure still latches ERRORED. + + While `#pendingDeferredRestart` holds, a reader request does *not* supersede the window: `#enqueueBuild` queues the project and returns without re-arming, and the request resolves when the deferred rebuild runs. Pulling the restart forward would build into a still-arriving burst; resetting it per request would let live-reload traffic defer the rebuild indefinitely. Only source changes reset the window. 6. The first speculative build after a source change from a quiet state is held for a short first-build window (`FIRST_BUILD_SETTLE_MS` = 100 ms, also reported as `SETTLING`) rather than the snappy debounce — this absorbs an editor's own multi-file save fan-out (100 ms sits far below the watcher's 500 ms coalescing cap, roughly at its 50 ms floor) so a save-all doesn't fire a build into a half-written tree. It applies only to a build that is already pending (a reader request queued but not yet started); laziness is preserved — with nothing queued, a change still waits for a reader request. On its own it does not cover a multi-second `git checkout`; full coverage of that comes from the transient-failure deferral above. 7. Queued resource changes are flushed via `#flushResourceChanges()` before the next build starts (must happen before `projectBuilder.build`) -The server also emits a `sourcesChanged` event to drive live-reload notifications. Emission is **leading-edge**: the first change of a quiet period notifies immediately (a lone edit reaches clients at the watcher's own ~50 ms latency floor with no debounce added), and a trailing settle window (`SOURCES_CHANGED_SETTLE_MS` = 550 ms, above the watcher's 500 ms cap) coalesces the remainder of a burst into one further emit. Because emission is leading-edge, the window size does not affect single-edit latency — it only controls burst coalescing. +The server also emits a `sourcesChanged` event to drive live-reload notifications. Emission is **leading-edge**: the first change of a quiet period notifies immediately (a lone edit reaches clients at the watcher's own ~50 ms latency floor with no debounce added), and a trailing settle window (`SOURCES_CHANGED_SETTLE_MS` = `WATCHER_BURST_SETTLE_MS` = 550 ms, above the watcher's 500 ms cap) coalesces the remainder of a burst into one further emit. Because emission is leading-edge, the window size does not affect single-edit latency: it only controls burst coalescing. + +The three source-watcher settle windows (`SOURCES_CHANGED_SETTLE_MS`, `ABORTED_BUILD_RESTART_SETTLE_MS`, and the ProjectDefinitionWatcher's `DEFINITION_CHANGED_SETTLE_MS`) all resolve to `WATCHER_BURST_SETTLE_MS` in `watchSettle.js`, sized above `@parcel/watcher`'s 500 ms `MAX_WAIT_TIME` so each batch resets the window rather than terminating it. `FIRST_BUILD_SETTLE_MS` (100 ms) is deliberately separate: it absorbs an editor's save fan-out, not a multi-batch operation. ### State Machine (per project) @@ -186,6 +194,31 @@ After a build cycle ends with some projects still in INITIAL (e.g. dependencies A build request preempts an in-flight pass: `#triggerRequestQueue` awaits `#stopActiveValidation` before claiming the builder's `buildIsRunning` lock. The pass's `finally` re-invokes `#reconcileServerState({mayValidate: false})` — `mayValidate=false` prevents stack recursion into another validation pass over the projects the previous one just released. +## Two Watchers: Source vs. Definition + +Two independent `@parcel/watcher` consumers feed different pipelines: + +- **Source watcher** (`WatchHandler`, owned by the `BuildServer`): watches source paths, emits `change` events that drive incremental rebuilds *inside* the BuildServer (the File Watch and Abort flow above). +- **Definition watcher** (`ProjectDefinitionWatcher`, owned by `@ui5/server`'s `Supervisor`): watches project-definition files, drives a full re-init of the serving stack *above* the BuildServer. A definition change (topology, config) requires re-resolving the graph, which no incremental rebuild can do, so it re-creates the graph + Express app + BuildServer behind the stable `http.Server`. + +The split is why the source watcher tolerates a `git checkout` moving paths under it: the definition watcher owns the re-init that re-targets it at the new graph, so the source watcher only has to survive the churn. + +Both watchers share `RecoveryBudget` (loop protection, one budget each), `drainSubscriptions` (parallel unsubscribe), and `WATCHER_BURST_SETTLE_MS`. + +### ProjectDefinitionWatcher + +`ProjectDefinitionWatcher extends EventEmitter`, modeled on `WatchHandler`. Documented here because it shares the watch helpers and settle discipline, though it is owned by `@ui5/server`. + +- **Watch set** (`#resolveWatchSet`): traverses `graph.traverseBreadthFirst()` collecting each `project.getRootPath()`. Per project it watches `package.json` always, plus `ui5.yaml` (except the root when a custom `rootConfigPath` (`--config`) is given, which is watched instead and may live outside the root). Adds `workspaceConfigPath` (default `ui5-workspace.yaml` at cwd) when set, and `dependencyDefinitionPath` in `--dependency-definition` mode, where that file is itself a topology definition. +- **Include-based model**: subscribes to each distinct directory (deduplicated across projects); the callback drops every event whose resolved path is not in `#watchedFiles`. The `node_modules`/`.git` ignore globs only reduce OS-level watch load; correctness comes from the include set. +- **Events**: + - `definitionChanging` on the leading edge (first watched event), used to placeholder the project version during re-resolution. + - `definitionChanged` on the trailing edge after `DEFINITION_CHANGED_SETTLE_MS` (= `WATCHER_BURST_SETTLE_MS`), coalescing a `git checkout` burst into a single re-init. Trailing-only: re-creating the stack on the first byte of a checkout would be wasted. +- **Recovery** (`#recoverWatcher`): mirrors `BuildServer.#recoverWatcher`. A synchronous re-entrancy guard collapses parcel's per-path error storm into one recovery, `RecoveryBudget` caps attempts, exhaustion escalates to a terminal `error`. The include set is unchanged; only OS-level handles are renewed. +- **`destroy()`**: idempotent (drains `#subscriptions` to `[]` first), aggregates unsubscribe failures into an `AggregateError` emitted as `error`. + +The supervisor owns the watcher because it outlives individual BuildServer instances (destroyed on every swap) and is re-targeted over the new graph after each swap. See `@ui5/server`'s `Supervisor` for the re-init/swap wiring. + ## Caching Architecture ### Cache Layers @@ -452,7 +485,7 @@ Stage metadata stored on disk includes: ## Key Architectural Patterns 1. **Lazy building**: Projects built on-demand when readers are requested -2. **Request batching**: Multiple pending build requests processed in single batch (`BUILD_REQUEST_DEBOUNCE_MS` = 10ms debounce). A source-change-driven first build is held on a short settle window (`FIRST_BUILD_SETTLE_MS` = 100ms) to absorb editor save fan-out, and a source-change-aborted or transiently-failed build restarts on a longer window (`ABORTED_BUILD_RESTART_SETTLE_MS` = 550ms) so a burst collapses into one rebuild. Both windows report the `SETTLING` state; a reader request supersedes them at the snappy 10ms debounce. +2. **Request batching**: Multiple pending build requests processed in single batch (`BUILD_REQUEST_DEBOUNCE_MS` = 10ms debounce). A source-change-driven first build is held on `FIRST_BUILD_SETTLE_MS` = 100ms to absorb editor save fan-out; a source-change-aborted or transiently-failed build restarts on `ABORTED_BUILD_RESTART_SETTLE_MS` (= `WATCHER_BURST_SETTLE_MS` = 550ms) so a burst collapses into one rebuild. Both windows report `SETTLING`. A reader request supersedes the first-build window at the 10ms debounce, but not the deferred post-abort/transient restart (`#pendingDeferredRestart`): the queued request waits for the deferred rebuild. 3. **Abort/retry**: File changes abort running builds; projects re-queued automatically 4. **Structural sharing**: Derived hash trees share unchanged subtrees, reducing memory 5. **Content-addressed storage**: Resources deduplicated via integrity hashes in custom CAS (synchronous path resolution, gzip-compressed) diff --git a/packages/cli/lib/cli/commands/serve.js b/packages/cli/lib/cli/commands/serve.js index e70924bc53f..77dc3c8b3ba 100644 --- a/packages/cli/lib/cli/commands/serve.js +++ b/packages/cli/lib/cli/commands/serve.js @@ -147,23 +147,36 @@ serve.handler = async function(argv) { const {graphFromStaticFile, graphFromPackageDependencies} = await import("@ui5/project/graph"); - let graph; - if (argv.dependencyDefinition) { - graph = await graphFromStaticFile({ - filePath: argv.dependencyDefinition, - rootConfigPath: argv.config, - versionOverride: argv.frameworkVersion, - snapshotCache: argv.snapshotCache ?? argv.cacheMode ?? "Default", // Use cacheMode as fallback - }); - } else { - graph = await graphFromPackageDependencies({ - rootConfigPath: argv.config, - versionOverride: argv.frameworkVersion, - snapshotCache: argv.snapshotCache ?? argv.cacheMode ?? "Default", // Use cacheMode as fallback - workspaceConfigPath: argv.workspaceConfig, - workspaceName: argv.workspace === false ? null : argv.workspace, - }); - } + // Workspace resolution is active unless static-graph mode is used or --workspace is disabled. + // Single source for both the graph resolve (below) and the watcher config, so the two cannot + // drift on whether workspace resolution runs. + const workspaceActive = !argv.dependencyDefinition && argv.workspace !== false; + + // One graph-construction site, reused for the initial graph and every re-init the server + // performs on a project-definition change. Captures argv so the server can re-resolve with + // identical parameters. + const buildGraph = async () => { + if (argv.dependencyDefinition) { + return graphFromStaticFile({ + filePath: argv.dependencyDefinition, + rootConfigPath: argv.config, + versionOverride: argv.frameworkVersion, + snapshotCache: argv.snapshotCache ?? argv.cacheMode ?? "Default", // Use cacheMode as fallback + }); + } else { + return graphFromPackageDependencies({ + rootConfigPath: argv.config, + versionOverride: argv.frameworkVersion, + snapshotCache: argv.snapshotCache ?? argv.cacheMode ?? "Default", // Use cacheMode as fallback + workspaceConfigPath: argv.workspaceConfig, + workspaceName: workspaceActive ? argv.workspace : null, + }); + } + }; + + // Build the initial graph up front: its root server settings resolve the port and + // live-reload defaults below, before the server binds. + const graph = await buildGraph(); let port = argv.port; let changePortIfInUse = false; @@ -210,6 +223,11 @@ serve.handler = async function(argv) { cache: argv.cache, includedTasks: argv["include-task"], excludedTasks: argv["exclude-task"], + // Threaded to the definition watcher so it watches the same files buildGraph resolves from. + // null selects the watcher's default ui5-workspace.yaml, undefined disables workspace watching. + rootConfigPath: argv.config, + workspaceConfigPath: workspaceActive ? (argv.workspaceConfig ?? null) : undefined, + dependencyDefinitionPath: argv.dependencyDefinition, }; if (serverConfig.h2) { @@ -221,9 +239,11 @@ serve.handler = async function(argv) { const {promise: pOnError, reject} = Promise.withResolvers(); const {serve: serverServe} = await import("@ui5/server"); + // Pass buildGraph as the graphFactory so the server can re-resolve the graph and re-create + // the serving stack when its definition watcher observes a project-definition change. const {h2, port: actualPort} = await serverServe(graph, serverConfig, function(err) { reject(err); - }); + }, {graphFactory: buildGraph}); if (argv.open !== undefined) { const protocol = h2 ? "https" : "http"; diff --git a/packages/cli/test/lib/cli/commands/serve.js b/packages/cli/test/lib/cli/commands/serve.js index ffab26787f0..bcf35f7f263 100644 --- a/packages/cli/test/lib/cli/commands/serve.js +++ b/packages/cli/test/lib/cli/commands/serve.js @@ -125,9 +125,21 @@ test.serial("ui5 serve: default", async (t) => { liveReload: true, includedTasks: undefined, excludedTasks: undefined, + rootConfigPath: undefined, + workspaceConfigPath: null, + dependencyDefinitionPath: undefined, } ]); t.is(typeof server.serve.getCall(0).args[2], "function"); + + // The 4th argument carries a graphFactory the server can call to re-resolve the graph on a + // project-definition change. It must produce the same graph via the same builder + args. + const options = server.serve.getCall(0).args[3]; + t.is(typeof options.graphFactory, "function"); + await options.graphFactory(); + t.is(graph.graphFromPackageDependencies.callCount, 2, "graphFactory re-invokes the same builder"); + t.deepEqual(graph.graphFromPackageDependencies.getCall(1).args, graph.graphFromPackageDependencies.getCall(0).args, + "graphFactory re-resolves with identical parameters"); }); test.serial("ui5 serve --h2", async (t) => { @@ -169,6 +181,9 @@ test.serial("ui5 serve --h2", async (t) => { liveReload: true, includedTasks: undefined, excludedTasks: undefined, + rootConfigPath: undefined, + workspaceConfigPath: null, + dependencyDefinitionPath: undefined, } ]); @@ -204,6 +219,9 @@ test.serial("ui5 serve --accept-remote-connections", async (t) => { liveReload: true, includedTasks: undefined, excludedTasks: undefined, + rootConfigPath: undefined, + workspaceConfigPath: null, + dependencyDefinitionPath: undefined, } ]); }); diff --git a/packages/logger/lib/writers/Console.js b/packages/logger/lib/writers/Console.js index 8f308704539..2f101a6c86e 100644 --- a/packages/logger/lib/writers/Console.js +++ b/packages/logger/lib/writers/Console.js @@ -113,7 +113,7 @@ class Console { process.on("ui5.project-build-metadata", this._handleProjectBuildMetadataEvent); process.on("ui5.build-status", this._handleBuildStatusEvent); process.on("ui5.project-build-status", this._handleProjectBuildStatusEvent); - process.on("ui5.project-resolved", this._handleProjectResolvedEvent); + process.on("ui5.project-resolve-succeeded", this._handleProjectResolvedEvent); process.on("ui5.server-listening", this._handleServerListeningEvent); process.on("ui5.log.stop-console", this._handleStop); } @@ -129,7 +129,7 @@ class Console { process.off("ui5.project-build-metadata", this._handleProjectBuildMetadataEvent); process.off("ui5.build-status", this._handleBuildStatusEvent); process.off("ui5.project-build-status", this._handleProjectBuildStatusEvent); - process.off("ui5.project-resolved", this._handleProjectResolvedEvent); + process.off("ui5.project-resolve-succeeded", this._handleProjectResolvedEvent); process.off("ui5.server-listening", this._handleServerListeningEvent); process.off("ui5.log.stop-console", this._handleStop); if (this.#progressBarContainer) { diff --git a/packages/logger/lib/writers/InteractiveConsole.js b/packages/logger/lib/writers/InteractiveConsole.js index b0f35c737e8..a398075e444 100644 --- a/packages/logger/lib/writers/InteractiveConsole.js +++ b/packages/logger/lib/writers/InteractiveConsole.js @@ -9,6 +9,8 @@ import { setProject, setFramework, enableProjectPlaceholders, + setVersionResolving, + clearVersionResolving, } from "./interactiveConsole/state/project.js"; import {createServerState, setListening, enableServerPlaceholders} from "./interactiveConsole/state/server.js"; import { @@ -97,8 +99,6 @@ class InteractiveConsole { stderr: {orig: null, partial: ""}, }; - #seenProjectResolved = false; - // Bound listeners so we can `process.off` them on stop(). #onLog; #onBuildMetadata; @@ -109,6 +109,8 @@ class InteractiveConsole { #onToolMode; #onProjectResolved; #onProjectFrameworkResolved; + #onProjectResolving; + #onProjectResolveFailed; #onServerListening; #onStopConsole; #onResize; @@ -299,6 +301,8 @@ class InteractiveConsole { this.#onToolMode = (evt) => this.#handleToolMode(evt); this.#onProjectResolved = (evt) => this.#handleProjectResolved(evt); this.#onProjectFrameworkResolved = (evt) => this.#handleProjectFrameworkResolved(evt); + this.#onProjectResolving = () => this.#handleProjectResolving(); + this.#onProjectResolveFailed = () => this.#handleProjectResolveFailed(); this.#onServerListening = (evt) => this.#handleServerListening(evt); this.#onStopConsole = () => this.disable(); this.#onResize = () => this.#handleResize(); @@ -310,8 +314,10 @@ class InteractiveConsole { process.on("ui5.serve-status", this.#onServeStatus); process.on("ui5.tool-info", this.#onToolInfo); process.on("ui5.tool-mode", this.#onToolMode); - process.on("ui5.project-resolved", this.#onProjectResolved); + process.on("ui5.project-resolve-succeeded", this.#onProjectResolved); process.on("ui5.project-framework-resolved", this.#onProjectFrameworkResolved); + process.on("ui5.project-resolve-started", this.#onProjectResolving); + process.on("ui5.project-resolve-failed", this.#onProjectResolveFailed); process.on("ui5.server-listening", this.#onServerListening); process.on("ui5.log.stop-console", this.#onStopConsole); if (typeof this.#stderr.on === "function") { @@ -327,8 +333,10 @@ class InteractiveConsole { process.off("ui5.serve-status", this.#onServeStatus); process.off("ui5.tool-info", this.#onToolInfo); process.off("ui5.tool-mode", this.#onToolMode); - process.off("ui5.project-resolved", this.#onProjectResolved); + process.off("ui5.project-resolve-succeeded", this.#onProjectResolved); process.off("ui5.project-framework-resolved", this.#onProjectFrameworkResolved); + process.off("ui5.project-resolve-started", this.#onProjectResolving); + process.off("ui5.project-resolve-failed", this.#onProjectResolveFailed); process.off("ui5.server-listening", this.#onServerListening); process.off("ui5.log.stop-console", this.#onStopConsole); if (typeof this.#stderr.off === "function") { @@ -369,15 +377,11 @@ class InteractiveConsole { } #handleProjectResolved(evt) { - if (this.#seenProjectResolved) { - // The writer's model is single-root-project. A second - // `ui5.project-resolved` event means the emitter violated that - // invariant, making subsequent event attribution ambiguous, so fail - // fast instead of trying to deduplicate. - throw new Error( - `writers/InteractiveConsole: Received duplicate ui5.project-resolved event`); - } - this.#seenProjectResolved = true; + // The writer's model is single-root-project, but the root can be resolved more + // than once in a process: `Supervisor.reinitialize()` re-resolves the graph + // on a project-definition change (a `git checkout`, a hand-edit of ui5.yaml), and + // re-emits this event for the same root. A repeat updates the project region in + // place; framework data is updated through `ui5.project-framework-resolved`. setProject(this.#projectState, evt); this.#render(); } @@ -387,6 +391,21 @@ class InteractiveConsole { this.#render(); } + // A definition change is coming: blank the version slot(s) to a "resolving…" placeholder. A + // completed resolve arrives as `ui5.project-resolve-succeeded` and repopulates them via + // #handleProjectResolved; an abandoned/failed resolve arrives as `ui5.project-resolve-failed`. + #handleProjectResolving() { + setVersionResolving(this.#projectState); + this.#render(); + } + + // A re-resolve was abandoned or failed without a completing `ui5.project-resolve-succeeded`: release the + // placeholder back to the last-known version rather than leaving it on "resolving…". + #handleProjectResolveFailed() { + clearVersionResolving(this.#projectState); + this.#render(); + } + #handleServerListening(evt) { setListening(this.#serverState, evt); this.#render(); diff --git a/packages/logger/lib/writers/interactiveConsole/render.js b/packages/logger/lib/writers/interactiveConsole/render.js index 2ee193e803d..2a9b90c804e 100644 --- a/packages/logger/lib/writers/interactiveConsole/render.js +++ b/packages/logger/lib/writers/interactiveConsole/render.js @@ -45,8 +45,14 @@ export function renderProjectRegion(projectState) { if (projectState.project) { const project = projectState.project; const framework = projectState.framework; + const resolving = projectState.versionResolving; const projectType = project.type ? chalk.dim(`(${project.type})`) : ""; - const projectVersion = project.version ? chalk.dim("v" + project.version) : ""; + // While a re-resolve is pending, the version is about to change: show a placeholder in + // its slot rather than the stale value. The name and type keep showing so the region + // does not collapse. + const projectVersion = resolving ? + placeholder("resolving…") : + (project.version ? chalk.dim("v" + project.version) : ""); lines.push(`${chalk.dim("Project")} ${chalk.bold(project.name)}` + (projectType ? ` ${projectType}` : "") + (projectVersion ? ` ${projectVersion}` : "")); @@ -56,8 +62,12 @@ export function renderProjectRegion(projectState) { // making "(none)" misleading; omitting the row also avoids a stale // "Framework resolving…" placeholder flashing before it disappears. if (framework?.name) { - const frameworkVersion = framework.version ? ` ${framework.version}` : ""; - lines.push(`${chalk.dim("Framework")} ${chalk.bold(framework.name + frameworkVersion)}`); + // The framework version goes stale alongside the project version on a branch + // switch: placeholder it too, keeping the name. + const frameworkVersion = resolving ? + ` ${placeholder("resolving…")}` : + (framework.version ? chalk.bold(` ${framework.version}`) : ""); + lines.push(`${chalk.dim("Framework")} ${chalk.bold(framework.name)}${frameworkVersion}`); } } else { // Placeholder mode: reserve only the Project row. The Framework row is diff --git a/packages/logger/lib/writers/interactiveConsole/state/project.js b/packages/logger/lib/writers/interactiveConsole/state/project.js index f0a58286fc9..04d0b37afb8 100644 --- a/packages/logger/lib/writers/interactiveConsole/state/project.js +++ b/packages/logger/lib/writers/interactiveConsole/state/project.js @@ -1,4 +1,4 @@ -// Region 2 — root project. Populated by `ui5.project-resolved` and, when +// Region 2 — root project. Populated by `ui5.project-resolve-succeeded` and, when // framework usage is actually resolved for the current run, by // `ui5.project-framework-resolved`. export function createProjectState() { @@ -9,11 +9,19 @@ export function createProjectState() { // of real data. Enabled by `ui5.tool-mode` before the graph is built so // the layout is stable from the very first frame. showPlaceholders: false, + // When true, the version slot(s) render a dim "resolving…" placeholder while the + // project name and type keep showing. Enabled by `ui5.project-resolve-started` once a + // definition change is known to be coming (a `git checkout`, a ui5.yaml edit) and the + // resolved version is about to change. `setProject` clears it; a failed re-resolve + // releases it via `clearVersionResolving`. + versionResolving: false, }; } export function setProject(state, evt) { state.project = {name: evt.name, type: evt.type, version: evt.version}; + // A resolve completed: drop the placeholder in favour of the new version. + state.versionResolving = false; } export function setFramework(state, framework) { @@ -26,3 +34,14 @@ export function setFramework(state, framework) { export function enableProjectPlaceholders(state) { state.showPlaceholders = true; } + +// Blanks the version slot(s) to a "resolving…" placeholder: a re-resolve is coming. +export function setVersionResolving(state) { + state.versionResolving = true; +} + +// Releases the placeholder back to the last-known version: a re-resolve was abandoned or failed +// without a completing `ui5.project-resolve-succeeded`. +export function clearVersionResolving(state) { + state.versionResolving = false; +} diff --git a/packages/logger/test/lib/writers/Console.js b/packages/logger/test/lib/writers/Console.js index e0bf2a9473f..834cb102f4a 100644 --- a/packages/logger/test/lib/writers/Console.js +++ b/packages/logger/test/lib/writers/Console.js @@ -1122,7 +1122,7 @@ test.serial("Build metadata events (same project)", (t) => { test.serial("Project resolved event renders a one-line summary", (t) => { const {stderrWriteStub} = t.context; - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", @@ -1134,7 +1134,7 @@ test.serial("Project resolved event renders a one-line summary", (t) => { test.serial("Project resolved event omits the type label when the type is missing", (t) => { const {stderrWriteStub} = t.context; - process.emit("ui5.project-resolved", {name: "my.app", version: "1.0.0"}); + process.emit("ui5.project-resolve-succeeded", {name: "my.app", version: "1.0.0"}); t.is(stderrWriteStub.callCount, 1); t.is(stripAnsi(stderrWriteStub.getCall(0).args[0]), `info Root project: my.app (1.0.0)\n`); @@ -1142,7 +1142,7 @@ test.serial("Project resolved event omits the type label when the type is missin test.serial("Project resolved event omits the version suffix when the version is missing", (t) => { const {stderrWriteStub} = t.context; - process.emit("ui5.project-resolved", {name: "my.app", type: "library"}); + process.emit("ui5.project-resolve-succeeded", {name: "my.app", type: "library"}); t.is(stderrWriteStub.callCount, 1); t.is(stripAnsi(stderrWriteStub.getCall(0).args[0]), `info Root project: library project my.app\n`); diff --git a/packages/logger/test/lib/writers/InteractiveConsole.js b/packages/logger/test/lib/writers/InteractiveConsole.js index 853fe032be6..c3c526decd5 100644 --- a/packages/logger/test/lib/writers/InteractiveConsole.js +++ b/packages/logger/test/lib/writers/InteractiveConsole.js @@ -47,7 +47,7 @@ test.serial("tool-info populates the header region", (t) => { test.serial("project-resolved populates the project region", (t) => { const {writer} = createWriter(); - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", @@ -79,26 +79,86 @@ test.serial("project-framework-resolved populates the framework region", (t) => writer.disable(); }); -test.serial("duplicate project-resolved throws", (t) => { +test.serial("repeated project-resolved updates the project region in place", (t) => { const {writer} = createWriter(); - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", }); + process.emit("ui5.project-framework-resolved", { + framework: {name: "SAPUI5", version: "1.150.0"}, + }); - // The writer's model is single-root-project. A second event means the - // caller violated the invariant. - t.throws(() => { - process.emit("ui5.project-resolved", { - name: "other.app", - type: "application", - version: "2.0.0", - }); - }, { - message: /duplicate ui5\.project-resolved/, + // A re-init (Supervisor.reinitialize) re-resolves the graph and re-emits + // for the same root; the framework version and type may have changed across the + // switch. The writer updates the region in place rather than throwing. + process.emit("ui5.project-resolve-succeeded", { + name: "my.app", + type: "library", + version: "2.0.0", + }); + process.emit("ui5.project-framework-resolved", { + framework: {name: "OpenUI5", version: "1.151.0"}, + }); + + const state = writer._getStateForTest(); + t.deepEqual(state.project.project, {name: "my.app", type: "library", version: "2.0.0"}); + t.deepEqual(state.project.framework, {name: "OpenUI5", version: "1.151.0"}); + + writer.disable(); +}); + +test.serial("project-resolving shows a version placeholder, project-resolved clears it", (t) => { + const {writer, stderr} = createWriter(); + + process.emit("ui5.project-resolve-succeeded", { + name: "my.app", + type: "application", + version: "1.0.0", }); + process.emit("ui5.project-framework-resolved", { + framework: {name: "SAPUI5", version: "1.150.0"}, + }); + + // A definition change is coming (a branch switch): the version is stale and + // about to change, so the version slots render a placeholder in place. + process.emit("ui5.project-resolve-started"); + t.true(writer._getStateForTest().project.versionResolving); + let output = stripAnsi(stderr.writes.join("")); + t.regex(output, /Project\s+my\.app\s+\(application\)\s+resolving…/, "version slot shows the placeholder"); + + // The completed resolve arrives and replaces the placeholder with the new version. + process.emit("ui5.project-resolve-succeeded", { + name: "my.app", + type: "application", + version: "2.0.0", + }); + process.emit("ui5.project-framework-resolved", { + framework: {name: "SAPUI5", version: "1.151.0"}, + }); + t.false(writer._getStateForTest().project.versionResolving, "a completed resolve clears the placeholder"); + output = stripAnsi(stderr.writes.join("")); + t.regex(output, /v2\.0\.0/, "the new version is shown after resolve"); + + writer.disable(); +}); + +test.serial("project-resolve-failed clears the placeholder back to the last-known version", (t) => { + const {writer} = createWriter(); + + process.emit("ui5.project-resolve-succeeded", { + name: "my.app", type: "application", version: "1.0.0", + }); + process.emit("ui5.project-resolve-started"); + t.true(writer._getStateForTest().project.versionResolving); + + // A re-resolve was abandoned without a project-resolved (e.g. a failed graph + // resolution): the placeholder is released, falling back to the last version. + process.emit("ui5.project-resolve-failed"); + t.false(writer._getStateForTest().project.versionResolving); + t.is(writer._getStateForTest().project.project.version, "1.0.0", "last-known version is retained"); writer.disable(); }); @@ -189,7 +249,7 @@ test.serial("regions are order-tolerant — server before project", (t) => { urls: [{label: "Local", url: "http://localhost:8080"}], acceptRemoteConnections: false, }); - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", }); @@ -302,7 +362,7 @@ test.serial("frame includes visible content for each populated region", (t) => { const {writer, stderr} = createWriter(); process.emit("ui5.tool-info", {name: "UI5 CLI", version: "1.2.3"}); - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", }); process.emit("ui5.project-framework-resolved", { @@ -380,7 +440,7 @@ test.serial("tool-mode 'serve' placeholders are replaced by real data", (t) => { t.regex(midOutput, /binding…/); t.regex(midOutput, /starting/); - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: "my.app", type: "application", version: "1.0.0", }); process.emit("ui5.project-framework-resolved", { @@ -1005,7 +1065,7 @@ test.serial("#registerSignalHandlers() is a no-op when the map is already popula // process's listeners to baseline on teardown. const events = [ "ui5.log", "ui5.build-metadata", "ui5.build-status", "ui5.project-build-status", - "ui5.serve-status", "ui5.tool-info", "ui5.tool-mode", "ui5.project-resolved", + "ui5.serve-status", "ui5.tool-info", "ui5.tool-mode", "ui5.project-resolve-succeeded", "ui5.server-listening", "ui5.log.stop-console", "SIGHUP", "SIGINT", "SIGTERM", "SIGBREAK", "exit", ]; diff --git a/packages/logger/test/lib/writers/interactiveConsole/render.js b/packages/logger/test/lib/writers/interactiveConsole/render.js index 216d9950fa1..ce446b4bf63 100644 --- a/packages/logger/test/lib/writers/interactiveConsole/render.js +++ b/packages/logger/test/lib/writers/interactiveConsole/render.js @@ -22,7 +22,7 @@ import { } from "../../../../lib/writers/interactiveConsole/state/build.js"; import {createHeaderState, setTool} from "../../../../lib/writers/interactiveConsole/state/header.js"; -import {createProjectState, setProject, setFramework, enableProjectPlaceholders} from +import {createProjectState, setProject, setFramework, enableProjectPlaceholders, setVersionResolving} from "../../../../lib/writers/interactiveConsole/state/project.js"; import {createServerState, setListening, enableServerPlaceholders} from "../../../../lib/writers/interactiveConsole/state/server.js"; @@ -128,6 +128,32 @@ test("renderProjectRegion: project rendered without type/version still includes t.notRegex(plain, /bare\.project\s+\(/, "no type marker is rendered when type is absent"); }); +test("renderProjectRegion: version slot shows a placeholder while resolving, name and type stay", (t) => { + const state = createProjectState(); + setProject(state, { + name: "my.app", + type: "application", + version: "1.0.0", + }); + setFramework(state, {name: "SAPUI5", version: "1.150.0"}); + setVersionResolving(state); + const plain = renderProjectRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /Project\s+my\.app\s+\(application\)\s+resolving…/, "name and type stay, version is a placeholder"); + t.notRegex(plain, /v1\.0\.0/, "the stale project version is not shown while resolving"); + // The framework version goes stale alongside it — placeholdered, name kept. + t.regex(plain, /Framework\s+SAPUI5\s+resolving…/, "framework name stays, its version is a placeholder"); + t.notRegex(plain, /1\.150\.0/, "the stale framework version is not shown while resolving"); +}); + +test("renderProjectRegion: resolving over a frameworkless project placeholders only the project version", (t) => { + const state = createProjectState(); + setProject(state, {name: "my.app", type: "application", version: "1.0.0"}); + setVersionResolving(state); + const plain = renderProjectRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /Project\s+my\.app\s+\(application\)\s+resolving…/); + t.notRegex(plain, /Framework/, "no Framework row is invented while resolving"); +}); + // ---- renderServerRegion ------------------------------------------------------- test("renderServerRegion: empty when no urls and no placeholders", (t) => { diff --git a/packages/logger/test/lib/writers/interactiveConsole/state/project.js b/packages/logger/test/lib/writers/interactiveConsole/state/project.js index 6a8d180272f..96eb98c7930 100644 --- a/packages/logger/test/lib/writers/interactiveConsole/state/project.js +++ b/packages/logger/test/lib/writers/interactiveConsole/state/project.js @@ -1,6 +1,7 @@ import test from "ava"; -import {createProjectState, setProject, setFramework, enableProjectPlaceholders} from +import {createProjectState, setProject, setFramework, enableProjectPlaceholders, setVersionResolving, + clearVersionResolving} from "../../../../../lib/writers/interactiveConsole/state/project.js"; test("createProjectState: fresh state has no project/framework and placeholders disabled", (t) => { @@ -8,6 +9,7 @@ test("createProjectState: fresh state has no project/framework and placeholders project: null, framework: null, showPlaceholders: false, + versionResolving: false, }); }); @@ -47,3 +49,18 @@ test("enableProjectPlaceholders: flips the showPlaceholders flag", (t) => { enableProjectPlaceholders(state); t.true(state.showPlaceholders); }); + +test("setVersionResolving: sets the versionResolving flag; clearVersionResolving releases it", (t) => { + const state = createProjectState(); + setVersionResolving(state); + t.true(state.versionResolving); + clearVersionResolving(state); + t.false(state.versionResolving); +}); + +test("setProject: clears a pending versionResolving flag", (t) => { + const state = createProjectState(); + setVersionResolving(state); + setProject(state, {name: "my.app", type: "application", version: "2.0.0"}); + t.false(state.versionResolving, "a completed resolve drops the placeholder"); +}); diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 67ad1e55407..b847cec42a6 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -3,6 +3,8 @@ import {createReaderCollectionPrioritized} from "@ui5/fs/resourceFactory"; import BuildReader from "./BuildReader.js"; import WatchHandler from "./helpers/WatchHandler.js"; import {isAbortError} from "./helpers/abort.js"; +import {WATCHER_BURST_SETTLE_MS} from "./helpers/watchSettle.js"; +import RecoveryBudget, {WATCHER_RECOVERY_MAX_ATTEMPTS, WATCHER_RECOVERY_WINDOW_MS} from "./helpers/RecoveryBudget.js"; import {getLogger} from "@ui5/logger"; import ServeLogger from "@ui5/logger/internal/loggers/Serve"; const log = getLogger("build:BuildServer"); @@ -13,14 +15,8 @@ const log = getLogger("build:BuildServer"); // well under 100 ms on small projects, where a trailing debounce would dominate edit-to-reload // latency. The emit is therefore leading-edge (the first change of a quiet period fires // immediately), followed by this window that coalesces the rest of a burst into one trailing emit. -// -// The value is tied to @parcel/watcher's MAX_WAIT_TIME (500 ms): the watcher caps its own -// coalescing there, so a continuous operation (e.g. `git checkout`) arrives as batches up to -// 500 ms apart rather than one quiet-terminated batch. A window below the cap would see quiet -// between batches and emit per batch; above it, each batch resets the window so the whole -// operation collapses to one leading + one trailing emit. Do not lower below 500 ms without -// revisiting that relationship. -const SOURCES_CHANGED_SETTLE_MS = 550; +// Sized to WATCHER_BURST_SETTLE_MS so a multi-batch operation collapses (see that constant). +const SOURCES_CHANGED_SETTLE_MS = WATCHER_BURST_SETTLE_MS; // Debounce for the request queue. A reader request enqueues a build and triggers the queue after // this short delay so near-simultaneous requests build together. Serving a request must not wait, @@ -41,20 +37,12 @@ const FIRST_BUILD_SETTLE_MS = 100; // Settle window for restarting a build that a source change aborted. When a change lands mid-build // the running build is aborted at once, but the restart is held until changes have been quiet for -// this long (each further change resets it). During a burst (a `git checkout`, a save-all, a bundler -// writing many files) @parcel/watcher delivers batches up to its MAX_WAIT_TIME (500 ms) apart; -// restarting on BUILD_REQUEST_DEBOUNCE_MS would spawn a build per batch, each aborted by the next. -// Holding the restart above the watcher's cap collapses the burst into a single build against the -// settled tree. Reader-request-driven builds keep the short debounce, so this delay only applies to -// the speculative post-abort restart, not to serving a request. -const ABORTED_BUILD_RESTART_SETTLE_MS = 550; - -// Loop protection for watcher recovery, so a persistently failing watcher (e.g. a watched path -// that keeps erroring on re-subscribe, or an FS that keeps dropping events) does not cycle -// error -> recover -> error forever. More than WATCHER_RECOVERY_MAX_ATTEMPTS recoveries within -// WATCHER_RECOVERY_WINDOW_MS is treated as unrecoverable and escalates to the terminal ERROR state. -const WATCHER_RECOVERY_MAX_ATTEMPTS = 5; -const WATCHER_RECOVERY_WINDOW_MS = 60000; +// this long (each further change resets it). Sized to WATCHER_BURST_SETTLE_MS so a burst (a `git +// checkout`, a save-all, a bundler writing many files) collapses into a single build against the +// settled tree rather than one aborted build per watcher batch (see that constant). Reader-request- +// driven builds keep the short debounce, so this delay only applies to the speculative post-abort +// restart, not to serving a request. +const ABORTED_BUILD_RESTART_SETTLE_MS = WATCHER_BURST_SETTLE_MS; // The server's ACTIVITY state: what the server is doing right now. Mutated exclusively through // #setState. Orthogonal to project staleness (which projects are up-to-date), which is reported @@ -141,10 +129,10 @@ class BuildServer extends EventEmitter { #validationAbort = null; // Watcher recovery state. `#recoveringWatcher` guards against re-entrant recovery while a // recovery pass is in flight (a dropped-events fault emits one error per subscribed path - // in a synchronous burst). `#watcherRecoveryTimestamps` holds the completion times of - // recent recoveries for the loop-protection window. + // in a synchronous burst). `#watcherRecoveryBudget` caps recoveries within a sliding window + // (its own budget, independent of the ProjectDefinitionWatcher's). #recoveringWatcher = false; - #watcherRecoveryTimestamps = []; + #watcherRecoveryBudget = new RecoveryBudget(); /** * Creates a new BuildServer instance @@ -296,10 +284,7 @@ class BuildServer extends EventEmitter { // Loop protection: a persistently failing watcher would otherwise cycle forever, since // dropped-events faults arrive via the subscription callback (not a watch() rejection) // and so never trip the reject-based fallback below. - const now = Date.now(); - this.#watcherRecoveryTimestamps = this.#watcherRecoveryTimestamps - .filter((ts) => now - ts < WATCHER_RECOVERY_WINDOW_MS); - if (this.#watcherRecoveryTimestamps.length >= WATCHER_RECOVERY_MAX_ATTEMPTS) { + if (!this.#watcherRecoveryBudget.withinBudget()) { this.#recoveringWatcher = false; log.error(`File watcher failed to recover after ${WATCHER_RECOVERY_MAX_ATTEMPTS} attempts ` + `within ${WATCHER_RECOVERY_WINDOW_MS} ms. Giving up.`); @@ -348,7 +333,7 @@ class BuildServer extends EventEmitter { status.invalidate({reason: "File watcher recovery", fileAddedOrRemoved: true}); } - this.#watcherRecoveryTimestamps.push(Date.now()); + this.#watcherRecoveryBudget.recordRecovery(); log.info(`File watcher recovered. Re-scanning all project sources.`); // Every project is now non-fresh. The build was quiesced above, so the server is at rest: @@ -654,14 +639,22 @@ class BuildServer extends EventEmitter { } /** - * Enqueues a project for building and triggers the request queue at the short debounce. + * Enqueues a project for building and triggers the request queue. * - * Serving a request must not wait, so this always (re-)arms the queue at + * Serving a request must not wait, so this normally (re-)arms the queue at * BUILD_REQUEST_DEBOUNCE_MS - even when the project is already queued. A reader - * request thereby supersedes a deferred settle window (the post-abort/transient restart at - * ABORTED_BUILD_RESTART_SETTLE_MS, or the first-build window at - * FIRST_BUILD_SETTLE_MS): the parked build is pulled forward to the short debounce - * rather than left waiting out the longer window. + * request thereby supersedes the first-build settle window (FIRST_BUILD_SETTLE_MS): + * the parked build is pulled forward to the short debounce rather than left waiting. + * + * The exception is a source-change-aborted or transiently-failed build waiting out its restart + * window (#pendingDeferredRestart). That window exists to collapse a burst delivered + * as multiple watcher batches (a git checkout, a save-all) into one rebuild against + * the settled tree. A reader request that arrives mid-burst (as a browser's live-reload does) + * must neither pull the restart forward (firing a build into a still-arriving burst) nor reset + * the window (a stream of live-reload requests would defer the rebuild indefinitely). So while a + * restart is deferred, leave its armed timer untouched: the request is still queued on the status + * and resolves when the deferred rebuild (which processes the whole pending set) runs. The + * window is reset only by further source changes (see {@link #_projectResourceChanged}). * * @param {string} projectName Name of the project to enqueue */ @@ -670,9 +663,14 @@ class BuildServer extends EventEmitter { log.verbose(`Enqueuing project '${projectName}' for build`); this.#pendingBuildRequest.add(projectName); } - // Always re-arm at the default debounce: an explicit reader request supersedes any longer - // settle window an earlier source change may have armed. - this.#triggerRequestQueue(); + if (this.#pendingDeferredRestart) { + // A deferred post-abort/transient restart owns the armed timer. Don't disturb it: the + // project is queued above and the restart will build it against the settled tree. + log.verbose(`Reader request for project '${projectName}' queued behind deferred restart`); + return; + } + // Re-arm at the default debounce: a reader request supersedes the short first-build window. + this.#triggerRequestQueue(BUILD_REQUEST_DEBOUNCE_MS); } #triggerRequestQueue(delay = BUILD_REQUEST_DEBOUNCE_MS) { diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index af84f8ae952..386d0658782 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -1283,36 +1283,36 @@ export default class ProjectBuildCache { * #cachedFrozenSourceMetadata (re-read from the persisted cache during * #initSourceIndex). * - * No-op in {@link @ui5/project/build/cache/Cache}.Off mode, where no index or result - * cache exists to reset. - * * @public */ discardIncrementalState() { if (this.#cacheMode === Cache.Off) { return; } + // RESTORING_PROJECT_INDICES re-arms initSourceIndex (see its guard), so the next build + // re-runs it before validateCache, re-globbing the source tree from scratch. this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES; - this.#sourceIndex = null; this.#taskCache.clear(); // Reset the result cache state: a prior validateCache may have moved it to // NO_CACHE or FRESH_AND_IN_USE. The next build's validateCache asserts // PENDING_VALIDATION after the dependency-index restore step, so any other // value would trip that assertion. this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; - this.#changedProjectSourcePaths = []; + // Not touched by #initSourceIndex, so this reset is required. this.#changedDependencyResourcePaths = []; // Derived per-build state a discarded (failed) build must not leave behind. // #currentResultSignature drives the #findResultCache early return; #currentStageSignatures - // drives the isInitialImport/setStage guards in #importStages; #writtenResultResourcePaths - // is normally emptied by allTasksCompleted, which a thrown build never reaches. + // drives the isInitialImport/setStage guards in #importStages. this.#currentResultSignature = undefined; - this.#cachedResultSignature = undefined; this.#currentStageSignatures = new Map(); - this.#writtenResultResourcePaths = []; // Reset the stage pipeline so #importStages re-initializes stages and re-imports // cached results instead of reusing the failed build's partial writers. this.#project.getProjectResources().reset(); + + // Overwritten before read on the next build (#sourceIndex by #initSourceIndex, + // #cachedResultSignature by #findResultCache); reset only so this stays a complete reset. + this.#sourceIndex = null; + this.#cachedResultSignature = undefined; } /** diff --git a/packages/project/lib/build/helpers/RecoveryBudget.js b/packages/project/lib/build/helpers/RecoveryBudget.js new file mode 100644 index 00000000000..1e4d8521196 --- /dev/null +++ b/packages/project/lib/build/helpers/RecoveryBudget.js @@ -0,0 +1,52 @@ +// Default loop-protection budget for watcher recovery: more than WATCHER_RECOVERY_MAX_ATTEMPTS +// recoveries within WATCHER_RECOVERY_WINDOW_MS is treated as unrecoverable by the owning watcher. +export const WATCHER_RECOVERY_MAX_ATTEMPTS = 5; +export const WATCHER_RECOVERY_WINDOW_MS = 60000; + +/** + * Sliding-window loop protection for watcher recovery. A watcher that keeps failing would otherwise + * cycle error → recover → error forever; this caps recoveries to maxAttempts within a + * trailing windowMs. Counts completed recoveries: check {@link withinBudget} + * before attempting one and call {@link recordRecovery} once it succeeds. + * + * Owned per watcher (the source {@link WatchHandler}'s recovery in BuildServer and the + * {@link @ui5/project/graph/ProjectDefinitionWatcher} each hold their own), so a fault in one does + * not consume the other's budget. The re-entrancy guard, the recovery work, and the escalation on + * exhaustion stay with the owner, which knows how to escalate (a state transition, a terminal event). + * + * @private + * @memberof @ui5/project/build/helpers + */ +class RecoveryBudget { + #timestamps = []; + #maxAttempts; + #windowMs; + + constructor(maxAttempts = WATCHER_RECOVERY_MAX_ATTEMPTS, windowMs = WATCHER_RECOVERY_WINDOW_MS) { + this.#maxAttempts = maxAttempts; + this.#windowMs = windowMs; + } + + /** + * Prunes recoveries older than the window and reports whether another attempt fits the budget. + * + * @returns {boolean} true if fewer than maxAttempts recoveries remain + * within the trailing window + */ + withinBudget() { + const now = Date.now(); + this.#timestamps = this.#timestamps.filter((ts) => now - ts < this.#windowMs); + return this.#timestamps.length < this.#maxAttempts; + } + + /** + * Records a completed recovery against the window. + * + * @returns {void} + */ + recordRecovery() { + this.#timestamps.push(Date.now()); + } +} + +export default RecoveryBudget; diff --git a/packages/project/lib/build/helpers/WatchHandler.js b/packages/project/lib/build/helpers/WatchHandler.js index 7ec3eebb89e..21d27227561 100644 --- a/packages/project/lib/build/helpers/WatchHandler.js +++ b/packages/project/lib/build/helpers/WatchHandler.js @@ -1,8 +1,21 @@ import EventEmitter from "node:events"; +import {access} from "node:fs/promises"; import parcelWatcher from "@parcel/watcher"; import {getLogger} from "@ui5/logger"; +import {drainSubscriptions} from "./watchSubscriptions.js"; const log = getLogger("build:helpers:WatchHandler"); +// Resolves true if the path is accessible, false otherwise. Used to distinguish a subscribe failure +// caused by a vanished source path (skippable) from a genuine watcher fault (must escalate). +async function pathExists(p) { + try { + await access(p); + return true; + } catch { + return false; + } +} + /** * Context of a build process * @@ -29,19 +42,33 @@ class WatchHandler extends EventEmitter { log.verbose(`Watching source path(s): ${paths.join(", ")}`); await Promise.all(paths.map(async (path) => { - const subscription = await parcelWatcher.subscribe(path, (err, events) => { - if (err) { - this.emit("error", err); - return; - } - for (const event of events) { - try { - this.#handleWatchEvents(event.type, event.path, project); - } catch (handlerErr) { - this.emit("error", handlerErr); + let subscription; + try { + subscription = await parcelWatcher.subscribe(path, (err, events) => { + if (err) { + this.emit("error", err); + return; + } + for (const event of events) { + try { + this.#handleWatchEvents(event.type, event.path, project); + } catch (handlerErr) { + this.emit("error", handlerErr); + } } + }); + } catch (err) { + if (await pathExists(path)) { + // Path present but subscribe failed: a genuine watcher fault. Let it propagate so + // BuildServer's loop-protected recovery/escalation path still applies. + throw err; } - }); + // Source path vanished (e.g. removed by a branch switch) before we could subscribe. + // Skip it rather than wedging the whole server in terminal ERROR; the next graph + // resolution re-targets the watcher. At worst the project stays stale until then. + log.warn(`Skipping watch on missing source path (moved/removed): ${path}`); + return; + } this.#subscriptions.push(subscription); })); } @@ -51,10 +78,7 @@ class WatchHandler extends EventEmitter { // failure cannot leave stale handles behind to be unsubscribed twice. const subscriptions = this.#subscriptions; this.#subscriptions = []; - // Run in parallel and collect failures so a single misbehaving subscription - // cannot leak the others. - const results = await Promise.allSettled(subscriptions.map((s) => s.unsubscribe())); - const failures = results.filter((r) => r.status === "rejected").map((r) => r.reason); + const failures = await drainSubscriptions(subscriptions); if (failures.length) { const err = new AggregateError(failures, "Failed to unsubscribe one or more file watchers"); this.emit("error", err); @@ -62,7 +86,18 @@ class WatchHandler extends EventEmitter { } #handleWatchEvents(eventType, filePath, project) { - const resourcePath = project.getVirtualPath(filePath); + let resourcePath; + try { + resourcePath = project.getVirtualPath(filePath); + } catch (err) { + // Source dir moved/removed under the running watcher (e.g. git checkout): the path no + // longer maps into the project the watcher was armed with. Drop the event rather than + // escalating to a fatal watcher error; the definition watcher re-inits the serving stack + // over the new graph, which re-targets the watcher at the current paths. + log.verbose(`Dropping watch event for unmappable path ${filePath} ` + + `in project '${project.getName()}': ${err.message}`); + return; + } if (log.isLevelEnabled("silly")) { log.silly(`FS event: ${eventType} ${filePath} (as ${resourcePath} in project '${project.getName()}')`); } diff --git a/packages/project/lib/build/helpers/watchSettle.js b/packages/project/lib/build/helpers/watchSettle.js new file mode 100644 index 00000000000..2db60614b9d --- /dev/null +++ b/packages/project/lib/build/helpers/watchSettle.js @@ -0,0 +1,15 @@ +/** + * Settle window (ms) for collapsing a filesystem-event burst into a single trailing action, shared + * by every @parcel/watcher consumer in the build layer. + * + * @parcel/watcher caps its own event coalescing at MAX_WAIT_TIME (500 ms): during a continuous + * operation (a `git checkout`, an editor's save-all, a bundler writing many files) it delivers + * events as batches up to 500 ms apart rather than one quiet-terminated batch. A window that + * collapses such a burst must therefore sit above that cap, so each further batch resets + * the window rather than prematurely terminating it; 550 ms adds a small margin. Lowering it below + * 500 ms breaks the coalescing relationship. + * + * @private + * @type {number} + */ +export const WATCHER_BURST_SETTLE_MS = 550; diff --git a/packages/project/lib/build/helpers/watchSubscriptions.js b/packages/project/lib/build/helpers/watchSubscriptions.js new file mode 100644 index 00000000000..42533cb8e7a --- /dev/null +++ b/packages/project/lib/build/helpers/watchSubscriptions.js @@ -0,0 +1,16 @@ +/** + * Unsubscribes every subscription in parallel and returns the failures. Callers drain their + * subscription list to [] before calling, so a second drain is a no-op and a partial + * failure cannot leave stale handles behind to be unsubscribed twice. Running in parallel and + * collecting failures keeps a single misbehaving subscription from leaking the others. + * + * @private + * @param {object[]} subscriptions Subscriptions to drain, each exposing an async + * unsubscribe() + * @returns {Promise} The reasons of any rejected unsubscribe() calls, empty + * when all succeeded + */ +export async function drainSubscriptions(subscriptions) { + const results = await Promise.allSettled(subscriptions.map((s) => s.unsubscribe())); + return results.filter((r) => r.status === "rejected").map((r) => r.reason); +} diff --git a/packages/project/lib/graph/ProjectDefinitionWatcher.js b/packages/project/lib/graph/ProjectDefinitionWatcher.js new file mode 100644 index 00000000000..9487b4cd739 --- /dev/null +++ b/packages/project/lib/graph/ProjectDefinitionWatcher.js @@ -0,0 +1,239 @@ +import EventEmitter from "node:events"; +import path from "node:path"; +import parcelWatcher from "@parcel/watcher"; +import {getLogger} from "@ui5/logger"; +import {drainSubscriptions} from "../build/helpers/watchSubscriptions.js"; +import {WATCHER_BURST_SETTLE_MS} from "../build/helpers/watchSettle.js"; +import RecoveryBudget, { + WATCHER_RECOVERY_MAX_ATTEMPTS, WATCHER_RECOVERY_WINDOW_MS, +} from "../build/helpers/RecoveryBudget.js"; +const log = getLogger("graph:ProjectDefinitionWatcher"); + +// Default filename of the workspace configuration, resolved against cwd. +const WORKSPACE_CONFIG_DEFAULT = "ui5-workspace.yaml"; + +// Settle window for the `definitionChanged` event, in milliseconds. +// +// A `git checkout` or branch switch writes ui5.yaml + package.json + sources within one operation; +// a trailing timer, reset on each further event, collapses that burst into a single emit. Unlike +// BuildServer's live-reload emit, a re-init needs no leading edge (re-creating the serving stack on +// the first byte of a checkout is wasteful), so this window is trailing-only. Sized to +// WATCHER_BURST_SETTLE_MS so each batch resets the window rather than terminating it (see that +// constant). +const DEFINITION_CHANGED_SETTLE_MS = WATCHER_BURST_SETTLE_MS; + +/** + * Watches the project-definition files (ui5.yaml, package.json, the workspace config, and, in + * static-graph mode, the dependency-definition file) and emits a settled, coalesced + * definitionChanged when one changes. A definitionChanging fires on the + * leading edge of a burst (the first watched event, before the settle window) so an owner can react + * to a pending change ahead of the coalesced definitionChanged. + * + * Separate from the source {@link WatchHandler}: source events drive incremental rebuilds inside + * the BuildServer, definition events drive a full re-init of the serving stack above it. The watch + * model is include-based: @parcel/watcher subscribes to each distinct directory and the change + * callback drops every event whose path is not in the resolved definition-file set. The + * node_modules/.git ignore globs only reduce OS-level watch load; + * correctness comes from the include set. + * + * @private + * @memberof @ui5/project/graph + */ +class ProjectDefinitionWatcher extends EventEmitter { + #subscriptions = []; + // Absolute paths of the definition files to react to. The subscription callback filters + // against this set; everything else is dropped. + #watchedFiles = new Set(); + // dir -> Set: the distinct directories to subscribe, each mapped to its + // definition files. Retained so recovery can re-subscribe the same set. + #watchDirs = new Map(); + + #settleTimer = null; + #lastEvent = null; + + #recovering = false; + #recoveryBudget = new RecoveryBudget(); + #destroyed = false; + + /** + * Resolves the watch set, subscribes to each distinct directory, awaits readiness, and returns + * the watcher. Mirrors BuildServer awaiting WatchHandler readiness before serving, so a change + * made immediately after startup is not missed. + * + * @param {object} options + * @param {@ui5/project/graph/ProjectGraph} options.graph The resolved project graph + * @param {string} [options.rootConfigPath] Custom config path for the root project (--config) + * @param {string} [options.workspaceConfigPath] Workspace config path (default ui5-workspace.yaml). + * Omit in static-graph mode, which does not use the workspace. + * @param {string} [options.dependencyDefinitionPath] Static dependency-definition file + * (--dependency-definition), watched when present + * @param {string} [options.cwd=process.cwd()] Base directory for resolving relative paths + * @returns {Promise} The armed watcher + */ + static async create({graph, rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd} = {}) { + const watcher = new ProjectDefinitionWatcher(); + await watcher.#resolveWatchSet({graph, rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd}); + await watcher.#subscribeAll(); + return watcher; + } + + // Builds the dir -> {definition files} include set from the graph and the threaded paths. + async #resolveWatchSet({graph, rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd}) { + const baseDir = cwd ? path.resolve(cwd) : process.cwd(); + const resolve = (p) => (path.isAbsolute(p) ? p : path.join(baseDir, p)); + + const add = (filePath) => { + const abs = path.resolve(filePath); + this.#watchedFiles.add(abs); + const dir = path.dirname(abs); + let files = this.#watchDirs.get(dir); + if (!files) { + files = new Set(); + this.#watchDirs.set(dir, files); + } + files.add(abs); + }; + + const rootName = graph.getRoot().getName(); + const rootCustomConfig = rootConfigPath ? resolve(rootConfigPath) : null; + + await graph.traverseBreadthFirst(({project}) => { + const rootPath = project.getRootPath(); + add(path.join(rootPath, "package.json")); + if (rootCustomConfig && project.getName() === rootName) { + // The root carries a custom --config file, which may live outside its root. + add(rootCustomConfig); + } else { + add(path.join(rootPath, "ui5.yaml")); + } + }); + + // The workspace config lives at cwd. It may not exist yet; a create event on it still + // matters (it can introduce workspace resolution on the next re-init). + if (workspaceConfigPath !== undefined) { + add(resolve(workspaceConfigPath || WORKSPACE_CONFIG_DEFAULT)); + } + + // Static-graph mode: the dependency-definition file is itself a topology definition; + // editing it changes the graph exactly like package.json does. + if (dependencyDefinitionPath) { + add(resolve(dependencyDefinitionPath)); + } + } + + // Subscribes to every distinct directory in parallel, resolving once all are armed. + async #subscribeAll() { + const dirs = [...this.#watchDirs.keys()]; + log.verbose(`Watching definition file(s) in: ${dirs.join(", ")}`); + await Promise.all(dirs.map((dir) => this.#subscribeDir(dir))); + } + + async #subscribeDir(dir) { + const subscription = await parcelWatcher.subscribe(dir, (err, events) => { + if (err) { + this.#recoverWatcher(err); + return; + } + for (const event of events) { + // Include-set filter: drop every event that is not a watched definition file. + if (!this.#watchedFiles.has(path.resolve(event.path))) { + continue; + } + this.#onDefinitionEvent(event.type, event.path); + } + }, {ignore: ["**/node_modules/**", "**/.git/**"]}); + this.#subscriptions.push(subscription); + } + + // Trailing-only settle: reset the timer on each event so a multi-batch operation collapses to + // a single emit once changes have been quiet for the window. + #onDefinitionEvent(eventType, filePath) { + if (log.isLevelEnabled("silly")) { + log.silly(`Definition file event: ${eventType} ${filePath}`); + } + this.#lastEvent = {eventType, filePath}; + if (this.#settleTimer) { + clearTimeout(this.#settleTimer); + } else { + // Leading edge of a burst: a re-init (and a version change) is now known to be coming, + // though the trailing `definitionChanged` is still a settle window away. Owners use + // this to signal the pending change ahead of the re-resolve. + this.emit("definitionChanging", this.#lastEvent); + } + this.#settleTimer = setTimeout(() => { + this.#settleTimer = null; + const event = this.#lastEvent; + this.#lastEvent = null; + this.emit("definitionChanged", event); + }, DEFINITION_CHANGED_SETTLE_MS); + } + + // Recreates the subscriptions after a watcher error. Modeled on BuildServer.#recoverWatcher: + // a synchronous re-entrancy guard collapses parcel's per-path error storm into one recovery, + // and loop protection escalates to a terminal "error" if the watcher keeps failing. + async #recoverWatcher(err) { + // Set synchronously before the first await so re-entrant emissions bail here. + if (this.#destroyed || this.#recovering) { + return; + } + this.#recovering = true; + log.warn(`Definition watcher error, attempting to recover: ${err?.message ?? err}`); + if (err?.stack) { + log.verbose(err.stack); + } + + if (!this.#recoveryBudget.withinBudget()) { + this.#recovering = false; + log.error(`Definition watcher failed to recover after ${WATCHER_RECOVERY_MAX_ATTEMPTS} attempts ` + + `within ${WATCHER_RECOVERY_WINDOW_MS} ms. Giving up.`); + this.emit("error", err); + return; + } + + try { + // Tear down the current subscriptions and re-subscribe the same watch set. The include + // set (#watchedFiles / #watchDirs) is unchanged; only the OS-level handles are renewed. + // Teardown failures are ignored here: the handles are being discarded regardless, and a + // re-subscribe failure below is what determines whether recovery succeeded. + const subscriptions = this.#subscriptions; + this.#subscriptions = []; + await drainSubscriptions(subscriptions); + if (this.#destroyed) { + return; + } + await this.#subscribeAll(); + this.#recoveryBudget.recordRecovery(); + log.info(`Definition watcher recovered.`); + } catch (recoveryErr) { + log.error(`Definition watcher recovery failed: ${recoveryErr?.message ?? recoveryErr}`); + this.emit("error", recoveryErr); + } finally { + this.#recovering = false; + } + } + + /** + * Unsubscribes all watchers. Idempotent; a second call is a no-op. Unsubscribe failures are + * aggregated into an AggregateError emitted as error. + * + * @returns {Promise} Resolves once every subscription has been drained + */ + async destroy() { + this.#destroyed = true; + if (this.#settleTimer) { + clearTimeout(this.#settleTimer); + this.#settleTimer = null; + } + // Drain the subscriptions list first so a second destroy() is a no-op and a partial + // failure cannot leave stale handles behind to be unsubscribed twice. + const subscriptions = this.#subscriptions; + this.#subscriptions = []; + const failures = await drainSubscriptions(subscriptions); + if (failures.length) { + const err = new AggregateError(failures, "Failed to unsubscribe one or more definition watchers"); + this.emit("error", err); + } + } +} + +export default ProjectDefinitionWatcher; diff --git a/packages/project/lib/graph/projectGraphBuilder.js b/packages/project/lib/graph/projectGraphBuilder.js index 4c404559ba3..1e6562bd863 100644 --- a/packages/project/lib/graph/projectGraphBuilder.js +++ b/packages/project/lib/graph/projectGraphBuilder.js @@ -143,7 +143,7 @@ async function projectGraphBuilder(nodeProvider, workspace) { // traversal. Consumed by @ui5/logger writers to populate their header / // scrollback lines. Framework information is emitted separately once a // caller actually resolves framework usage for the current run. - process.emit("ui5.project-resolved", { + process.emit("ui5.project-resolve-succeeded", { name: rootProject.getName(), type: rootProject.getType(), version: rootProject.getVersion(), diff --git a/packages/project/package.json b/packages/project/package.json index 17b2e8957c4..a2833c90958 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -29,6 +29,7 @@ "./validation/validator": "./lib/validation/validator.js", "./validation/ValidationError": "./lib/validation/ValidationError.js", "./graph/ProjectGraph": "./lib/graph/ProjectGraph.js", + "./graph/ProjectDefinitionWatcher": "./lib/graph/ProjectDefinitionWatcher.js", "./graph/projectGraphBuilder": "./lib/graph/projectGraphBuilder.js", "./graph": "./lib/graph/graph.js", "./package.json": "./package.json" diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 4311adb3798..6613420ddb8 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -85,9 +85,19 @@ test.beforeEach(async (t) => { } } + // BuildReader is constructed in the BuildServer constructor. Most tests don't exercise it, + // but the reader-request path (#getReaderForProject → #enqueueBuild) is only reachable through + // the buildServerInterface handed to it. Capture that interface so tests can drive reader + // requests directly, mirroring what BuildReader.byPath does for a resource lookup. + t.context.capturedInterfaces = []; + class BuildReader { + constructor(_name, _projects, buildServerInterface) { + t.context.capturedInterfaces.push(buildServerInterface); + } + } + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { - // BuildReader is constructed in the BuildServer constructor but not exercised here. - "../../../lib/build/BuildReader.js": class BuildReader {}, + "../../../lib/build/BuildReader.js": BuildReader, "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, })).default; t.context.BuildServer = BuildServer; @@ -369,6 +379,69 @@ test.serial( await clock.tickAsync(0); }); +test.serial( + "abort-restart: a reader request mid-window must not pull the deferred restart forward", + async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock, capturedInterfaces} = + t.context; + const {ABORTED_BUILD_RESTART_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = + BuildServer.__internals__; + const statusEvents = makeStatusRecorder(t); + + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((opts, cb) => new Promise((resolve, reject) => { + resolvers.push(() => { + if (opts.signal?.aborted) { + const err = new Error("Build aborted"); + err.name = "AbortError"; + reject(err); + return; + } + cb("root.project", {getReader: () => ({fakeReader: true})}); + resolve(["root.project"]); + }); + })); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.context.buildServer = buildServer; + // The interface handed to this server's BuildReaders carries #getReaderForProject — the same + // entry point BuildReader.byPath uses to request a project's built resources. The three + // readers all receive the same interface object, so any of the three just-pushed entries + // works; take the last one to avoid the entries recorded for the beforeEach server. + const {getReaderForProject} = capturedInterfaces[capturedInterfaces.length - 1]; + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, "initial build started"); + + // A source change aborts the running build; the restart is deferred to the settle window. + buildServer._projectResourceChanged(rootProject, "/a.js", false); + resolvers[0](); + await clock.tickAsync(0); + t.is(projectBuilder.build.callCount, 1, "no restart yet — the settle window is open"); + t.is(statusEvents[statusEvents.length - 1].status, "serve-settling", + "state is settling while the restart is deferred"); + + // A browser/live-reload request for the (now invalidated) project lands mid-window. It must + // wait for the settled rebuild, not provoke a build into the still-arriving burst. The + // request is parked; a rejection here would be an unhandled rejection, so swallow it. + getReaderForProject("root.project").catch(() => {}); + + // The burst is still in flight: the settle window has NOT elapsed. Advance past the short + // request debounce (well below the settle window) — no build must start. + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, + "a reader request must not fire a build before the deferred-restart window elapses"); + t.is(statusEvents[statusEvents.length - 1].status, "serve-settling", + "still settling — the reader request did not flip the server out of the settle window"); + + // Only once the window fully elapses does the single deferred rebuild fire. + await clock.tickAsync(ABORTED_BUILD_RESTART_SETTLE_MS); + t.is(projectBuilder.build.callCount, 2, "single deferred rebuild after the window elapsed"); + + // Settle the restarted build so no promise is left dangling. + resolvers[1]?.(); + await clock.tickAsync(0); + }); + test.serial( "serve-status: transient failure during a change burst reports settling, not error", async (t) => { const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; @@ -1706,6 +1779,12 @@ function makeRescanBuilder(sinon) { const droppedEventsError = () => new Error("Events were dropped by the FSEvents client. File system must be re-scanned."); +// Recovery outcome hinges on whether the fresh WatchHandler's watch() resolves or rejects. A +// branch switch that removed a watched source dir is absorbed inside WatchHandler itself — the +// vanished path is skipped, so watch() resolves and recovery settles on STALE (the "recreates +// watcher and forces a full re-scan" test below). A watch() that genuinely rejects (a present-path +// subscribe fault, faked here via failWatchFrom) still escalates to fatal ERROR ("failure to +// re-subscribe" below), and the loop-protection budget still trips on a persistent fault. test.serial( "watcher-recovery: dropped-events error recreates watcher and forces a full re-scan", async (t) => { const {sinon, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; diff --git a/packages/project/test/lib/build/helpers/RecoveryBudget.js b/packages/project/test/lib/build/helpers/RecoveryBudget.js new file mode 100644 index 00000000000..6869b61a344 --- /dev/null +++ b/packages/project/test/lib/build/helpers/RecoveryBudget.js @@ -0,0 +1,56 @@ +import test from "ava"; +import sinon from "sinon"; +import RecoveryBudget, { + WATCHER_RECOVERY_MAX_ATTEMPTS, WATCHER_RECOVERY_WINDOW_MS, +} from "../../../../lib/build/helpers/RecoveryBudget.js"; + +test.afterEach.always(() => { + sinon.restore(); +}); + +test.serial("withinBudget: allows up to maxAttempts recorded recoveries, then refuses", (t) => { + const clock = sinon.useFakeTimers(); + const budget = new RecoveryBudget(3, 1000); + + for (let i = 0; i < 3; i++) { + t.true(budget.withinBudget(), `attempt ${i + 1} is within budget`); + budget.recordRecovery(); + } + t.false(budget.withinBudget(), "the attempt past maxAttempts is refused"); + + clock.restore(); +}); + +test.serial("withinBudget: recoveries older than the window no longer count", (t) => { + const clock = sinon.useFakeTimers(); + const budget = new RecoveryBudget(2, 1000); + + budget.recordRecovery(); + budget.recordRecovery(); + t.false(budget.withinBudget(), "budget exhausted within the window"); + + // Advance past the window: the earlier recoveries fall out and the budget frees up. + clock.tick(1001); + t.true(budget.withinBudget(), "recoveries outside the window are pruned"); + + clock.restore(); +}); + +test("defaults: constructed budget uses the exported default attempts/window", (t) => { + const clock = sinon.useFakeTimers(); + const budget = new RecoveryBudget(); + + for (let i = 0; i < WATCHER_RECOVERY_MAX_ATTEMPTS; i++) { + t.true(budget.withinBudget()); + budget.recordRecovery(); + } + t.false(budget.withinBudget(), "refuses past the default max attempts"); + + // Still refused just inside the default window, allowed just past it. + clock.tick(WATCHER_RECOVERY_WINDOW_MS - 1); + t.false(budget.withinBudget()); + clock.tick(2); + t.true(budget.withinBudget()); + + clock.restore(); +}); diff --git a/packages/project/test/lib/build/helpers/WatchHandler.js b/packages/project/test/lib/build/helpers/WatchHandler.js index 87f5d8e3935..ac7e1016a17 100644 --- a/packages/project/test/lib/build/helpers/WatchHandler.js +++ b/packages/project/test/lib/build/helpers/WatchHandler.js @@ -4,14 +4,19 @@ import esmock from "esmock"; let WatchHandler; let subscribeStub; +let accessStub; test.before(async () => { subscribeStub = sinon.stub(); + accessStub = sinon.stub(); WatchHandler = await esmock("../../../../lib/build/helpers/WatchHandler.js", { "@parcel/watcher": { default: { subscribe: subscribeStub } + }, + "node:fs/promises": { + access: accessStub } }); }); @@ -19,6 +24,7 @@ test.before(async () => { test.afterEach.always(() => { sinon.restore(); subscribeStub.reset(); + accessStub.reset(); }); function createMockSubscription() { @@ -175,7 +181,10 @@ test.serial("watch: forwards dropped-events error from watcher callback", async await handler.destroy(); }); -test.serial("watch: emits error when handler throws", async (t) => { +// A source dir moved/removed under the running watcher (e.g. git checkout) makes getVirtualPath +// throw the unmappable-path error. The event is dropped — neither a `change` nor a fatal `error` is +// emitted — so a branch switch does not escalate the server to terminal ERROR. +test.serial("watch: drops event whose path no longer maps to a virtual path", async (t) => { const subscription = createMockSubscription(); const callbackReady = captureCallback(subscription); @@ -183,7 +192,8 @@ test.serial("watch: emits error when handler throws", async (t) => { const project = { getSourcePaths: () => ["/src"], getVirtualPath: () => { - throw new Error("virtual path error"); + throw new Error( + "Unable to convert source path /src/file.js to virtual path for project test-project"); }, getName: () => "test-project" }; @@ -191,14 +201,15 @@ test.serial("watch: emits error when handler throws", async (t) => { await handler.watch([project]); const callback = await callbackReady; - const errorPromise = new Promise((resolve) => { - handler.on("error", (err) => { - t.is(err.message, "virtual path error"); - resolve(); - }); - }); + const changeSpy = sinon.spy(); + const errorSpy = sinon.spy(); + handler.on("change", changeSpy); + handler.on("error", errorSpy); + callback(null, [{type: "update", path: "/src/file.js"}]); - await errorPromise; + + t.is(changeSpy.callCount, 0, "unmappable event is dropped, not forwarded as a change"); + t.is(errorSpy.callCount, 0, "unmappable event does not escalate to a fatal error"); await handler.destroy(); }); @@ -226,6 +237,54 @@ test.serial("watch: subscribes to each source path", async (t) => { t.true(subB.unsubscribe.calledOnce); }); +// A branch switch can remove a watched source dir before recovery re-subscribes. parcel then +// rejects with a bare "No such file or directory" (no code/errno), so the decision is made on +// current disk state: a missing path is skipped rather than escalated to a fatal error. +test.serial("watch: skips subscribe on a missing source path", async (t) => { + const goodSub = createMockSubscription(); + subscribeStub.withArgs("/src").resolves(goodSub); + subscribeStub.withArgs("/gone").rejects(new Error("No such file or directory")); + // /gone no longer exists on disk; /src does. + accessStub.withArgs("/gone").rejects(new Error("ENOENT")); + accessStub.withArgs("/src").resolves(); + + const handler = new WatchHandler(); + const project = { + getSourcePaths: () => ["/src", "/gone"], + getVirtualPath: (filePath) => filePath, + getName: () => "test-project" + }; + + const errorSpy = sinon.spy(); + handler.on("error", errorSpy); + + await t.notThrowsAsync(handler.watch([project]), "watch resolves despite the missing path"); + t.is(errorSpy.callCount, 0, "no error emitted for a skipped missing path"); + + await handler.destroy(); + t.true(goodSub.unsubscribe.calledOnce, "the surviving path was subscribed and torn down"); +}); + +// A subscribe failure on a path that still exists is a genuine watcher fault, not a vanished +// source dir. It must propagate so BuildServer's loop-protected recovery/escalation applies. +test.serial("watch: rejects when subscribe fails on an existing path", async (t) => { + subscribeStub.withArgs("/src").rejects(new Error("EMFILE: too many open files")); + accessStub.withArgs("/src").resolves(); // path present → genuine fault + + const handler = new WatchHandler(); + const project = { + getSourcePaths: () => ["/src"], + getVirtualPath: (filePath) => filePath, + getName: () => "test-project" + }; + + await t.throwsAsync(handler.watch([project]), { + message: "EMFILE: too many open files" + }, "subscribe failure on an existing path propagates"); + + await handler.destroy(); +}); + test.serial("destroy: unsubscribes subscriptions in parallel", async (t) => { const subA = createMockSubscription(); const subB = createMockSubscription(); diff --git a/packages/project/test/lib/build/helpers/watchSubscriptions.js b/packages/project/test/lib/build/helpers/watchSubscriptions.js new file mode 100644 index 00000000000..1fe6c6fbb33 --- /dev/null +++ b/packages/project/test/lib/build/helpers/watchSubscriptions.js @@ -0,0 +1,36 @@ +import test from "ava"; +import sinon from "sinon"; +import {drainSubscriptions} from "../../../../lib/build/helpers/watchSubscriptions.js"; + +test("drainSubscriptions: unsubscribes every subscription and returns no failures on success", async (t) => { + const subs = [ + {unsubscribe: sinon.stub().resolves()}, + {unsubscribe: sinon.stub().resolves()}, + ]; + + const failures = await drainSubscriptions(subs); + + t.deepEqual(failures, [], "no failures when all unsubscribe cleanly"); + t.true(subs[0].unsubscribe.calledOnce); + t.true(subs[1].unsubscribe.calledOnce); +}); + +test("drainSubscriptions: unsubscribes all in parallel even when some reject, collecting the reasons", + async (t) => { + const errA = new Error("unsub A failed"); + const errC = new Error("unsub C failed"); + const subs = [ + {unsubscribe: sinon.stub().rejects(errA)}, + {unsubscribe: sinon.stub().resolves()}, + {unsubscribe: sinon.stub().rejects(errC)}, + ]; + + const failures = await drainSubscriptions(subs); + + t.deepEqual(failures, [errA, errC], "returns the reasons of the rejected unsubscribes only"); + t.true(subs[1].unsubscribe.calledOnce, "a rejecting sibling does not prevent the others"); + }); + +test("drainSubscriptions: an empty list resolves to no failures", async (t) => { + t.deepEqual(await drainSubscriptions([]), []); +}); diff --git a/packages/project/test/lib/graph/ProjectDefinitionWatcher.js b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js new file mode 100644 index 00000000000..07481901ce2 --- /dev/null +++ b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js @@ -0,0 +1,401 @@ +import test from "ava"; +import sinon from "sinon"; +import esmock from "esmock"; +import path from "node:path"; + +let ProjectDefinitionWatcher; +let subscribeStub; + +test.before(async () => { + subscribeStub = sinon.stub(); + ProjectDefinitionWatcher = await esmock("../../../lib/graph/ProjectDefinitionWatcher.js", { + "@parcel/watcher": { + default: { + subscribe: subscribeStub + } + } + }); +}); + +test.afterEach.always(() => { + sinon.restore(); + subscribeStub.reset(); +}); + +function createMockSubscription() { + return { + unsubscribe: sinon.stub().resolves() + }; +} + +// A fake graph with a root project plus the given dependency roots. Each entry is {name, rootPath}. +function createGraph(root, deps = []) { + const projects = [root, ...deps]; + return { + getRoot: () => ({getName: () => root.name}), + traverseBreadthFirst: async (cb) => { + for (const p of projects) { + await cb({project: {getName: () => p.name, getRootPath: () => p.rootPath}}); + } + } + }; +} + +// Captures the change callback of the subscription for a given directory, keyed by the subscribe +// call's first argument. +function captureCallbacks(subscription = createMockSubscription()) { + const callbacks = new Map(); + subscribeStub.callsFake(async (dir, cb) => { + callbacks.set(dir, cb); + return subscription; + }); + return callbacks; +} + +test.serial("create: resolves watch set to distinct roots with ui5.yaml + package.json", async (t) => { + captureCallbacks(); + const graph = createGraph( + {name: "root", rootPath: "/app"}, + [{name: "dep-a", rootPath: "/deps/a"}, {name: "dep-b", rootPath: "/deps/b"}] + ); + + const watcher = await ProjectDefinitionWatcher.create({graph}); + + const dirs = subscribeStub.getCalls().map((c) => c.args[0]).sort(); + t.deepEqual(dirs, ["/app", "/deps/a", "/deps/b"], "subscribed once per distinct root"); + for (const call of subscribeStub.getCalls()) { + t.deepEqual(call.args[2].ignore, ["**/node_modules/**", "**/.git/**"], "ignore globs passed"); + } + + await watcher.destroy(); +}); + +test.serial("create: shared parent directory is subscribed once", async (t) => { + captureCallbacks(); + // Two projects under the same directory: only one subscription for that dir. + const graph = createGraph( + {name: "root", rootPath: "/mono"}, + [{name: "dep-a", rootPath: "/mono"}] + ); + + const watcher = await ProjectDefinitionWatcher.create({graph}); + + t.is(subscribeStub.callCount, 1, "distinct directory subscribed once"); + t.is(subscribeStub.firstCall.args[0], "/mono"); + + await watcher.destroy(); +}); + +test.serial("create: custom rootConfigPath replaces root ui5.yaml and subscribes its directory", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + + const watcher = await ProjectDefinitionWatcher.create({ + graph, rootConfigPath: "/configs/custom.yaml", cwd: "/app" + }); + + const dirs = subscribeStub.getCalls().map((c) => c.args[0]).sort(); + t.deepEqual(dirs, ["/app", "/configs"].sort(), "subscribes the custom config's directory too"); + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + // The root's default ui5.yaml must NOT be watched. + const rootCb = subscribeStub.getCalls().find((c) => c.args[0] === "/app").args[1]; + rootCb(null, [{type: "update", path: "/app/ui5.yaml"}]); + clock.tick(600); + t.is(emitted.length, 0, "default root ui5.yaml is not watched when a custom config is set"); + + // The custom config must be watched. + const configCb = subscribeStub.getCalls().find((c) => c.args[0] === "/configs").args[1]; + configCb(null, [{type: "update", path: "/configs/custom.yaml"}]); + clock.tick(600); + clock.restore(); + t.is(emitted.length, 1, "custom config change emits"); + + await watcher.destroy(); +}); + +test.serial("create: relative rootConfigPath is resolved against cwd", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + + const watcher = await ProjectDefinitionWatcher.create({ + graph, rootConfigPath: "custom/ui5.yaml", cwd: "/app" + }); + + const dirs = subscribeStub.getCalls().map((c) => c.args[0]); + t.true(dirs.includes(path.join("/app", "custom")), "relative config resolved against cwd"); + + await watcher.destroy(); +}); + +test.serial("create: workspaceConfigPath included (default filename applied) when set", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + + // null → active, use default filename ui5-workspace.yaml at cwd. + const watcher = await ProjectDefinitionWatcher.create({graph, workspaceConfigPath: null, cwd: "/ws"}); + + const wsCall = subscribeStub.getCalls().find((c) => c.args[0] === "/ws"); + t.truthy(wsCall, "workspace directory subscribed"); + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + const clock = sinon.useFakeTimers(); + wsCall.args[1](null, [{type: "create", path: "/ws/ui5-workspace.yaml"}]); + clock.tick(600); + clock.restore(); + t.is(emitted.length, 1, "workspace config change emits"); + + await watcher.destroy(); +}); + +test.serial("create: workspaceConfigPath undefined skips the workspace file", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + + const watcher = await ProjectDefinitionWatcher.create({graph, workspaceConfigPath: undefined, cwd: "/ws"}); + + t.falsy(subscribeStub.getCalls().find((c) => c.args[0] === "/ws"), "workspace dir not subscribed"); + await watcher.destroy(); +}); + +test.serial("create: dependencyDefinitionPath is watched when present", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + + const watcher = await ProjectDefinitionWatcher.create({ + graph, dependencyDefinitionPath: "deps.yaml", cwd: "/app" + }); + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + const appCall = subscribeStub.getCalls().find((c) => c.args[0] === "/app"); + const clock = sinon.useFakeTimers(); + appCall.args[1](null, [{type: "update", path: "/app/deps.yaml"}]); + clock.tick(600); + clock.restore(); + t.is(emitted.length, 1, "static dependency definition change emits"); + + await watcher.destroy(); +}); + +test.serial("filtering: non-definition file events do not emit", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const callback = subscribeStub.firstCall.args[1]; + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + callback(null, [ + {type: "update", path: "/app/src/main.js"}, + {type: "create", path: "/app/node_modules/foo/package.json"} + ]); + clock.tick(600); + clock.restore(); + + t.is(emitted.length, 0, "source file and node_modules path filtered out"); + await watcher.destroy(); +}); + +test.serial("filtering: a watched ui5.yaml event emits", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const callback = subscribeStub.firstCall.args[1]; + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + callback(null, [{type: "update", path: "/app/ui5.yaml"}]); + clock.tick(600); + clock.restore(); + + t.is(emitted.length, 1, "watched ui5.yaml emits"); + t.deepEqual(emitted[0], {eventType: "update", filePath: "/app/ui5.yaml"}); + await watcher.destroy(); +}); + +test.serial("settle: a burst within the window emits definitionChanged exactly once", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const callback = subscribeStub.firstCall.args[1]; + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + // A checkout burst: ui5.yaml + package.json + a source, spread across batches within the window. + callback(null, [{type: "update", path: "/app/ui5.yaml"}]); + clock.tick(200); + callback(null, [{type: "update", path: "/app/package.json"}]); + clock.tick(200); + callback(null, [{type: "update", path: "/app/src/main.js"}]); // filtered, but no reset either + clock.tick(200); // 200 < 550 from last watched event resets — still within window + t.is(emitted.length, 0, "not yet emitted mid-burst"); + clock.tick(600); // quiet past the window + t.is(emitted.length, 1, "collapsed to a single emit"); + + // A later, separate change emits again. + callback(null, [{type: "update", path: "/app/ui5.yaml"}]); + clock.tick(600); + t.is(emitted.length, 2, "later change emits again"); + clock.restore(); + + await watcher.destroy(); +}); + +test.serial("leading edge: definitionChanging fires once on the first event of a burst, before definitionChanged", + async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const callback = subscribeStub.firstCall.args[1]; + + const changing = []; + const changed = []; + watcher.on("definitionChanging", (e) => changing.push(e)); + watcher.on("definitionChanged", (e) => changed.push(e)); + + const clock = sinon.useFakeTimers(); + // First watched event of a burst: definitionChanging fires immediately, definitionChanged does not. + callback(null, [{type: "update", path: "/app/ui5.yaml"}]); + t.is(changing.length, 1, "definitionChanging fires on the leading edge"); + t.deepEqual(changing[0], {eventType: "update", filePath: "/app/ui5.yaml"}); + t.is(changed.length, 0, "definitionChanged has not fired yet"); + + // Further events within the window do not re-fire definitionChanging. + clock.tick(200); + callback(null, [{type: "update", path: "/app/package.json"}]); + t.is(changing.length, 1, "definitionChanging fires once per burst, not per event"); + + // Once quiet, definitionChanged fires once. + clock.tick(600); + t.is(changed.length, 1, "definitionChanged fires after the settle window"); + + // A separate, later burst re-arms the leading edge. + callback(null, [{type: "update", path: "/app/ui5.yaml"}]); + t.is(changing.length, 2, "a new burst fires definitionChanging again"); + clock.tick(600); + clock.restore(); + + await watcher.destroy(); + }); + +test.serial("leading edge: a filtered (non-definition) event does not fire definitionChanging", async (t) => { + captureCallbacks(); + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const callback = subscribeStub.firstCall.args[1]; + + const changing = []; + watcher.on("definitionChanging", (e) => changing.push(e)); + + const clock = sinon.useFakeTimers(); + callback(null, [{type: "update", path: "/app/src/main.js"}]); + clock.tick(600); + clock.restore(); + + t.is(changing.length, 0, "a non-definition file does not fire the leading edge"); + await watcher.destroy(); +}); + +test.serial("recovery: a watcher error tears down and re-subscribes", async (t) => { + const sub1 = createMockSubscription(); + const sub2 = createMockSubscription(); + let cb; + subscribeStub.onFirstCall().callsFake(async (_dir, callback) => { + cb = callback; + return sub1; + }); + subscribeStub.onSecondCall().resolves(sub2); + + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + + t.is(subscribeStub.callCount, 1); + cb(new Error("watcher blew up")); + // Let the async recovery settle. + await new Promise((resolve) => setImmediate(resolve)); + + t.true(sub1.unsubscribe.calledOnce, "old subscription torn down"); + t.is(subscribeStub.callCount, 2, "re-subscribed"); + + await watcher.destroy(); +}); + +test.serial("recovery: loop protection escalates to error after the max attempts", async (t) => { + const subs = []; + subscribeStub.callsFake(async () => { + const s = createMockSubscription(); + subs.push(s); + return s; + }); + + const graph = createGraph({name: "root", rootPath: "/app"}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + + const errors = []; + watcher.on("error", (err) => errors.push(err)); + + // Drive repeated failures. Each recovery re-subscribes, exposing a fresh callback via the + // latest subscribe call. Trigger via the callback captured at subscribe time. + const callbacks = () => subscribeStub.getCalls().map((c) => c.args[1]); + // 5 recoveries allowed, the 6th escalates. + for (let i = 0; i < 6; i++) { + const cbs = callbacks(); + cbs[cbs.length - 1](new Error(`fail ${i}`)); + await new Promise((resolve) => setImmediate(resolve)); + } + + t.is(errors.length, 1, "escalated once after exceeding the budget"); + await watcher.destroy(); +}); + +test.serial("destroy: unsubscribes all, is idempotent", async (t) => { + const sub = createMockSubscription(); + subscribeStub.resolves(sub); + + const graph = createGraph( + {name: "root", rootPath: "/app"}, + [{name: "dep", rootPath: "/dep"}] + ); + const watcher = await ProjectDefinitionWatcher.create({graph}); + + await watcher.destroy(); + await watcher.destroy(); + + t.is(sub.unsubscribe.callCount, subscribeStub.callCount, + "each subscription unsubscribed exactly once across two destroy calls"); +}); + +test.serial("destroy: aggregates unsubscribe failures into an error", async (t) => { + const subA = createMockSubscription(); + const subB = createMockSubscription(); + subA.unsubscribe = sinon.stub().rejects(new Error("unsub A failed")); + subB.unsubscribe = sinon.stub().resolves(); + subscribeStub.onFirstCall().resolves(subA); + subscribeStub.onSecondCall().resolves(subB); + + const graph = createGraph( + {name: "root", rootPath: "/app"}, + [{name: "dep", rootPath: "/dep"}] + ); + const watcher = await ProjectDefinitionWatcher.create({graph}); + + const errorSpy = sinon.spy(); + watcher.on("error", errorSpy); + + await watcher.destroy(); + + t.is(errorSpy.callCount, 1, "single aggregated error emitted"); + t.true(errorSpy.firstCall.args[0] instanceof AggregateError); + t.is(errorSpy.firstCall.args[0].errors.length, 1); +}); diff --git a/packages/project/test/lib/graph/projectGraphBuilder.js b/packages/project/test/lib/graph/projectGraphBuilder.js index 67b36ed4647..56d46db0a17 100644 --- a/packages/project/test/lib/graph/projectGraphBuilder.js +++ b/packages/project/test/lib/graph/projectGraphBuilder.js @@ -966,11 +966,11 @@ test("Multiple dependencies to different module containing the same extension", }); }); -test.serial("Emits ui5.project-resolved with the root project's shape", async (t) => { +test.serial("Emits ui5.project-resolve-succeeded with the root project's shape", async (t) => { const events = []; const listener = (evt) => events.push(evt); - process.on("ui5.project-resolved", listener); - t.teardown(() => process.off("ui5.project-resolved", listener)); + process.on("ui5.project-resolve-succeeded", listener); + t.teardown(() => process.off("ui5.project-resolve-succeeded", listener)); t.context.getRootNode.resolves(createNode({ id: "id1", @@ -979,7 +979,7 @@ test.serial("Emits ui5.project-resolved with the root project's shape", async (t await projectGraphBuilder(t.context.provider); - t.is(events.length, 1, "ui5.project-resolved emitted exactly once"); + t.is(events.length, 1, "ui5.project-resolve-succeeded emitted exactly once"); t.is(events[0].name, "root.project"); t.is(events[0].type, "library"); t.is(events[0].version, "1.0.0"); diff --git a/packages/project/test/lib/package-exports.js b/packages/project/test/lib/package-exports.js index 684e8634a84..14da420ec01 100644 --- a/packages/project/test/lib/package-exports.js +++ b/packages/project/test/lib/package-exports.js @@ -13,7 +13,7 @@ test("export of package.json", (t) => { // Check number of definied exports test("check number of exports", (t) => { const packageJson = require("@ui5/project/package.json"); - t.is(Object.keys(packageJson.exports).length, 14); + t.is(Object.keys(packageJson.exports).length, 15); }); // Public API contract (exported modules) @@ -29,6 +29,7 @@ test("check number of exports", (t) => { "validation/validator", "validation/ValidationError", "graph/ProjectGraph", + "graph/ProjectDefinitionWatcher", "graph/projectGraphBuilder", {exportedSpecifier: "graph", mappedModule: "../../lib/graph/graph.js"}, ].forEach((v) => { diff --git a/packages/server/lib/serve/Supervisor.js b/packages/server/lib/serve/Supervisor.js new file mode 100644 index 00000000000..20c6880cb8a --- /dev/null +++ b/packages/server/lib/serve/Supervisor.js @@ -0,0 +1,286 @@ +import http from "node:http"; +import process from "node:process"; +import {EventEmitter} from "node:events"; +import {getLogger} from "@ui5/logger"; +import ProjectDefinitionWatcher from "@ui5/project/graph/ProjectDefinitionWatcher"; +import buildApp from "./stack.js"; +import attachLiveReloadServer from "../liveReload/server.js"; +import {listen, addSsl, announceListening} from "./httpListener.js"; + +const log = getLogger("server:Supervisor"); + +/** + * Owns the stable HTTP front door for a served project and re-creates the serving stack + * (graph + Express app + BuildServer) when the project definition changes. + * + * The port is bound once. Every request is routed through a stable trampoline to the + * current Express app; a re-initialization swaps that app behind the trampoline, + * so the socket, the bound port, and connected live-reload clients survive the swap. + * + * Re-initialization is build-new-then-swap: the new graph is resolved and + * the new app built before the old one is torn down. If the new definition fails to resolve + * (e.g. an invalid ui5.yaml), the previous working app keeps serving. + * + * @private + */ +class Supervisor extends EventEmitter { + #config; + #graphFactory; + #error; + + #httpServer = null; + #port = null; + + // The current serving stack: {app, buildServer, liveReloadOptions}. Reassigned on swap; the + // trampoline reads #stack.app on every request so a swap retargets transparently. + #stack = null; + + // Stable emitter live-reload subscribes to once; its upstream is retargeted on swap + // so connected browsers stay connected across a re-init. + #sourcesChangedRelay = new EventEmitter(); + #relayUnsubscribe = null; + #liveReloadHandle = null; + + // Watches the project-definition files and drives reinitialize() on a change. Owned by the + // supervisor (not the BuildServer) so it outlives each swapped-out stack, and re-targeted to + // the new graph after every swap. + #definitionWatcher = null; + + #destroyed = false; + #reinitInProgress = false; + #reinitQueued = false; + + constructor(config, error, {graphFactory} = {}) { + super(); + this.#config = config; + this.#error = error; + this.#graphFactory = graphFactory; + } + + /** + * Creates the supervisor, builds the initial serving stack, binds the port, and attaches + * the live-reload WebSocket server. + * + * @param {@ui5/project/graph/ProjectGraph} graph Initial (already resolved) project graph + * @param {object} config Resolved server configuration + * @param {Function} [error] Error callback for out-of-band BuildServer errors + * @param {object} [options] + * @param {Function} [options.graphFactory] Async factory returning a fresh ProjectGraph; + * required for {@link Supervisor#reinitialize} to do anything + * @returns {Promise} The listening supervisor + */ + static async create(graph, config, error, {graphFactory} = {}) { + const supervisor = new Supervisor(config, error, {graphFactory}); + await supervisor.#init(graph); + return supervisor; + } + + async #init(graph) { + const { + port: requestedPort, changePortIfInUse = false, h2 = false, key, cert, + acceptRemoteConnections = false, liveReload = false, + } = this.#config; + + if (h2) { + const nodeVersion = parseInt(process.versions.node.split(".")[0], 10); + if (nodeVersion >= 24) { + log.error("ERROR: With Node v24, usage of HTTP/2 is no longer supported. " + + "Please check https://github.com/UI5/cli/issues/327 for updates."); + process.exit(1); + } + } + + // Build the initial stack before binding so a construction failure surfaces to the caller. + this.#stack = await buildApp(graph, this.#config, this.#error); + + // Stable request handler. Reads #stack.app on every request so a swap retargets + // transparently without touching the bound socket. + const trampoline = (req, res) => this.#stack.app(req, res); + + let port; let server; + try { + const listenTarget = h2 ? + await addSsl({app: trampoline, key, cert}) : + http.createServer(trampoline); + ({port, server} = await listen(listenTarget, requestedPort, changePortIfInUse, acceptRemoteConnections)); + } catch (err) { + // Release the BuildServer (source watcher + cache handle) before rethrowing so a + // failed bind does not leak a running build server. + await this.#stack.buildServer.destroy(); + throw err; + } + this.#httpServer = server; + this.#port = port; + + if (liveReload) { + // Attach once to the stable http server, subscribed to the relay rather than the + // BuildServer directly, so connected clients persist across swaps. + this.#liveReloadHandle = attachLiveReloadServer({ + httpServer: server, + buildServer: this.#sourcesChangedRelay, + token: this.#config.webSocketToken, + }); + } + this.#relayFrom(this.#stack.buildServer); + + // Arm the definition watcher over the initial graph, after the port is bound and the first + // stack is live. Only meaningful with a graphFactory (no factory → reinitialize is a no-op). + await this.#startDefinitionWatcher(graph); + + announceListening({port, h2, acceptRemoteConnections}); + } + + // Creates a definition watcher over the given graph and wires it to reinitialize(). A no-op + // without a graphFactory, since reinitialize() cannot re-resolve the graph without one. + async #startDefinitionWatcher(graph) { + if (!this.#graphFactory) { + return; + } + const {rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd} = this.#config; + const watcher = await ProjectDefinitionWatcher.create({ + graph, rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd, + }); + watcher.on("definitionChanged", () => this.reinitialize()); + // Leading edge of a definition-file burst: a re-resolve (and a likely version change) is + // coming. Blank the interactive console's version slot so the Project region shows a + // "resolving…" placeholder until the swap's own resolve repopulates it via + // `ui5.project-resolve-succeeded` (or a failed swap releases it; see #swap). Attached here so it + // survives watcher re-targeting after each swap, mirroring the definitionChanged wiring. + watcher.on("definitionChanging", () => process.emit("ui5.project-resolve-started")); + watcher.on("error", (err) => log.warn(`Definition watcher error: ${err?.message ?? err}`)); + this.#definitionWatcher = watcher; + } + + // Forwards the current BuildServer's sourcesChanged onto the stable relay. Detaches any + // previous subscription first so a swapped-out BuildServer stops driving live-reload. + #relayFrom(buildServer) { + this.#detachRelay(); + const onSourcesChanged = () => this.#sourcesChangedRelay.emit("sourcesChanged"); + buildServer.on("sourcesChanged", onSourcesChanged); + this.#relayUnsubscribe = () => buildServer.off("sourcesChanged", onSourcesChanged); + } + + #detachRelay() { + if (this.#relayUnsubscribe) { + this.#relayUnsubscribe(); + this.#relayUnsubscribe = null; + } + } + + /** + * Re-resolves the graph and re-creates the serving stack behind the stable HTTP server. + * + * Build-new-then-swap: on a resolution/build failure the previous stack keeps serving and + * the error is logged (never emitted as a fatal "error"). Overlapping calls + * collapse into a single trailing pass. + * + * @returns {Promise} Resolves once the swap (or the no-op) completes + */ + async reinitialize() { + if (this.#destroyed) { + return; + } + if (!this.#graphFactory) { + log.warn("Cannot re-initialize server: no graph factory was provided"); + return; + } + if (this.#reinitInProgress) { + // Collapse overlapping requests into one trailing pass against the settled definition. + // The version slot stays on the "resolving…" placeholder (armed by definitionChanging) + // until the trailing pass resolves and repaints it via `ui5.project-resolve-succeeded`. + this.#reinitQueued = true; + return; + } + this.#reinitInProgress = true; + try { + do { + this.#reinitQueued = false; + await this.#swap(); + } while (this.#reinitQueued && !this.#destroyed); + } finally { + this.#reinitInProgress = false; + } + } + + async #swap() { + const oldStack = this.#stack; + let newStack; + let newGraph; + try { + newGraph = await this.#graphFactory(); + newStack = await buildApp(newGraph, this.#config, this.#error); + } catch (err) { + // Keep the last-good stack serving. A subsequent valid edit will swap cleanly. + log.error(`Failed to re-initialize server: ${err?.message ?? err}`); + if (err?.stack) { + log.verbose(err.stack); + } + // A failed resolve never emits `ui5.project-resolve-succeeded`, so the version slot would keep + // the "resolving…" placeholder indefinitely. Release it back to the last-known version + // the old (still-serving) stack resolved. Harmless if the failure was in buildApp, + // where the resolve already repainted the slot. + process.emit("ui5.project-resolve-failed"); + return; + } + if (this.#destroyed) { + // Destroyed while building: discard the new stack instead of adopting it. + await newStack.buildServer.destroy(); + return; + } + // Swap: retarget the trampoline, move live-reload to the new BuildServer, notify clients. + this.#stack = newStack; + this.#relayFrom(newStack.buildServer); + this.#sourcesChangedRelay.emit("sourcesChanged"); + // Re-target the definition watcher to the new graph: the project set or their roots may + // have changed. A create failure here must not crash the swap; keep serving and log, + // mirroring the build-failure path above, so the old watcher keeps driving re-inits. + const oldWatcher = this.#definitionWatcher; + this.#definitionWatcher = null; + try { + await this.#startDefinitionWatcher(newGraph); + await oldWatcher?.destroy(); + } catch (err) { + log.warn(`Failed to re-target definition watcher: ${err?.message ?? err}`); + // Keep the old watcher driving re-inits if the new one failed to arm. + this.#definitionWatcher ??= oldWatcher; + } + // Tear down the old stack. Its BuildServer releases the source watcher and the cache + // handle; the new BuildServer already reopened the same (refcounted) cache. + await oldStack.buildServer.destroy(); + } + + getPort() { + return this.#port; + } + + /** + * Stops the server: closes live-reload, the HTTP socket, and the current BuildServer. + * Mirrors the tolerant teardown of the single-shot wrapper: the socket is closed even if + * the BuildServer's destroy rejects. + * + * @param {Function} [callback] Invoked once the HTTP server has closed + * @returns {Promise} Resolves once teardown completes + */ + async destroy(callback) { + this.#destroyed = true; + // Stop the definition watcher early so a late event cannot start a re-init mid-teardown. + // The reinitialize() #destroyed guard already no-ops such an event; this is defensive. + const definitionWatcher = this.#definitionWatcher; + this.#definitionWatcher = null; + this.#liveReloadHandle?.close(); + this.#detachRelay(); + this.#httpServer?.close(callback); + try { + await definitionWatcher?.destroy(); + } catch (err) { + log.verbose(`Error while destroying definition watcher: ${err?.message ?? err}`); + } + try { + await this.#stack?.buildServer.destroy(); + } catch (err) { + log.verbose(`Error while destroying BuildServer: ${err?.message ?? err}`); + } + } +} + +export default Supervisor; diff --git a/packages/server/lib/serve/httpListener.js b/packages/server/lib/serve/httpListener.js new file mode 100644 index 00000000000..03b264086b6 --- /dev/null +++ b/packages/server/lib/serve/httpListener.js @@ -0,0 +1,121 @@ +import os from "node:os"; +import portscanner from "portscanner"; + +/** + * HTTP-listener helpers shared between the single-shot {@link module:@ui5/server.serve} + * wrapper and the {@link Supervisor}, which binds the port once and swaps the + * request handler behind it. + * + * @private + * @module @ui5/server/serve/httpListener + */ + +/** + * Binds an HTTP/HTTPS server to a free port and resolves once it is listening. + * + * @param {object} app The express application (or spdy server) to listen with + * @param {number} port Desired port to listen to + * @param {boolean} changePortIfInUse If true and the port is already in use, an unused port is searched + * @param {boolean} acceptRemoteConnections If true, listens to remote connections and not only to localhost + * @returns {Promise} Resolves with the bound port and the server instance + * @private + */ +export function listen(app, port, changePortIfInUse, acceptRemoteConnections) { + return new Promise(function(resolve, reject) { + const options = {}; + + if (!acceptRemoteConnections) { + // Unless remote connections are allowed, bind to the IPv4 loopback address + options.host = "127.0.0.1"; + } // If remote connections are allowed, do not set host so the server listens on all supported interfaces + + const portScanHost = options.host || "127.0.0.1"; + const portMax = changePortIfInUse ? port + 30 : port; + + portscanner.findAPortNotInUse(port, portMax, portScanHost, function(error, foundPort) { + if (error) { + reject(error); + return; + } + + if (!foundPort) { + const err = new Error(changePortIfInUse ? + `EADDRINUSE: Could not find available ports between ${port} and ${portMax}.` : + `EADDRINUSE: Port ${port} is already in use.`); + err.code = "EADDRINUSE"; + err.errno = "EADDRINUSE"; + err.address = portScanHost; + err.port = portMax; + reject(err); + return; + } + + options.port = foundPort; + const server = app.listen(options, function() { + resolve({port: options.port, server}); + }); + + server.on("error", function(err) { + reject(err); + }); + }); + }); +} + +/** + * Adds SSL support to an express application. + * + * @param {object} parameters + * @param {object} parameters.app The original express application + * @param {string} parameters.key Path to private key to be used for https + * @param {string} parameters.cert Path to certificate to be used for for https + * @returns {Promise} The express application with SSL support + * @private + */ +export async function addSsl({app, key, cert}) { + // Using spdy as http2 server as the native http2 implementation + // from Node v8.4.0 doesn't seem to work with express + const {default: spdy} = await import("spdy"); + return spdy.createServer({cert, key}, app); +} + +/** + * Announces the bound URLs on the event bus. The server owns the network-interface lookup + * because it knows the actual bound port (which may differ from the requested one when + * changePortIfInUse is set). Consumers (@ui5/logger writers) shape their own display from the + * label/url pairs. + * + * @param {object} parameters + * @param {number} parameters.port The actual bound port + * @param {boolean} parameters.h2 Whether HTTP/2 (https) is in use + * @param {boolean} parameters.acceptRemoteConnections Whether the server binds to all interfaces + * @private + */ +export function announceListening({port, h2, acceptRemoteConnections}) { + const protocol = h2 ? "https" : "http"; + const urls = [{label: "Local", url: `${protocol}://localhost:${port}`}]; + if (acceptRemoteConnections) { + for (const addr of findNetworkInterfaceAddresses()) { + urls.push({label: "Network", url: `${protocol}://${addr}:${port}`}); + } + } + process.emit("ui5.server-listening", { + urls, + acceptRemoteConnections: !!acceptRemoteConnections, + }); +} + +// Collects all non-internal IPv4 addresses so `ui5.server-listening` can list every reachable +// URL when the server binds to all interfaces. Empty array if none is found. +function findNetworkInterfaceAddresses() { + const interfaces = os.networkInterfaces(); + const addresses = []; + for (const name of Object.keys(interfaces)) { + for (const iface of interfaces[name] ?? []) { + if (iface.family === "IPv4" && !iface.internal) { + addresses.push(iface.address); + } + } + } + return addresses; +} diff --git a/packages/server/lib/serve/stack.js b/packages/server/lib/serve/stack.js new file mode 100644 index 00000000000..53b71305739 --- /dev/null +++ b/packages/server/lib/serve/stack.js @@ -0,0 +1,157 @@ +import express from "express"; +import MiddlewareManager from "../middleware/MiddlewareManager.js"; +import createErrorHandler from "../middleware/errorHandler.js"; +import {createReaderCollection} from "@ui5/fs/resourceFactory"; +import ReaderCollectionPrioritized from "@ui5/fs/ReaderCollectionPrioritized"; +import {getLogger} from "@ui5/logger"; +import Cache from "@ui5/project/build/cache/Cache"; + +const log = getLogger("server"); + +/** + * Builds the middleware-bearing router and its BuildServer for a project graph, without + * binding to a port and without a terminal error handler. This is the router-agnostic core + * shared between the {@link module:@ui5/server.serve} wrapper (via {@link buildApp}) and + * the {@link module:@ui5/server.serveMiddleware} embedding API: everything needed to answer + * requests is mounted onto an {@link https://expressjs.com/en/4x/api.html#router Express Router}, + * which both an Express application and a Connect app accept via use(). + * + * @private + * @param {@ui5/project/graph/ProjectGraph} graph Project graph + * @param {object} config Server configuration: the resolved options + * (sendSAPTargetCSP, simpleIndex, liveReload, + * serveCSPReports, cache, ui5DataDir, + * includedTasks, excludedTasks), plus a stable + * webSocketToken shared across swaps + * @param {Function} [error] Error callback invoked when the BuildServer emits an error outside + * of request handling + * @returns {Promise} Resolves with {router, buildServer, liveReloadOptions} + */ +export async function buildRouter(graph, config, error) { + const { + sendSAPTargetCSP = false, simpleIndex = false, liveReload = false, serveCSPReports = false, + cache = Cache.Default, ui5DataDir, includedTasks, excludedTasks, webSocketToken = null, + } = config; + const rootProject = graph.getRoot(); + + const readers = []; + await graph.traverseBreadthFirst(async function({project: dep}) { + if (dep.getName() === rootProject.getName()) { + // Ignore root project + return; + } + readers.push(dep.getSourceReader("runtime")); + }); + + const dependencies = createReaderCollection({ + name: `Dependency reader collection for sources of project ${rootProject.getName()}`, + readers + }); + + const rootReader = rootProject.getSourceReader("runtime"); + + // TODO change to ReaderCollection once duplicates are sorted out + const combo = new ReaderCollectionPrioritized({ + name: "Server: Reader for sources of all projects", + readers: [rootReader, dependencies] + }); + const sources = { + rootProject: rootReader, + dependencies: dependencies, + all: combo + }; + + const initialBuildIncludedDependencies = []; + if (graph.getProject("sap.ui.core")) { + // Ensure sap.ui.core is always built initially (if present in the graph) + initialBuildIncludedDependencies.push("sap.ui.core"); + } + const buildServer = await graph.serve({ + initialBuildIncludedDependencies, + includedTasks, + excludedTasks, + cache, + ui5DataDir, + }); + + // graph.serve() above already started the BuildServer, which holds a source watcher and a + // build-cache handle. If middleware assembly below throws, the caller gets neither the + // buildServer nor a close() handle, so destroy it here before rethrowing. + try { + const resources = { + rootProject: buildServer.getRootReader(), + dependencies: buildServer.getDependenciesReader(), + all: buildServer.getReader(), + }; + + buildServer.on("error", (err) => { + if (typeof error === "function") { + error(err); + return; + } + log.error(`BuildServer error: ${err?.message ?? err}`); + if (err?.stack) { + log.verbose(err.stack); + } + }); + + const liveReloadOptions = { + active: liveReload, + token: webSocketToken + }; + + const middlewareManager = new MiddlewareManager({ + graph, + rootProject, + sources, + resources, + options: { + sendSAPTargetCSP, + serveCSPReports, + simpleIndex, + liveReload: liveReloadOptions, + // Consulted by the serveBuildError gate to divert HTML navigations while the + // build server is globally in ERROR. + getServeError: () => buildServer.getServeError() + } + }); + + // eslint-disable-next-line new-cap -- express.Router is a factory, not a constructor + const router = express.Router(); + await middlewareManager.applyMiddleware(router); + return {router, buildServer, liveReloadOptions}; + } catch (err) { + await buildServer.destroy(); + throw err; + } +} + +/** + * Builds the Express application and its BuildServer for a project graph, without binding + * to a port. Wraps {@link buildRouter} with an {@link express} application and the + * terminal error handler, so the {@link module:@ui5/server.serve} wrapper and the + * {@link Supervisor} can own the listener and re-create the app behind a stable HTTP + * server on a graph swap. The error handler and the live-reload WebSocket server are + * CLI-owned concerns and stay out of the router core used by the embedding API. + * + * @private + * @module @ui5/server/serve/stack + * @param {@ui5/project/graph/ProjectGraph} graph Project graph + * @param {object} config Server configuration; see {@link buildRouter} + * @param {Function} [error] Error callback invoked when the BuildServer emits an error outside + * of request handling + * @returns {Promise} Resolves with {app, buildServer, liveReloadOptions} + */ +export default async function buildApp(graph, config, error) { + const {router, buildServer, liveReloadOptions} = await buildRouter(graph, config, error); + + const app = express(); + app.use(router); + // Terminal error handler for the middleware chain. Registered after the router so it sits + // last and intercepts every next(err), including those from custom middleware we can't wrap + // in a try/catch. Threads the live-reload config in so the HTML error page can embed the + // live-reload client script. + app.use(createErrorHandler({liveReload: liveReloadOptions})); + + return {app, buildServer, liveReloadOptions}; +} diff --git a/packages/server/lib/serveMiddleware.js b/packages/server/lib/serveMiddleware.js new file mode 100644 index 00000000000..b40e17a7761 --- /dev/null +++ b/packages/server/lib/serveMiddleware.js @@ -0,0 +1,87 @@ +import Cache from "@ui5/project/build/cache/Cache"; +import {buildRouter} from "./serve/stack.js"; + +/** + * Assembles the UI5 middleware for a project graph and returns it as a single connect/Express + * middleware, for mounting into an existing HTTP server rather than starting one. + * + * Unlike {@link module:@ui5/server.serve}, this does not bind a port, attach the live-reload + * WebSocket server, or install the terminal HTML error handler; those belong to the server the + * UI5 CLI owns. The caller mounts the returned middleware on its own + * express() app or Express Router/Connect app via + * app.use(middleware) and owns error handling and the HTTP listener. + * + * close must be called on teardown to release the BuildServer's source watcher and + * build-cache handle. + * + * Note: A project graph can be served only once. Do not call both serveMiddleware + * and {@link module:@ui5/server.serve} for the same graph. + * + * @example + * import express from "express"; + * import {serveMiddleware} from "@ui5/server"; + * + * const app = express(); + * const {middleware, close} = await serveMiddleware(graph); + * app.use(middleware); + * const listener = app.listen(8080); + * + * // On teardown, release the BuildServer's watcher and build-cache handle. + * listener.close(); + * await close(); + * + * @public + * @alias module:@ui5/server.serveMiddleware + * @param {@ui5/project/graph/ProjectGraph} graph Project graph + * @param {object} [options] Options + * @param {boolean|module:@ui5/server.SAPTargetCSPOptions} [options.sendSAPTargetCSP=false] + * If set to true or an object, then the default (or configured) + * set of security policies that SAP and UI5 aim for (AKA 'target policies'), + * are send for any requested *.html file + * @param {boolean} [options.serveCSPReports=false] Enable CSP reports serving for request url + * '/.ui5/csp/csp-reports.json' + * @param {boolean} [options.simpleIndex=false] Use a simplified view for the server directory listing + * @param {string} [options.cache="Default"] Cache mode to use for building UI5 projects. + * @param {string} [options.ui5DataDir] Explicit UI5 data directory to use for the build cache. + * Overrides the UI5_DATA_DIR environment variable, + * the UI5 configuration file, and the default of ~/.ui5. + * @param {string[]} [options.includedTasks] A list of tasks to be added to the default execution set. + * Takes precedence over excludedTasks. + * @param {string[]} [options.excludedTasks] A list of tasks to be excluded from the default task + * execution set. + * @param {Function} [error] Error callback. Will be called when the BuildServer emits an error + * outside of request handling. + * @returns {Promise} Promise resolving with an object containing the + * middleware (a connect/Express-compatible handler to be + * mounted via app.use()), the buildServer, and a + * close function releasing the BuildServer's watcher and cache. + */ +export default async function serveMiddleware(graph, { + sendSAPTargetCSP = false, simpleIndex = false, serveCSPReports = false, cache = Cache.Default, + ui5DataDir, includedTasks, excludedTasks, +} = {}, error) { + const config = { + sendSAPTargetCSP, simpleIndex, serveCSPReports, cache, + ui5DataDir, includedTasks, excludedTasks, + // The live-reload WebSocket server belongs to the UI5 CLI's HTTP server, which this + // embedding API does not create. Inactive with no token leaves the liveReloadClient + // middleware in place but dormant (no client script injection). + liveReload: false, + webSocketToken: null, + }; + + const {router, buildServer} = await buildRouter(graph, config, error); + + let destroyed = false; + return { + middleware: router, + buildServer, + close: async function close() { + if (destroyed) { + return; + } + destroyed = true; + await buildServer.destroy(); + }, + }; +} diff --git a/packages/server/lib/server.js b/packages/server/lib/server.js index 201b9f3a485..50165834345 100644 --- a/packages/server/lib/server.js +++ b/packages/server/lib/server.js @@ -1,14 +1,6 @@ import {getRandomValues} from "node:crypto"; -import os from "node:os"; -import process from "node:process"; -import express from "express"; -import portscanner from "portscanner"; -import MiddlewareManager from "./middleware/MiddlewareManager.js"; -import createErrorHandler from "./middleware/errorHandler.js"; -import attachLiveReloadServer from "./liveReload/server.js"; -import {createReaderCollection} from "@ui5/fs/resourceFactory"; -import ReaderCollectionPrioritized from "@ui5/fs/ReaderCollectionPrioritized"; import {getLogger} from "@ui5/logger"; +import Supervisor from "./serve/Supervisor.js"; const log = getLogger("server"); /** @@ -16,90 +8,6 @@ const log = getLogger("server"); * @module @ui5/server */ -/** - * Returns a promise resolving by starting the server. - * - * @param {object} app The express application object - * @param {number} port Desired port to listen to - * @param {boolean} changePortIfInUse If true and the port is already in use, an unused port is searched - * @param {boolean} acceptRemoteConnections If true, listens to remote connections and not only to localhost connections - * @returns {Promise} Returns an object containing server related information like (selected port, protocol) - * @private - */ -function _listen(app, port, changePortIfInUse, acceptRemoteConnections) { - return new Promise(function(resolve, reject) { - const options = {}; - - if (!acceptRemoteConnections) { - // Unless remote connections are allowed, bind to the IPv4 loopback address - options.host = "127.0.0.1"; - } // If remote connections are allowed, do not set host so the server listens on all supported interfaces - - const portScanHost = options.host || "127.0.0.1"; - let portMax; - if (changePortIfInUse) { - portMax = port + 30; - } else { - portMax = port; - } - - portscanner.findAPortNotInUse(port, portMax, portScanHost, function(error, foundPort) { - if (error) { - reject(error); - return; - } - - if (!foundPort) { - if (changePortIfInUse) { - const error = new Error( - `EADDRINUSE: Could not find available ports between ${port} and ${portMax}.`); - error.code = "EADDRINUSE"; - error.errno = "EADDRINUSE"; - error.address = portScanHost; - error.port = portMax; - reject(error); - return; - } else { - const error = new Error(`EADDRINUSE: Port ${port} is already in use.`); - error.code = "EADDRINUSE"; - error.errno = "EADDRINUSE"; - error.address = portScanHost; - error.port = portMax; - reject(error); - return; - } - } - - options.port = foundPort; - const server = app.listen(options, function() { - resolve({port: options.port, server}); - }); - - server.on("error", function(err) { - reject(err); - }); - }); - }); -} - -/** - * Adds SSL support to an express application. - * - * @param {object} parameters - * @param {object} parameters.app The original express application - * @param {string} parameters.key Path to private key to be used for https - * @param {string} parameters.cert Path to certificate to be used for for https - * @returns {Promise} The express application with SSL support - * @private - */ -async function _addSsl({app, key, cert}) { - // Using spdy as http2 server as the native http2 implementation - // from Node v8.4.0 doesn't seem to work with express - const {default: spdy} = await import("spdy"); - return spdy.createServer({cert, key}, app); -} - - /** * SAP target CSP middleware options * @@ -142,178 +50,69 @@ async function _addSsl({app, key, cert}) { * Takes precedence over excludedTasks. * @param {string[]} [options.excludedTasks] A list of tasks to be excluded from the default task * execution set. + * @param {string} [options.rootConfigPath] Custom config path for the root project (from --config), + * threaded to the definition watcher so it watches the right file. + * @param {string} [options.workspaceConfigPath] Workspace config path (default ui5-workspace.yaml); + * threaded to the definition watcher. Omit in static-graph mode. + * @param {string} [options.dependencyDefinitionPath] Static dependency-definition file + * (from --dependency-definition); watched when present. + * @param {string} [options.cwd] Base directory for resolving the watcher's relative paths. * @param {Function} error Error callback. Will be called when an error occurs outside of request handling. + * @param {object} [options2] Additional options + * @param {Function} [options2.graphFactory] Async factory that re-resolves the project graph with the + * same parameters used to build the initial graph. When provided, + * the returned reinitialize re-creates the serving stack on a + * project-definition change. Omitted → reinitialize is a no-op. * @returns {Promise} Promise resolving once the server is listening. * It resolves with an object containing the port, - * h2-flag and a close function, - * which can be used to stop the server. + * h2-flag, a close function to stop the server, + * and a reinitialize function to re-create the serving stack. */ export async function serve(graph, { - port: requestedPort, changePortIfInUse = false, h2 = false, key, cert, + port, changePortIfInUse = false, h2 = false, key, cert, acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false, liveReload = false, serveCSPReports = false, cache = "Default", ui5DataDir, includedTasks, excludedTasks, -}, error) { - const rootProject = graph.getRoot(); - - const readers = []; - await graph.traverseBreadthFirst(async function({project: dep}) { - if (dep.getName() === rootProject.getName()) { - // Ignore root project - return; - } - readers.push(dep.getSourceReader("runtime")); - }); - - const dependencies = createReaderCollection({ - name: `Dependency reader collection for sources of project ${rootProject.getName()}`, - readers - }); - - const rootReader = rootProject.getSourceReader("runtime"); - - // TODO change to ReaderCollection once duplicates are sorted out - const combo = new ReaderCollectionPrioritized({ - name: "Server: Reader for sources of all projects", - readers: [rootReader, dependencies] - }); - const sources = { - rootProject: rootReader, - dependencies: dependencies, - all: combo - }; - - const initialBuildIncludedDependencies = []; - if (graph.getProject("sap.ui.core")) { - // Ensure sap.ui.core is always built initially (if present in the graph) - initialBuildIncludedDependencies.push("sap.ui.core"); - } - const buildServer = await graph.serve({ - initialBuildIncludedDependencies, - includedTasks, - excludedTasks, - cache, - ui5DataDir, - }); - - const resources = { - rootProject: buildServer.getRootReader(), - dependencies: buildServer.getDependenciesReader(), - all: buildServer.getReader(), - }; - - buildServer.on("error", (err) => { - if (typeof error === "function") { - error(err); - return; - } - log.error(`BuildServer error: ${err?.message ?? err}`); - if (err?.stack) { - log.verbose(err.stack); - } - }); - - // Random 72 bits (9 * 8 bits), base64url-encoded to a 12-character string, should be sufficient for uniqueness. - // OWASP recommends at least 64 bits of entropy for session IDs: + rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd, +}, error, {graphFactory} = {}) { + // The live-reload token is generated once and shared with every serving stack the supervisor + // builds, so connected clients keep authenticating across a re-initialization. + // Random 72 bits (9 * 8 bits), base64url-encoded to a 12-character string. OWASP recommends + // at least 64 bits of entropy for session IDs: // https://owasp.org/www-community/vulnerabilities/Insufficient_Session-ID_Length const webSocketToken = liveReload ? Buffer.from(getRandomValues(new Uint8Array(9))).toString("base64url") : null; - const liveReloadOptions = { - active: liveReload, - token: webSocketToken + const config = { + port, changePortIfInUse, h2, key, cert, + acceptRemoteConnections, sendSAPTargetCSP, + simpleIndex, liveReload, serveCSPReports, cache, + ui5DataDir, includedTasks, excludedTasks, webSocketToken, + rootConfigPath, workspaceConfigPath, dependencyDefinitionPath, cwd, }; - const middlewareManager = new MiddlewareManager({ - graph, - rootProject, - sources, - resources, - options: { - sendSAPTargetCSP, - serveCSPReports, - simpleIndex, - liveReload: liveReloadOptions, - // Consulted by the serveBuildError gate to divert HTML navigations while the - // build server is globally in ERROR. - getServeError: () => buildServer.getServeError() - } - }); - - let app = express(); - await middlewareManager.applyMiddleware(app); - // Terminal error handler for the middleware chain. Registered after applyMiddleware - // so it sits last and intercepts every next(err) — including those from custom - // middleware where we can't wrap a try/catch. Threads the live-reload config in - // so the HTML error page can embed the live-reload client script. - app.use(createErrorHandler({liveReload: liveReloadOptions})); - - if (h2) { - const nodeVersion = parseInt(process.versions.node.split(".")[0], 10); - if (nodeVersion >= 24) { - log.error("ERROR: With Node v24, usage of HTTP/2 is no longer supported. Please check https://github.com/UI5/cli/issues/327 for updates."); - process.exit(1); - } - - app = await _addSsl({app, key, cert}); - } - - let port; let server; + let supervisor; try { - ({port, server} = await _listen(app, requestedPort, changePortIfInUse, acceptRemoteConnections)); + supervisor = await Supervisor.create(graph, config, error, {graphFactory}); } catch (err) { - await buildServer.destroy(); + log.verbose(`Failed to start server: ${err?.message ?? err}`); throw err; } - let liveReloadHandle; - if (liveReload) { - liveReloadHandle = attachLiveReloadServer({httpServer: server, buildServer, token: webSocketToken}); - } - - // Announce the bound URLs on the event bus. The server owns the network- - // interface lookup because it knows the actual bound port (which may - // differ from the requested one when changePortIfInUse is set). Consumers - // (@ui5/logger writers) shape their own display from the label/url pairs. - const protocol = h2 ? "https" : "http"; - const urls = [{label: "Local", url: `${protocol}://localhost:${port}`}]; - if (acceptRemoteConnections) { - for (const addr of _findNetworkInterfaceAddresses()) { - urls.push({label: "Network", url: `${protocol}://${addr}:${port}`}); - } - } - process.emit("ui5.server-listening", { - urls, - acceptRemoteConnections: !!acceptRemoteConnections, - }); - return { h2, - port, + port: supervisor.getPort(), close: function(callback) { - liveReloadHandle?.close(); - buildServer.destroy().then(() => { - server.close(callback); - }, () => { - server.close(callback); - }); - } + supervisor.destroy(callback); + }, + reinitialize: function() { + return supervisor.reinitialize(); + }, }; } -// Collects all non-internal IPv4 addresses from the host's network interfaces -// so `ui5.server-listening` can list every reachable URL when the server binds -// to all interfaces. Returns an empty array if no suitable address is found. -function _findNetworkInterfaceAddresses() { - const interfaces = os.networkInterfaces(); - const addresses = []; - for (const name of Object.keys(interfaces)) { - for (const iface of interfaces[name] ?? []) { - if (iface.family === "IPv4" && !iface.internal) { - addresses.push(iface.address); - } - } - } - return addresses; -} +// Public API for integrating UI5's middleware into an existing HTTP server. Re-exported here +// so it is reachable via the package's main entry alongside serve(); the @public JSDoc lives +// on the function itself in serveMiddleware.js. +export {default as serveMiddleware} from "./serveMiddleware.js"; diff --git a/packages/server/test/lib/package-exports.js b/packages/server/test/lib/package-exports.js index 076fe12bb93..61d94e289b9 100644 --- a/packages/server/test/lib/package-exports.js +++ b/packages/server/test/lib/package-exports.js @@ -20,6 +20,8 @@ test("@ui5/server", async (t) => { const actual = await import("@ui5/server"); const expected = await import("../../lib/server.js"); t.is(actual, expected, "Correct module exported"); + t.is(typeof actual.serve, "function", "serve is exported"); + t.is(typeof actual.serveMiddleware, "function", "serveMiddleware is exported"); }); // Internal modules (only to be used by @ui5/* / SAP owned packages) diff --git a/packages/server/test/lib/server/reinitialize.js b/packages/server/test/lib/server/reinitialize.js new file mode 100644 index 00000000000..e26ed81adda --- /dev/null +++ b/packages/server/test/lib/server/reinitialize.js @@ -0,0 +1,116 @@ +import test from "ava"; +import supertest from "supertest"; +import fs from "node:fs/promises"; +import path from "node:path"; +import {serve} from "../../../lib/server.js"; +import {graphFromPackageDependencies} from "@ui5/project/graph"; +import {isolatedUi5DataDir} from "../../utils/buildCacheIsolation.js"; + +// Integration coverage for the Supervisor swap against a real graph + BuildServer: the port +// stays bound, the same socket keeps serving, and a re-init reuses the (config-keyed, refcounted) +// build cache rather than cold-rebuilding. Uses a fixed cwd so the graphFactory re-resolves the +// same project. + +async function buildGraph() { + return graphFromPackageDependencies({ + cwd: "./test/fixtures/application.a" + }); +} + +test.serial("reinitialize() keeps the port bound and continues serving", async (t) => { + const ui5DataDir = isolatedUi5DataDir(t); + const graph = await buildGraph(); + const server = await serve(graph, { + port: 3395, + changePortIfInUse: true, + liveReload: false, + ui5DataDir, + }, undefined, {graphFactory: buildGraph}); + + const port = server.port; + const request = supertest(`http://127.0.0.1:${port}`); + + const before = await request.get("/index.html"); + t.is(before.statusCode, 200, "serves before re-init"); + + await server.reinitialize(); + + // Same port, same socket — the request client is unchanged. + const after = await request.get("/index.html"); + t.is(after.statusCode, 200, "serves after re-init on the same port"); + t.is(server.port, port, "the bound port is unchanged across re-init"); + + await new Promise((resolve) => server.close(resolve)); +}); + +// Integration coverage for the ProjectDefinitionWatcher trigger: editing a project's ui5.yaml on disk +// drives a real re-init through the watcher (not a programmatic reinitialize() call). Runs against +// a fixture copied to test/tmp so the edit does not mutate the checked-in fixture. +test.serial("editing ui5.yaml triggers a re-init and keeps serving on the same port", async (t) => { + const ui5DataDir = isolatedUi5DataDir(t); + const tmpProject = path.join("./test/tmp", `reinitialize-watcher-${process.pid}`); + await fs.rm(tmpProject, {recursive: true, force: true}); + await fs.cp("./test/fixtures/application.a", tmpProject, {recursive: true}); + + const buildTmpGraph = () => graphFromPackageDependencies({cwd: tmpProject}); + const graph = await buildTmpGraph(); + + // Count re-resolutions so we can await the watcher-driven one. + let graphBuilds = 0; + const graphFactory = async () => { + graphBuilds++; + return buildTmpGraph(); + }; + + const server = await serve(graph, { + port: 3397, + changePortIfInUse: true, + liveReload: false, + ui5DataDir, + cwd: tmpProject, + }, undefined, {graphFactory}); + + const port = server.port; + const request = supertest(`http://127.0.0.1:${port}`); + t.is((await request.get("/index.html")).statusCode, 200, "serves before the edit"); + + // Edit ui5.yaml on disk. The watcher's settle window (550 ms) then coalesces the change and + // drives a re-init through the graphFactory. + const ui5YamlPath = path.join(tmpProject, "ui5.yaml"); + const original = await fs.readFile(ui5YamlPath, "utf8"); + await fs.writeFile(ui5YamlPath, original + "\n# watcher-triggered edit\n"); + + // Wait for the watcher to observe the change and complete the re-init (settle + graph rebuild). + const deadline = Date.now() + 15000; + while (graphBuilds === 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 100)); + } + t.true(graphBuilds >= 1, "the ui5.yaml edit drove at least one re-init via the watcher"); + + const after = await request.get("/index.html"); + t.is(after.statusCode, 200, "serves after the watcher-driven re-init"); + t.is(server.port, port, "the bound port is unchanged"); + + await new Promise((resolve) => server.close(resolve)); + await fs.rm(tmpProject, {recursive: true, force: true}); +}); + +test.serial("reinitialize() without a graphFactory is a no-op and keeps serving", async (t) => { + const ui5DataDir = isolatedUi5DataDir(t); + const graph = await buildGraph(); + // No 4th argument → no graphFactory. + const server = await serve(graph, { + port: 3396, + changePortIfInUse: true, + liveReload: false, + ui5DataDir, + }); + + const request = supertest(`http://127.0.0.1:${server.port}`); + await server.reinitialize(); // warns + no-ops + + const result = await request.get("/index.html"); + t.is(result.statusCode, 200, "still serving after a no-op re-init"); + + await new Promise((resolve) => server.close(resolve)); +}); diff --git a/packages/server/test/lib/server/serve/Supervisor.js b/packages/server/test/lib/server/serve/Supervisor.js new file mode 100644 index 00000000000..55d3bf3a9ff --- /dev/null +++ b/packages/server/test/lib/server/serve/Supervisor.js @@ -0,0 +1,487 @@ +import test from "ava"; +import sinon from "sinon"; +import esmock from "esmock"; +import {EventEmitter} from "node:events"; + +// A fake BuildServer: EventEmitter plus the reader/error/destroy surface the supervisor uses. +function createBuildServer() { + const buildServer = new EventEmitter(); + buildServer.getServeError = sinon.stub().returns(null); + buildServer.destroy = sinon.stub().resolves(); + return buildServer; +} + +// Builds a mock set for esmock. Each buildApp invocation returns the next queued stack, so a +// test can hand out distinct {app, buildServer} pairs across the initial build and re-inits. +function createMocks({stacks, buildAppImpl, definitionWatcherCreate} = {}) { + const httpServer = new EventEmitter(); + httpServer.close = sinon.stub().callsFake((cb) => cb && cb()); + + const createdHandlers = []; + const listen = sinon.stub().resolves({port: 3000, server: httpServer}); + const addSsl = sinon.stub().callsFake(async ({app}) => app); + const announceListening = sinon.stub(); + + const liveReloadHandle = {close: sinon.stub()}; + const attachLiveReloadServer = sinon.stub().returns(liveReloadHandle); + + // Fake ProjectDefinitionWatcher: each create() hands out a fresh EventEmitter with a destroy() stub, + // recorded so a test can assert re-targeting/teardown. A custom impl overrides create(). + const definitionWatchers = []; + const ProjectDefinitionWatcher = { + create: sinon.stub().callsFake(async (opts) => { + if (definitionWatcherCreate) { + return definitionWatcherCreate(opts); + } + const watcher = new EventEmitter(); + watcher.createOptions = opts; + watcher.destroy = sinon.stub().resolves(); + definitionWatchers.push(watcher); + return watcher; + }), + }; + + const stackQueue = stacks ? [...stacks] : null; + const buildApp = sinon.stub().callsFake(async (graph, config, error) => { + if (buildAppImpl) { + return buildAppImpl(graph, config, error); + } + return stackQueue.shift(); + }); + + const httpMock = { + default: { + createServer: sinon.stub().callsFake((handler) => { + createdHandlers.push(handler); + return httpServer; + }) + } + }; + + const mocks = { + "node:http": httpMock, + "@ui5/project/graph/ProjectDefinitionWatcher": {default: ProjectDefinitionWatcher}, + "../../../../lib/serve/stack.js": {default: buildApp}, + "../../../../lib/serve/httpListener.js": {listen, addSsl, announceListening}, + "../../../../lib/liveReload/server.js": {default: attachLiveReloadServer}, + }; + + return { + mocks, httpServer, listen, addSsl, announceListening, + attachLiveReloadServer, liveReloadHandle, buildApp, createdHandlers, + ProjectDefinitionWatcher, definitionWatchers, + }; +} + +function createStack(app) { + return { + app: app ?? sinon.stub(), + buildServer: createBuildServer(), + liveReloadOptions: {active: true, token: "tok"}, + }; +} + +async function importSupervisor(mocks) { + return esmock("../../../../lib/serve/Supervisor.js", mocks); +} + +const baseConfig = {port: 3000, liveReload: true, webSocketToken: "tok"}; + +test.afterEach.always(() => { + sinon.restore(); +}); + +test("create() builds the initial stack, binds once, attaches live-reload to a stable relay", async (t) => { + const stack = createStack(); + const {mocks, listen, attachLiveReloadServer, buildApp} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {}); + + t.is(supervisor.getPort(), 3000); + t.true(buildApp.calledOnce); + t.true(listen.calledOnce, "port is bound exactly once"); + // Live-reload is attached to the stable relay, not the BuildServer directly. + t.true(attachLiveReloadServer.calledOnce); + const {buildServer: relay} = attachLiveReloadServer.firstCall.args[0]; + t.not(relay, stack.buildServer, "live-reload subscribes to the relay, not the BuildServer"); + t.true(relay instanceof EventEmitter); +}); + +test("request trampoline retargets to the swapped app after reinitialize()", async (t) => { + const app1 = sinon.stub(); + const app2 = sinon.stub(); + const stack1 = createStack(app1); + const stack2 = createStack(app2); + const graphFactory = sinon.stub().resolves({}); + const {mocks, listen, createdHandlers} = createMocks({stacks: [stack1, stack2]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + // The stable request handler passed to http.createServer. + const trampoline = createdHandlers[0]; + trampoline("req", "res"); + t.true(app1.calledOnceWithExactly("req", "res"), "routed to app1 before swap"); + + await supervisor.reinitialize(); + + trampoline("req2", "res2"); + t.true(app2.calledOnceWithExactly("req2", "res2"), "routed to app2 after swap"); + t.true(listen.calledOnce, "the socket is not re-bound on reinitialize"); +}); + +test("reinitialize() is build-new-then-swap: new stack is built before the old is destroyed", async (t) => { + const order = []; + const stack1 = createStack(); + stack1.buildServer.destroy = sinon.stub().callsFake(async () => { + order.push("destroy-old"); + }); + const stack2 = createStack(); + const graphFactory = sinon.stub().callsFake(async () => { + order.push("graphFactory"); + return {}; + }); + const {mocks} = createMocks({ + buildAppImpl: async () => { + order.push("buildApp"); + return order.filter((s) => s === "buildApp").length === 1 ? stack1 : stack2; + } + }); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + order.length = 0; // drop the initial build + + await supervisor.reinitialize(); + + t.deepEqual(order, ["graphFactory", "buildApp", "destroy-old"], + "new graph resolved and new app built before the old BuildServer is destroyed"); +}); + +test("reinitialize() failure keeps the last-good stack serving", async (t) => { + let calls = 0; + const app1 = sinon.stub(); + const stack1 = createStack(app1); + const buildError = new Error("invalid ui5.yaml"); + const graphFactory = sinon.stub().resolves({}); + const {mocks, attachLiveReloadServer, createdHandlers} = createMocks({ + buildAppImpl: async () => { + calls++; + if (calls === 1) { + return stack1; + } + throw buildError; + } + }); + const {default: Supervisor} = await importSupervisor(mocks); + + const errorEvents = []; + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + supervisor.on("error", (err) => errorEvents.push(err)); + + await t.notThrowsAsync(supervisor.reinitialize(), "a broken definition does not reject"); + + // The trampoline still routes to the last-good app — the failed build was not adopted. + createdHandlers[0]("req", "res"); + t.true(app1.calledOnceWithExactly("req", "res"), "requests still route to the last-good app"); + t.false(stack1.buildServer.destroy.called, "old BuildServer is not destroyed on failure"); + t.is(errorEvents.length, 0, "no fatal 'error' event is emitted"); + t.true(attachLiveReloadServer.calledOnce); +}); + +test("live-reload subscription moves to the new BuildServer across a swap", async (t) => { + const stack1 = createStack(); + const stack2 = createStack(); + sinon.spy(stack1.buildServer, "off"); + sinon.spy(stack2.buildServer, "on"); + const graphFactory = sinon.stub().resolves({}); + const {mocks, attachLiveReloadServer} = createMocks({stacks: [stack1, stack2]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + const relay = attachLiveReloadServer.firstCall.args[0].buildServer; + + await supervisor.reinitialize(); + + t.true(stack1.buildServer.off.calledWith("sourcesChanged"), "old BuildServer is detached from the relay"); + t.true(stack2.buildServer.on.calledWith("sourcesChanged"), "new BuildServer is attached to the relay"); + + // A sourcesChanged from the new BuildServer still reaches relay subscribers. + let relayed = 0; + relay.on("sourcesChanged", () => relayed++); + stack2.buildServer.emit("sourcesChanged"); + t.is(relayed, 1, "new BuildServer drives the stable relay"); + + // The detached old BuildServer no longer drives it. + stack1.buildServer.emit("sourcesChanged"); + t.is(relayed, 1, "old BuildServer no longer drives the relay"); +}); + +test("overlapping reinitialize() calls collapse into one trailing pass", async (t) => { + const stack1 = createStack(); + // Pre-created gate so resolving it does not depend on the async callback having run yet. + const firstReinitGate = Promise.withResolvers(); + let buildCalls = 0; + const graphFactory = sinon.stub().resolves({}); + const {mocks} = createMocks({ + buildAppImpl: async () => { + buildCalls++; + if (buildCalls === 1) { + return stack1; // initial build + } + if (buildCalls === 2) { + // First re-init: block until released to create the overlap window. + await firstReinitGate.promise; + } + return createStack(); + } + }); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + const p1 = supervisor.reinitialize(); + const p2 = supervisor.reinitialize(); // collapses into a trailing pass while p1 is in flight + firstReinitGate.resolve(); + await Promise.all([p1, p2]); + + // One initial build + two re-init builds (the second is the single trailing pass). + t.is(buildCalls, 3, "the trailing pass runs exactly once, sequentially"); +}); + +test("a queued reinitialize() does not re-resolve early; the trailing pass owns the only extra resolve", + async (t) => { + const stack1 = createStack(); + // Hold the first re-init inside buildApp to open an overlap window, mirroring a slow + // framework build on a large project. + const firstBuildGate = Promise.withResolvers(); + let buildCalls = 0; + const {mocks} = createMocks({ + buildAppImpl: async () => { + buildCalls++; + if (buildCalls === 1) { + return stack1; // initial build + } + if (buildCalls === 2) { + await firstBuildGate.promise; // first re-init: park inside the build + } + return createStack(); + } + }); + const graphFactory = sinon.stub().resolves({}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + t.is(graphFactory.callCount, 0, "no resolve on the initial build"); + + const p1 = supervisor.reinitialize(); // starts #swap(): resolves, then parks in buildApp + // Let the first swap reach its (blocked) build so #reinitInProgress is set. + await new Promise((resolve) => setImmediate(resolve)); + t.is(graphFactory.callCount, 1, "first swap resolved the graph"); + + // A second definitionChanged lands while the first swap's build is still in flight. The + // queued branch only sets the trailing-pass flag — it must NOT resolve the graph early. + const p2 = supervisor.reinitialize(); + await p2; + t.is(graphFactory.callCount, 1, "the queued re-init does not re-resolve while the swap builds"); + + // Release the first build; the trailing pass then re-resolves once for the swap it performs. + firstBuildGate.resolve(); + await p1; + t.is(graphFactory.callCount, 2, "only the trailing pass adds a resolve"); + t.is(buildCalls, 3, "one initial build + first swap + trailing-pass swap"); + }); + +test("destroy() closes live-reload, the socket, and the BuildServer; reinitialize() is then a no-op", async (t) => { + const stack = createStack(); + const graphFactory = sinon.stub().resolves({}); + const {mocks, httpServer, liveReloadHandle} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + await new Promise((resolve) => supervisor.destroy(resolve)); + + t.true(liveReloadHandle.close.calledOnce); + t.true(httpServer.close.calledOnce); + t.true(stack.buildServer.destroy.calledOnce); + + await supervisor.reinitialize(); + t.true(graphFactory.notCalled, "reinitialize after destroy does nothing"); +}); + +test("destroy() closes the socket even when BuildServer.destroy() rejects", async (t) => { + const stack = createStack(); + stack.buildServer.destroy = sinon.stub().rejects(new Error("destroy failed")); + const {mocks, httpServer} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {}); + + await new Promise((resolve) => supervisor.destroy(resolve)); + t.true(httpServer.close.calledOnce, "socket is closed despite the BuildServer destroy rejection"); +}); + +test("reinitialize() warns and no-ops when no graphFactory was provided", async (t) => { + const stack = createStack(); + const {mocks, buildApp} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {}); + await supervisor.reinitialize(); + t.true(buildApp.calledOnce, "no re-init build happens without a graphFactory"); +}); + +test("definition watcher is created on create() only when a graphFactory is present", async (t) => { + const stack = createStack(); + const {mocks, ProjectDefinitionWatcher} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + await Supervisor.create({}, baseConfig, undefined, {}); + t.true(ProjectDefinitionWatcher.create.notCalled, "no watcher without a graphFactory"); +}); + +test("definition watcher is created with the threaded config params", async (t) => { + const stack = createStack(); + const graphFactory = sinon.stub().resolves({}); + const graph = {getRoot: () => ({})}; + const config = { + ...baseConfig, + rootConfigPath: "/app/custom.yaml", + workspaceConfigPath: null, + dependencyDefinitionPath: "/app/deps.yaml", + cwd: "/app", + }; + const {mocks, ProjectDefinitionWatcher} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + await Supervisor.create(graph, config, undefined, {graphFactory}); + + t.true(ProjectDefinitionWatcher.create.calledOnce); + const opts = ProjectDefinitionWatcher.create.firstCall.args[0]; + t.is(opts.graph, graph, "watcher gets the initial graph"); + t.is(opts.rootConfigPath, "/app/custom.yaml"); + t.is(opts.workspaceConfigPath, null); + t.is(opts.dependencyDefinitionPath, "/app/deps.yaml"); + t.is(opts.cwd, "/app"); +}); + +test("a definitionChanged event triggers reinitialize()", async (t) => { + const app2 = sinon.stub(); + const stack1 = createStack(); + const stack2 = createStack(app2); + const graphFactory = sinon.stub().resolves({}); + const {mocks, definitionWatchers, createdHandlers} = createMocks({stacks: [stack1, stack2]}); + const {default: Supervisor} = await importSupervisor(mocks); + + await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + // The watcher created on init drives the re-init. + definitionWatchers[0].emit("definitionChanged", {eventType: "update", filePath: "/app/ui5.yaml"}); + // reinitialize() is async; let the swap settle. + await new Promise((resolve) => setImmediate(resolve)); + + createdHandlers[0]("req", "res"); + t.true(app2.calledOnceWithExactly("req", "res"), "definition change swapped in the new app"); +}); + +test.serial("a definitionChanging event signals ui5.project-resolve-started (version-slot reset)", async (t) => { + const stack1 = createStack(); + const graphFactory = sinon.stub().resolves({}); + const {mocks, definitionWatchers} = createMocks({stacks: [stack1]}); + const {default: Supervisor} = await importSupervisor(mocks); + + await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + const emit = sinon.spy(process, "emit"); + // The watcher's leading-edge event: a re-resolve is coming, blank the version slot. + definitionWatchers[0].emit("definitionChanging", {eventType: "update", filePath: "/app/ui5.yaml"}); + + const resolving = emit.getCalls().find((c) => c.args[0] === "ui5.project-resolve-started"); + t.truthy(resolving, "ui5.project-resolve-started was emitted to reset the version slot"); +}); + +test.serial("a failed swap releases the version placeholder via ui5.project-resolve-failed", async (t) => { + const stack1 = createStack(); + let calls = 0; + const graphFactory = sinon.stub().resolves({}); + const {mocks} = createMocks({ + buildAppImpl: async () => { + calls++; + if (calls === 1) { + return stack1; // initial build + } + throw new Error("invalid ui5.yaml"); // the re-init build fails + } + }); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + const emit = sinon.spy(process, "emit"); + await supervisor.reinitialize(); + + const released = emit.getCalls().find((c) => c.args[0] === "ui5.project-resolve-failed"); + t.truthy(released, "a failed swap emits ui5.project-resolve-failed so the placeholder does not wedge"); +}); + +test("watcher is re-targeted to the new graph after a swap (old destroyed, new created)", async (t) => { + const stack1 = createStack(); + const stack2 = createStack(); + const newGraph = {name: "newGraph", getRoot: () => ({})}; + const graphFactory = sinon.stub().resolves(newGraph); + const {mocks, ProjectDefinitionWatcher, definitionWatchers} = createMocks({stacks: [stack1, stack2]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + t.is(ProjectDefinitionWatcher.create.callCount, 1, "watcher created on init"); + const firstWatcher = definitionWatchers[0]; + + await supervisor.reinitialize(); + + t.is(ProjectDefinitionWatcher.create.callCount, 2, "a fresh watcher created after the swap"); + t.is(ProjectDefinitionWatcher.create.secondCall.args[0].graph, newGraph, "new watcher targets the new graph"); + t.true(firstWatcher.destroy.calledOnce, "old watcher destroyed"); +}); + +test("a watcher-create failure during swap keeps the server serving", async (t) => { + const app1 = sinon.stub(); + const stack1 = createStack(app1); + const stack2 = createStack(); + const graphFactory = sinon.stub().resolves({getRoot: () => ({})}); + let createCalls = 0; + const {mocks, createdHandlers} = createMocks({ + stacks: [stack1, stack2], + definitionWatcherCreate: async () => { + createCalls++; + if (createCalls === 1) { + const watcher = new EventEmitter(); + watcher.destroy = sinon.stub().resolves(); + return watcher; + } + throw new Error("watcher failed to arm"); + }, + }); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + await t.notThrowsAsync(supervisor.reinitialize(), "a watcher-create failure does not reject the swap"); + + // The swap itself still committed — the new stack is serving. + createdHandlers[0]("req", "res"); + t.true(stack2.app.calledOnceWithExactly("req", "res"), "the new app serves despite the watcher failure"); +}); + +test("destroy() tears the definition watcher down", async (t) => { + const stack = createStack(); + const graphFactory = sinon.stub().resolves({}); + const {mocks, definitionWatchers} = createMocks({stacks: [stack]}); + const {default: Supervisor} = await importSupervisor(mocks); + + const supervisor = await Supervisor.create({}, baseConfig, undefined, {graphFactory}); + + await new Promise((resolve) => supervisor.destroy(resolve)); + t.true(definitionWatchers[0].destroy.calledOnce, "watcher destroyed on teardown"); +}); diff --git a/packages/server/test/lib/server/serve/stack.js b/packages/server/test/lib/server/serve/stack.js new file mode 100644 index 00000000000..a4fc0bed184 --- /dev/null +++ b/packages/server/test/lib/server/serve/stack.js @@ -0,0 +1,68 @@ +import test from "ava"; +import sinon from "sinon"; +import esmock from "esmock"; + +// Unit: the shared router core owns the BuildServer once graph.serve() has started it. A failure +// while assembling the middleware must destroy that BuildServer before rethrowing, since the +// caller never receives a buildServer or close() handle to release its watcher and cache. + +function createGraph(buildServer) { + const rootProject = { + getName: () => "root.project", + getSourceReader: () => ({}), + }; + return { + getRoot: () => rootProject, + getProject: () => undefined, + traverseBreadthFirst: async () => {}, + serve: sinon.stub().resolves(buildServer), + }; +} + +function createBuildServer() { + return { + getRootReader: () => ({}), + getDependenciesReader: () => ({}), + getReader: () => ({}), + getServeError: () => undefined, + on: sinon.stub(), + destroy: sinon.stub().resolves(), + }; +} + +async function importBuildRouter(applyMiddleware) { + return esmock("../../../../lib/serve/stack.js", { + "../../../../lib/middleware/MiddlewareManager.js": class MiddlewareManager { + applyMiddleware(...args) { + return applyMiddleware(...args); + } + }, + }); +} + +test("buildRouter() destroys the BuildServer when middleware assembly fails", async (t) => { + const buildServer = createBuildServer(); + const graph = createGraph(buildServer); + const assemblyError = new Error("applyMiddleware failed"); + const applyMiddleware = sinon.stub().rejects(assemblyError); + + const {buildRouter} = await importBuildRouter(applyMiddleware); + + const err = await t.throwsAsync(buildRouter(graph, {})); + t.is(err, assemblyError, "the original assembly error is rethrown"); + t.true(buildServer.destroy.calledOnce, + "the BuildServer is destroyed so its watcher and cache handle are released"); +}); + +test("buildRouter() returns the router and BuildServer on success", async (t) => { + const buildServer = createBuildServer(); + const graph = createGraph(buildServer); + const applyMiddleware = sinon.stub().resolves(); + + const {buildRouter} = await importBuildRouter(applyMiddleware); + + const result = await buildRouter(graph, {}); + t.is(result.buildServer, buildServer, "the BuildServer is returned to the caller"); + t.is(typeof result.router, "function", "an express router is returned"); + t.true(buildServer.destroy.notCalled, "the BuildServer is not destroyed on success"); +}); diff --git a/packages/server/test/lib/server/serveMiddleware.js b/packages/server/test/lib/server/serveMiddleware.js new file mode 100644 index 00000000000..181207847dc --- /dev/null +++ b/packages/server/test/lib/server/serveMiddleware.js @@ -0,0 +1,124 @@ +import test from "ava"; +import sinon from "sinon"; +import esmock from "esmock"; +import express from "express"; +import supertest from "supertest"; +import {graphFromPackageDependencies} from "@ui5/project/graph"; +import serveMiddleware from "../../../lib/serveMiddleware.js"; +import {INJECT_SCRIPT_TAG} from "../../../lib/liveReload/constants.js"; +import {isolatedUi5DataDir} from "../../utils/buildCacheIsolation.js"; + +// Integration: mount the returned middleware on a caller-owned express app and serve real +// requests through supertest, without binding a port or starting a UI5-owned HTTP server. + +let app; +let close; +let request; + +test.before(async (t) => { + const graph = await graphFromPackageDependencies({ + cwd: "./test/fixtures/application.a" + }); + + const result = await serveMiddleware(graph, { + ui5DataDir: isolatedUi5DataDir(t), + }); + close = result.close; + + app = express(); + app.use(result.middleware); + request = supertest(app); +}); + +test.after.always(async () => { + await close(); +}); + +async function get(path) { + const res = await request.get(path); + if (res.error) { + throw new Error(res.error); + } + return res; +} + +test("Serves index.html through a caller-owned express app", async (t) => { + const res = await get("/index.html"); + t.is(res.statusCode, 200, "Correct HTTP status code"); + t.regex(res.headers["content-type"], /html/, "Correct content type"); + t.regex(res.text, /Application A<\/title>/, "Correct response"); +}); + +test("Serves the UI5 version info", async (t) => { + const res = await get("/resources/sap-ui-version.json"); + t.is(res.statusCode, 200, "Correct HTTP status code"); + t.regex(res.headers["content-type"], /json/, "Correct content type"); +}); + +test("Does not inject the live-reload client script", async (t) => { + const res = await get("/index.html"); + t.false(res.text.includes(INJECT_SCRIPT_TAG), + "The live-reload client script is not injected in the embedding API"); +}); + +// Unit: the module contract independent of a real graph — the returned middleware is the +// router from the shared core, close() releases the BuildServer once and is idempotent, and +// the live-reload WebSocket path is disabled. + +function createBuildRouterMock() { + const router = sinon.stub(); + const buildServer = { + destroy: sinon.stub().resolves(), + }; + const buildRouter = sinon.stub().resolves({ + router, + buildServer, + liveReloadOptions: {active: false, token: null}, + }); + return {router, buildServer, buildRouter}; +} + +async function importServeMiddleware(buildRouter) { + return esmock("../../../lib/serveMiddleware.js", { + "../../../lib/serve/stack.js": {buildRouter}, + }); +} + +test("serveMiddleware() returns the router as middleware and disables live-reload", async (t) => { + const {router, buildRouter} = createBuildRouterMock(); + const {default: serveMiddlewareMocked} = await importServeMiddleware(buildRouter); + const graph = {}; + + const result = await serveMiddlewareMocked(graph, {ui5DataDir: "/tmp/data"}); + + t.true(buildRouter.calledOnce); + const [passedGraph, config] = buildRouter.firstCall.args; + t.is(passedGraph, graph, "the graph is threaded through to the shared core"); + t.is(config.liveReload, false, "live-reload is disabled for the embedding API"); + t.is(config.webSocketToken, null, "no WebSocket token is minted for the embedding API"); + t.is(config.ui5DataDir, "/tmp/data", "options are threaded into the config"); + t.is(result.middleware, router, "the shared core's router is returned as middleware"); + t.is(typeof result.close, "function"); +}); + +test("serveMiddleware() close() destroys the BuildServer once and is idempotent", async (t) => { + const {buildServer, buildRouter} = createBuildRouterMock(); + const {default: serveMiddlewareMocked} = await importServeMiddleware(buildRouter); + + const {close} = await serveMiddlewareMocked({}, {}); + await close(); + await close(); + + t.true(buildServer.destroy.calledOnce, "the BuildServer is destroyed exactly once"); +}); + +test("serveMiddleware() works without options", async (t) => { + const {buildRouter} = createBuildRouterMock(); + const {default: serveMiddlewareMocked} = await importServeMiddleware(buildRouter); + + await serveMiddlewareMocked({}); + + const config = buildRouter.firstCall.args[1]; + t.is(config.liveReload, false); + t.is(config.webSocketToken, null); +}); diff --git a/packages/server/test/lib/server/server.js b/packages/server/test/lib/server/server.js index fc85c81c5d2..1535c76a4e0 100644 --- a/packages/server/test/lib/server/server.js +++ b/packages/server/test/lib/server/server.js @@ -1,141 +1,95 @@ import test from "ava"; import sinon from "sinon"; import esmock from "esmock"; -import {EventEmitter} from "node:events"; -function createMockGraph(mockBuildServer) { - const mockProject = { - getName: sinon.stub().returns("test.project"), - getSourceReader: sinon.stub().returns({}) +// server.js is now a thin wrapper over Supervisor: it generates the live-reload token, builds +// the config, delegates to Supervisor.create(), and shapes the {h2, port, close, reinitialize} +// result. These tests exercise that wrapper; the swap/relay/trampoline behavior lives in +// serve/Supervisor.js and is covered there. + +function createSupervisorMock({port = 3000, createRejects = null} = {}) { + const supervisor = { + getPort: sinon.stub().returns(port), + destroy: sinon.stub().callsFake((cb) => cb && cb()), + reinitialize: sinon.stub().resolves(), }; - return { - getRoot: sinon.stub().returns(mockProject), - traverseBreadthFirst: sinon.stub().resolves(), - getProject: sinon.stub().returns(null), - serve: sinon.stub().resolves(mockBuildServer) + const create = createRejects ? + sinon.stub().rejects(createRejects) : + sinon.stub().resolves(supervisor); + const Supervisor = { + create, }; + return {supervisor, Supervisor}; } -function createMockBuildServer() { - const buildServer = new EventEmitter(); - buildServer.getRootReader = sinon.stub().returns({}); - buildServer.getDependenciesReader = sinon.stub().returns({}); - buildServer.getReader = sinon.stub().returns({}); - buildServer.destroy = sinon.stub().resolves(); - return buildServer; -} - -function createMockServer() { - const mockServer = new EventEmitter(); - mockServer.close = sinon.stub().callsFake((cb) => cb()); - return mockServer; +async function importServe(Supervisor) { + return esmock("../../../lib/server.js", { + "../../../lib/serve/Supervisor.js": {default: Supervisor}, + }); } -function createMocks(mockServer) { - const mockApp = { - use: sinon.stub(), - listen: sinon.stub().callsFake((options, cb) => { - process.nextTick(cb); - return mockServer; - }) - }; +test.afterEach.always(() => { + sinon.restore(); +}); - return { - "express": sinon.stub().returns(mockApp), - "portscanner": { - findAPortNotInUse: sinon.stub().callsFake((port, portMax, host, cb) => { - cb(null, port); - }) - }, - "../../../lib/middleware/MiddlewareManager.js": { - default: class MockMiddlewareManager { - applyMiddleware() {} - } - }, - "@ui5/fs/resourceFactory": { - createReaderCollection: sinon.stub().returns({}) - }, - "@ui5/fs/ReaderCollectionPrioritized": { - default: class MockReaderCollectionPrioritized {} - } - }; -} +test("serve() delegates to Supervisor.create and returns port/h2/close/reinitialize", async (t) => { + const {supervisor, Supervisor} = createSupervisorMock({port: 3000}); + const {serve} = await importServe(Supervisor); + const graph = {}; + const graphFactory = sinon.stub(); + + const result = await serve(graph, {port: 3000, h2: false, liveReload: true}, undefined, {graphFactory}); + + t.true(Supervisor.create.calledOnce); + const [passedGraph, config, , options] = Supervisor.create.firstCall.args; + t.is(passedGraph, graph); + t.is(options.graphFactory, graphFactory, "graphFactory is threaded through to the supervisor"); + t.is(typeof config.webSocketToken, "string", "a token is generated when liveReload is active"); + t.is(config.webSocketToken.length, 12, "the token is 72 bits base64url-encoded to 12 characters"); + t.is(result.port, 3000); + t.is(result.h2, false); + t.is(typeof result.close, "function"); + t.is(typeof result.reinitialize, "function"); + + result.reinitialize(); + t.true(supervisor.reinitialize.calledOnce, "reinitialize forwards to the supervisor"); +}); -test("server.on('error') rejects the serve promise", async (t) => { - const mockServer = createMockServer(); - const mockBuildServer = createMockBuildServer(); - const testError = new Error("server error"); - - const mockApp = { - use: sinon.stub(), - listen: sinon.stub().callsFake((options, cb) => { - // Emit error before the listen callback fires so reject() is called - process.nextTick(() => { - mockServer.emit("error", testError); - }); - return mockServer; - }) - }; +test("serve() does not generate a live-reload token when liveReload is off", async (t) => { + const {Supervisor} = createSupervisorMock(); + const {serve} = await importServe(Supervisor); - const mocks = { - "express": sinon.stub().returns(mockApp), - "portscanner": { - findAPortNotInUse: sinon.stub().callsFake((port, portMax, host, cb) => { - cb(null, port); - }) - }, - "../../../lib/middleware/MiddlewareManager.js": { - default: class MockMiddlewareManager { - applyMiddleware() {} - } - }, - "@ui5/fs/resourceFactory": { - createReaderCollection: sinon.stub().returns({}) - }, - "@ui5/fs/ReaderCollectionPrioritized": { - default: class MockReaderCollectionPrioritized {} - } - }; + await serve({}, {port: 3000, liveReload: false}, undefined, {}); - const {serve} = await esmock("../../../lib/server.js", mocks); - const graph = createMockGraph(mockBuildServer); - const error = await t.throwsAsync(serve(graph, {port: 3000})); - t.is(error, testError); + const config = Supervisor.create.firstCall.args[1]; + t.is(config.webSocketToken, null); }); +test("serve() close() forwards to supervisor.destroy()", async (t) => { + const {supervisor, Supervisor} = createSupervisorMock(); + const {serve} = await importServe(Supervisor); -test("buildServer 'error' event is forwarded to error callback", async (t) => { - const mockServer = createMockServer(); - const mockBuildServer = createMockBuildServer(); - const mocks = createMocks(mockServer); - const testError = new Error("build error"); - - const {serve} = await esmock("../../../lib/server.js", mocks); - const graph = createMockGraph(mockBuildServer); + const result = await serve({}, {port: 3000}, undefined, {}); + await new Promise((resolve) => result.close(resolve)); - const errorReceived = new Promise((resolve) => { - serve(graph, {port: 3000}, resolve).then(() => { - mockBuildServer.emit("error", testError); - }); - }); - - const err = await errorReceived; - t.is(err, testError); + t.true(supervisor.destroy.calledOnce); }); -test("close() still calls server.close when buildServer.destroy() rejects", async (t) => { - const mockServer = createMockServer(); - const mockBuildServer = createMockBuildServer(); - const mocks = createMocks(mockServer); +test("serve() rejects when Supervisor.create rejects", async (t) => { + const createError = new Error("bind failed"); + const {Supervisor} = createSupervisorMock({createRejects: createError}); + const {serve} = await importServe(Supervisor); - mockBuildServer.destroy = sinon.stub().rejects(new Error("destroy failed")); + const err = await t.throwsAsync(serve({}, {port: 3000}, undefined, {})); + t.is(err, createError); +}); - const {serve} = await esmock("../../../lib/server.js", mocks); - const graph = createMockGraph(mockBuildServer); - const result = await serve(graph, {port: 3000}); +test("serve() works without the 4th options argument (backward compatible)", async (t) => { + const {Supervisor} = createSupervisorMock(); + const {serve} = await importServe(Supervisor); - await new Promise((resolve) => { - result.close(resolve); - }); - t.true(mockServer.close.calledOnce, "server.close was called despite destroy rejection"); + const result = await serve({}, {port: 3000}, undefined); + t.is(result.port, 3000); + const options = Supervisor.create.firstCall.args[3]; + t.is(options.graphFactory, undefined); });