Skip to content

Loadable lifecycle improvements: abort handling and unified exception hook#4361

Open
lbwexler wants to merge 37 commits into
developfrom
staleAndAutoRefreshHandle
Open

Loadable lifecycle improvements: abort handling and unified exception hook#4361
lbwexler wants to merge 37 commits into
developfrom
staleAndAutoRefreshHandle

Conversation

@lbwexler

@lbwexler lbwexler commented May 4, 2026

Copy link
Copy Markdown
Member

Builds on spanItAll. Reduces per-model boilerplate and makes handling of stale, obsolete, and auto-refresh load errors consistent across the Loadable lifecycle.

Additions

  • LoadAbortedException (isRoutine + isAborted) - thrown when a load is superseded. Raised automatically by FetchService after each fetch (and on demand via LoadSpec.abortIfNeeded() for non-fetch async work), based on LoadSpec.shouldAbort. Recognized by ExceptionHandler and dropped silently.
  • Loadable.skipStaleLoads (default true) - skip/abort loads superseded by a newer started request. Loads superseded by a newer completed request are always skipped.
  • Loadable.skipAutoRefreshErrors (default true) - silence errors raised during an auto-refresh.
  • Loadable.handleLoadException(e, loadSpec) - new hook called only for surface-worthy failures (abort exceptions and, per the flags above, stale/auto-refresh errors are filtered out first). Default delegates to XH.handleException(e). Now a required member of the Loadable interface; HoistModel/HoistService provide the default.

Other changes

  • Renamed the isFetchAborted exception flag to isAborted (now also set by LoadAbortedException). Breaking - update references in catchWhen/catchDefaultWhen predicates etc.
  • Fixed LoadSpec.isStale to compare loadNumber (stable across cloneWithSpan) rather than object identity.
  • Migrated the admin/cmp models off hand-rolled if (loadSpec.isStale) return / .catch boilerplate to the new hook.

Behavior change to default exception handling — open for discussion

Previously, LoadSupport caught an exception from doLoadAsync only to record it and then re-threw it; it never called XH.handleException itself. For a surface-worthy load that an app did not handle, the error therefore reached the user only if something downstream surfaced it — and the standard refresh lifecycle (loadAllAsync via RefreshContextModel) swallows rejections with Promise.allSettled + logError, so the common outcome was a console log, no dialog.

Now LoadSupport routes such errors to handleLoadException (default: XH.handleException(e)), so a non-auto-refresh load that throws and isn't handled by the app will surface a standard error dialog by default. (Superseded/auto-refresh failures remain silent — no alert; genuine non-abort errors are still logged server-side.) Note also that loadAsync() no longer rejects for these errors — it resolves and routes them to the hook.

This is arguably the better default (a failed initial load surfaces instead of leaving a blank view), but it is an observable change for apps that relied on the old console-only behavior. If we want to soften the default, we could scale it back to either or both of:

  • a) Log on server only - default handleLoadException calls XH.handleException(e, {showAlert: false}), so failures are recorded but not shown to the user.
  • b) Re-throw - have the default re-throw (and/or have loadAsync() continue to reject), more closely matching the current behavior so existing callers/lifecycle handling stay in control.

lbwexler and others added 9 commits May 1, 2026 20:01
Matches the pattern already used in toolbox's .mcp.json. Default of
64342 is preserved for developers without the env var set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lbwexler added 2 commits May 5, 2026 09:28
Compare loadNumber against owner.lastRequested?.loadNumber instead of object
identity - clones produced by cloneWithSpan() share the same loadNumber but
are no longer === to lastRequested, so the previous identity check returned
true on every clone.
@lbwexler lbwexler linked an issue May 5, 2026 that may be closed by this pull request
lbwexler and others added 13 commits May 6, 2026 00:36
# Conflicts:
#	CHANGELOG.md
# Conflicts:
#	CHANGELOG.md
…spanPrefix

- Renamed `Runner.track`/`linkTo` → `withTrack`/`withObserver` and
  `logInfo`/`logDebug` → `withInfo`/`withDebug` for consistency and to
  disambiguate from the same-named Promise extensions and `HoistBase` log methods.
- Renamed `fetchJson` → `runFetchJson`, added `runPostJson`, and dropped the
  unused/rare `fetch`, `putJson`, `deleteJson` shortcuts. The `run` prefix marks
  terminals clearly; raw fetches and other verbs go through `.run(ctx => ...)`.
- Renamed `HoistBase.spanPrefix` → `telemetryPrefix` and updated all overrides.
- DataAccess: pulled post-fetch `View.fromBlob` transformations inside `.run()`
  lambdas so they're covered by the runner's span/observability.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@amcclain amcclain marked this pull request as draft May 11, 2026 21:40
lbwexler and others added 9 commits May 13, 2026 23:10
## API additions

* New `XH.metricsService` — lightweight client-side service recording
named timers and counters with optional tags, debouncing batches to the
server's `/xh/recordMetrics` endpoint where they flow into the
Micrometer registry. Requires hoist-core >= 40.0 (xh/hoist-core#556).
* `Runner.timer(name, tags?)` and `Runner.counter(name, tags?)` — record
on completion of the wrapped fn and automatically attach an `xh.outcome`
tag (`success` | `failure`) based on whether the fn threw.

## Behavior changes (minor)

* `Runner.counter()` semantic: previously fired before the wrapped fn
ran; now records on completion.

---------

Co-authored-by: Anselm McClain <atm@xh.io>
- Fix broken catch ternary that swallowed skipped errors incorrectly; route non-aborted skipped errors through XH.handleException
- Skip state updates (lastLoadCompleted/Exception/Succeeded) for skipped loads and capture local `requested` Date so elapsed remains correct for skipped/first-load cases
- Tag `lastRequested`, `lastSucceeded`, `target` as @internal and document their roles
- Clarify Loadable doc on `lastLoadCompleted`/`lastLoadException` re: skipped loads
- Replace lodash `pull` with `omitBy(isNil)` for the debug log payload

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CHANGELOG: fix stray paren / truncated sentence on `skipStaleLoads` description
- LoadAbortedException: reference correct flag name (`skipStaleLoads`, not `abortStaleLoads`)
- Loadable: clarify that skipped exceptions are silently swallowed (no alert, no server log), and reference `handleLoadException` (not `handleException`)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`HoistModel`/`HoistService.handleLoadException` now calls `XH.handleException(e)` directly. The previous implementation delegated to `LoadSupport.handleLoadException`, which itself called `XH.handleException` - one hop for no reason, since `LoadSupport.doLoadAsync` invokes the target's own `handleLoadException` (not its own). Dropped the now-unused `LoadSupport.handleLoadException`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns with sibling lifecycle methods (`doLoadAsync` etc.), which are all required with framework-supplied defaults. `HoistModel`/`HoistService` already implement it; added a no-op on `LoadSupport` (since it acts as orchestrator, not target) and a direct delegation on `UrlStore`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Calls `XH.handleException(e)` instead of being a no-op, matching the `Loadable.handleLoadException` JSDoc and the implementations on `HoistModel`/`HoistService`/`UrlStore`. Easier to reason about (uniform default) and safer if `LoadSupport.handleLoadException` is ever invoked directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lbwexler lbwexler mentioned this pull request May 26, 2026
6 tasks
Base automatically changed from spanItAll to develop June 2, 2026 21:53
@lbwexler lbwexler marked this pull request as ready for review June 17, 2026 15:45
- LoadSupport: superseded/auto-refresh load errors no longer alert the user, but genuine (non-abort) errors are still logged server-side (pass showAlert: false rather than relying on default opts).
- Loadable: corrected skipStaleLoads / skipAutoRefreshErrors docs to match actual behavior (logged-not-alerted, not "no server log").
- CHANGELOG: documented the isFetchAborted -> isAborted rename (Breaking Changes) and handleLoadException becoming a required Loadable member (TS API Adjustments).
@amcclain amcclain added this to the v87 milestone Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider logging exceptions caught by LoadSupport

2 participants