refactor(project, server): Improve error handling and recovery#1459
Draft
RandomByte wants to merge 16 commits into
Draft
refactor(project, server): Improve error handling and recovery#1459RandomByte wants to merge 16 commits into
RandomByte wants to merge 16 commits into
Conversation
RandomByte
force-pushed
the
refactor/serve-error-handling-2
branch
from
July 17, 2026 12:32
a57fb96 to
a46005d
Compare
Classify caught task errors into three branches: abort, transient (a concurrent source change), and normal. Transient errors re-queue the affected projects and keep held reader requests open so they resolve on the retry. Normal errors report via ServeLogger. The "error" event stays reserved for fatal failures like a watcher crash, so the process still exits on those.
A failed build latches its projects into a new ERRORED state and short-circuits #getReaderForProject with the captured error. An unchanged rebuild would just reproduce the failure, so hold the gate until real signal arrives. Three signals lift it: - Source change on the project itself: invalidate() already clears #lastError. - Source change on a dependent: _projectResourceChanged invalidate()s each dependent, lifting ERRORED as a side effect. - Source change on a transitive dependency, or any successful build cycle: both call the new ProjectBuildStatus.clearError() (ERRORED -> INVALIDATED, drops #lastError without touching the reader or aborting a build). Without this a request resolving through a gated dependency would replay the error forever. Correct the banner for the error path: _projectResourceChanged transitions ERROR -> STALE, #processBuildRequests re-signals BUILDING per iteration so retry progress replaces the red state, #getStaleProjectNames excludes ERRORED projects, and the post-cycle transition preserves ERROR for projects that stay gated. Add an integration test for the gate plus four unit tests: a dependent's change lifts its dependency's gate, a successful cycle lifts unrelated gates, no signal keeps the gate closed, and a change on the ERRORED project lifts its own gate.
Every next(err) in the middleware chain now flows through a custom 4-argument handler that responds with HTTP 500, text/plain, and the error stack (falling back to the message or String(err)). Registered last so it catches errors from every earlier middleware, including third-party custom middleware we cannot wrap in a try/catch. If the response has already started, delegate to Express's default handler, which closes the connection. Console-log routing (separating gated build errors from unexpected ones) stays out of this handler for now; it lands in a follow-up.
@parcel/watcher can report that OS-level FS events were dropped and its event
cache is unreliable ("Events were dropped by the FSEvents client. File system
must be re-scanned."). The build cache derives what changed solely from watcher
events fed through resourcesChanged(), so dropped events leave the change signal
incomplete and cached results can be served stale. Any WatchHandler error was
previously terminal: the server went to ERROR with no recovery.
Treat a watcher error as recoverable: recreate the watch subscriptions, force a
full source re-scan of every project, and return the server to STALE so the
next reader request rebuilds against a re-indexed tree.
- Extract ProjectBuildCache.resetForFullRescan() from the reset block
allTasksCompleted() already used for build-end source drift. It re-arms
initSourceIndex() to re-glob and diff against the persisted index, catching
changes the watcher never reported.
- Add ProjectBuilder.forceFullRescan() (guarded by #buildIsRunning), delegating
through BuildContext / ProjectBuildContext.
- BuildServer.#recoverWatcher() orchestrates recovery: guarded against
re-entrant error storms, quiesces validation and the active build, recreates
the watcher, forces the re-scan, invalidates all projects, and drains parked
reader requests. #triggerRequestQueue and #scheduleBackgroundValidation are
suppressed during recovery so nothing races the builder lock. A sliding-window
counter escalates to terminal ERROR if recovery keeps failing, since dropped
events arrive via the subscription callback rather than a watch() rejection
and would otherwise loop forever.
emit("error") stays reserved for the terminal fallback; a successful recovery
emits sourcesChanged so clients reload.
A build-task failure (e.g. a LESS syntax error from buildThemes) left the reused in-memory build state corrupted, so `ui5 serve` kept returning the same error after the source was fixed, until a server restart. When a task throws, runTasks rejects before allTasksCompleted() runs. The project's ProjectResources keeps the partial stage writers from the broken source, and #currentResultSignature keeps the last successful build's value. On the next build the recovered source signature matches the retained #currentResultSignature, so #findResultCache short-circuits, marks the project usesCache, and serves the stale broken stages. Dependent theme libraries then recompile against that and fail the same way. Discard the failing project's in-memory state on a genuine (non-abort) failure so a later rebuild re-imports clean stages from the persistent cache. ProjectBuilder#build tracks the in-flight context and calls resetForFullRescan() on it; abort and SourceChangedDuringBuildError are excluded via isAbortError. Harden resetForFullRescan() against the same corruption: also clear #currentResultSignature, #cachedResultSignature, #currentStageSignatures, and #writtenResultResourcePaths, and reset the stage pipeline via the new ProjectResources.reset(). Otherwise the #findResultCache early return would still fire and skip re-importing the cached stages after a reset.
…settle sourcesChanged drives live-reload, so a lone edit should reach clients fast. On small projects the triggered build finishes well under 100 ms, so the previous 100 ms trailing debounce dominated edit-to-reload latency. Emit on the leading edge: the first change of a quiet period notifies immediately, and a trailing settle window coalesces the rest of a burst into one further emit. A single edit now reaches clients at the watcher's own latency floor. Raise the window to 550 ms, above @parcel/watcher's 500 ms coalescing cap. The watcher delivers a continuous operation like `git checkout` as batches up to 500 ms apart; a window below the cap would emit per batch, while one above it lets each batch reset the window so the whole operation collapses to one leading plus one trailing emit. With leading-edge emission the window size no longer affects single-edit latency. Rename SOURCES_CHANGED_DEBOUNCE_MS to SOURCES_CHANGED_SETTLE_MS.
When a source change lands mid-build, the build is aborted and its projects re-queued. The restart used the same 10 ms debounce as a reader request, so a burst of changes restarted a build per batch, each aborted by the next. @parcel/watcher caps its coalescing at 500 ms, so a continuous operation (git checkout, save-all, a bundler writing many files) arrives as batches up to 500 ms apart, producing a build -> abort -> build cycle for the whole operation. Hold the post-abort restart until changes are quiet for 550 ms (above the watcher's cap), resetting the window on each further change via #_projectResourceChanged, so the burst collapses into a single rebuild. The delay is scoped to the speculative restart: a reader request still enqueues on BUILD_REQUEST_DEBOUNCE_MS and supersedes a pending restart, so serving a request is unaffected. Extract the inline 10 ms request-queue debounce into BUILD_REQUEST_DEBOUNCE_MS and give #triggerRequestQueue a delay parameter.
During a `git checkout` the serve status could stay on `building` or flip to red `error` while nothing ran, until the checkout finished. The banner claimed a build was in progress when #activeBuild was already null. The deferred post-abort restart had no state to report: during its settle window the server is waiting for changes to stop, but SERVER_STATES had no value meaning that, so it showed whatever the last transition left. Add a SETTLING lifecycle state: changes seen, rebuild pending, holding until changes go quiet. It sits between STALE and BUILDING with no active build (#activeBuild === null); the deferred timer moves it to BUILDING when the window elapses. Route both deferral sites through it: - Post-abort restart reports SETTLING instead of leaving the banner on `building`. - Failure-with-pending-changes (the signal.aborted || #resourceChangeQueue.size > 0 branch) now defers and reports like the abort path instead of parking on the failed build's state. This fixes the `git checkout` case: the doomed first build reports SETTLING and retries once the tree is quiet instead of a spurious serve-error. Genuine non-transient failures still latch ERRORED. BUILDING -> SETTLING skips the buildDone emission (like BUILDING -> ERROR), and #reconcileServerState bails on SETTLING so the deferred-restart timer owns the SETTLING -> BUILDING transition. ServeLogger gains settling(pendingProjects) emitting serve-settling, and the interactive banner renders a spinning "settling · waiting for changes to settle" line.
The first build after a quiet period fired on BUILD_REQUEST_DEBOUNCE_MS (10 ms). On a multi-file operation the tree is half-written when the first watcher event lands, so the build fires into an incomplete tree and fails transiently. Give the first speculative build a 100 ms settle window (FIRST_BUILD_SETTLE_MS) instead. This absorbs an editor's multi-file save fan-out; 100 ms sits well below @parcel/watcher's 500 ms cap and near its 50 ms floor, so single-edit latency is barely affected. The window reports SETTLING. Applies only when a build is already pending (a reader request queued but not started); laziness is preserved, since with nothing queued a source change still waits for a reader request. An explicit reader request supersedes the window by re-arming the queue at the snappy debounce, so serving a request is never delayed. Kept separate from the SETTLING reporting change so a regression in single-edit latency can be bisected cleanly.
transitionTo() in state/build.js resets spinFrame to 0 on every transition, and no other serve-* branch sets it at the call site. Drop the redundant assignment and rely on the helper.
…chines architecture.md had fallen behind BuildServer.js. Rewrite the affected sections to match the implementation: - Replace the 3-state per-project sketch (INITIAL / INVALIDATED / FRESH) with the actual 6-state ProjectBuildStatus diagram, adding VALIDATING, BUILDING, and ERRORED, and note which transitions each state accepts. - Add a Server lifecycle section for SERVER_STATES (IDLE / STALE / BUILDING / VALIDATING / ERROR) driven by #setState and #reconcileServerState, including SETTLING between STALE and BUILDING and the BUILDING -> SETTLING edge. - Document background cache validation (#scheduleBackgroundValidation / #runBackgroundValidation) promoting INITIAL projects to FRESH, and the reader-request wait-on-validation path in #getReaderForProject. - Add a Startup section for BuildServer.create() awaiting WatchHandler readiness (the Windows ReadDirectoryChangesW race). - Expand Build Request Flow with the ERRORED short-circuit, the VALIDATING wait, #stopActiveValidation, markBuilding, the transient-vs-non-transient failure split, and FIRST_BUILD_SETTLE_MS. - Expand File Watch and Abort with fileAddedOrRemoved reader eviction, leading-edge sourcesChanged (SOURCES_CHANGED_SETTLE_MS), the deferred post-abort restart (ABORTED_BUILD_RESTART_SETTLE_MS), the first-build 100 ms window, and error clearing on invalidate. - Correct the WatchHandler entry from chokidar to @parcel/watcher and record its 50 ms / 500 ms coalescing wait behind the debounce values. - Add an Error gating section for ERRORED, #lastError, and the transient-error carve-out. - Point the BuildServer row in the component table at ProjectBuildStatus and SERVER_STATES.
…igation
While the build server is errored, a top-level HTML navigation now renders an
error page with the live-reload client attached instead of a plain-text stack
trace, so the tab reloads itself once the source is fixed. The terminal error
handler previously answered every failure with plain text, losing the
live-reload script serveResources.js injects, so a fixed source emitted
sourcesChanged but the tab had no client listening.
Content-negotiate in createErrorHandler: for a top-level HTML navigation, render
errorPage.html with the error message and stack HTML-escaped, and embed
INJECT_SCRIPT_TAG when liveReload.active. Non-HTML requests keep the text/plain
stack response. The script tag reuses the constant serveResources.js injects, so
there is one source of truth. On the next sourcesChanged the client calls
location.reload(), landing on the recovered build or a fresh error page.
Negotiate on Sec-Fetch-Dest and an explicit text/html Accept rather than
req.accepts("html"): a wildcard Accept returns "html", which would serve the
HTML page to every failing subresource load and leave a <script> tag to run the
error page as JavaScript. Sec-Fetch-Dest ("document" only for top-level
navigations) wins when present; an explicit text/html Accept is the fallback, so
an older prefetch sending Accept: text/html for a subresource still gets plain
text. isDocumentNavigation is extracted to helper/ so the errorHandler and the
gate below share this logic. The template is loaded once at module load via
readFileSync, matching serveIndex/directory.html.
Surface the page on any navigation while errored, not only when the requested
path maps to the failed project. Previously, navigating to the app's index.html
while a dependency library build was broken served a normal 200. Add a
serveBuildError gate middleware before serveResources: for document navigations
it consults the server-level error and diverts to the errorHandler via
next(err); subresource loads pass through and keep their per-project behavior.
The gate is a 3-argument middleware, since the 4-argument errorHandler is only
reached once something calls next(err), which a would-be-200 navigation never
does.
- BuildServer: capture the error on entry to ERROR, clear it on every other
transition, expose it via getServeError(). Transient/aborted builds report
SETTLING, so getServeError() returns null during that window.
- Thread getServeError from server.js through MiddlewareManager options.
The error handler hand-rolled a five-character escape map. @ui5/server already depends on escape-html and uses it in serveIndex.cjs; it escapes the same ["'&<>] set to identical strings. Import it and drop the local HTML_ESCAPES map and escapeHtml function.
#setState already defaults the STALE and SETTLING project lists to #getStaleProjectNames(). Three call sites passed exactly that default explicitly. Drop the argument and rely on the ?? fallback inside #setState. The post-build STALE call keeps its argument: it forwards an already-computed set alongside hrtime.
The context is assigned at the top of each loop iteration, before any failure is known; whether the project is failing is only established in the catch clause. Name it for what it tracks — the project currently being processed — rather than for the failure path that reads it.
Set the status via res.statusCode, the content type via res.setHeader, and the body via res.end, rather than the Express res.status/res.type/ res.send helpers. This keeps the middleware independent from the server framework, matching the convention already used in liveReloadClient. The test mock res is updated to the same Node.js interface.
RandomByte
force-pushed
the
refactor/serve-error-handling-2
branch
from
July 17, 2026 12:33
a46005d to
7edd38f
Compare
matz3
reviewed
Jul 17, 2026
| // is globally in ERROR. Placed before serveResources so it can preempt an otherwise- | ||
| // successful 200; after liveReloadClient so the client script is still served during | ||
| // ERROR and the error page can auto-reload once the source is fixed. | ||
| await this.addMiddleware("serveBuildError", { |
Member
There was a problem hiding this comment.
New middleware should be added to docs, right?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.