Skip to content

fix(guest): decouple host-method call timeout from hardcoded 10000ms - #156

Open
fe-lix- wants to merge 4 commits into
mainfrom
fix/extension-loading-timeout-guest-call
Open

fe-lix- wants to merge 4 commits into
mainfrom
fix/extension-loading-timeout-guest-call

Conversation

@fe-lix-

@fe-lix- fe-lix- commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug found while investigating why a single broken/404'ing extension in an org's registry caused unrelated, healthy extensions to fail with false "...timed out after 10000ms" errors in Universal Editor. This PR addresses the uix-sdk-level piece of the root cause.

Root cause: a guest's call to a host method (guest.host.*) was subject to a hardcoded 10000ms timeout in @adobe/uix-guest's host proxy (guest.ts), completely decoupled from GuestConfig.timeout (which only ever governs the initial connectParentWindow handshake). @adobe/uix-host's Port defaults its own connection timeout to 20000ms — so any host method call that legitimately took between 10s and 20s (e.g. because the host was still resolving a load batch containing an unrelated slow/broken guest) failed with a false timeout, even though the calling guest itself was healthy and already connected.

Fix:

  • Adds GuestConfig.callTimeout (default 20000ms, matching Port's default connection timeout) and uses it in place of the hardcoded 10000, so the two packages' default timeouts can't silently drift out of sync again, while remaining independently configurable from GuestConfig.timeout per guest.
  • Removes invokeAwaiter's dead final setTimeout: it never actually enforced anything (its rejection was never attached to the returned promise chain) and only produced a stray unhandled promise rejection whenever a call was still pending at the 20000ms mark. This was harmless while the real call timeout was a shorter 10000ms (it always settled first), but became a direct collision once the real timeout also defaults to 20000ms. The real timeout enforcement has always been the outer timeoutPromise(...) wrapping this method's result.

Test plan

  • New unit tests added in a prior commit on this branch (guest.test.ts, host.test.ts, plus a tripwire in port.test.ts) characterizing both the old buggy behavior and Host's (correct, unrelated) batching behavior. uix-guest previously had zero test files and wasn't registered as a jest project — wired that up as part of this work.
  • The characterization test for the fixed behavior ("does not time out a call that resolves at 12000ms, past the old 10000ms ceiling") was committed in a deliberately failing (red) state, then turned green by this fix with no changes to the test itself.
  • Updated the two tests that asserted the old hardcoded-10000ms-regardless-of-config behavior to assert the new decoupled, configurable behavior; added a test for the new callTimeout override.
  • Full unit suite passes across all 4 packages (97 tests), npm run build, npm run declarations:build, and npm run lint all clean.

🤖 Generated with Claude Code

fe-lix- and others added 4 commits September 24, 2026 11:55
Adds unit tests documenting the root cause found while investigating a
regression where a single unreachable/404 extension caused RPC calls
from unrelated, healthy extensions to fail with false "timed out"
errors (docs/extension-loading-timeout-investigation.md).

- guest.test.ts (new): a host method call is subject to a hardcoded
  10000ms timeout (guest.ts's `host` proxy) that is completely
  decoupled from GuestConfig.timeout, which only governs the initial
  connectParentWindow handshake. Includes one intentionally-red test:
  a call resolving at 12000ms should succeed once the guest-side call
  timeout is raised/made configurable to be >= Port's 20000ms connect
  default, instead of spuriously timing out. Wires up @adobe/uix-guest
  for unit testing for the first time (it had no test files and wasn't
  registered as a jest project).

- host.test.ts (new): confirms Host's Promise.all-based batching in
  addLoadsNewGuests is correct by design -- the per-guest "guestload"
  event and getLoadedGuests() reflect a healthy guest immediately,
  without waiting on a broken sibling's full connection timeout, while
  "loadallguests" correctly waits for the whole cohort to settle.

- port.test.ts: adds a tripwire asserting Port's default connection
  timeout stays 20000ms, since the guest-side fix's target value is
  anchored to it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Root cause (docs/extension-loading-timeout-investigation.md): a guest's
call to a host method was subject to a hardcoded 10000ms timeout,
completely independent of GuestConfig.timeout (which only ever governed
the initial connectParentWindow handshake). uix-host's Port defaults its
own connection timeout to 20000ms, so any host method call that
legitimately took between 10s and 20s -- e.g. because the host was still
resolving a load batch containing an unrelated slow/broken (404)
extension -- failed with a false "timed out" error, even though the
calling guest itself was healthy and already connected.

Adds GuestConfig.callTimeout (default 20000ms, matching Port's default)
and uses it in place of the hardcoded value, so the two packages' default
timeouts can't drift out of sync again, while remaining independently
tunable from GuestConfig.timeout and configurable per guest.

Also removes invokeAwaiter's dead "final" setTimeout: it never actually
enforced anything (its rejection was never attached to the returned
promise chain) and only produced a stray unhandled rejection whenever a
call was still pending at the 20000ms mark -- harmless while the real
call timeout was a shorter 10000ms, but a direct collision once the
default here also became 20000ms. The real timeout enforcement was
always the outer timeoutPromise() wrapping this method's result.

Turns green the previously-red test added in the prior commit
(guest.test.ts's "does not time out a call that resolves at 12000ms").
Updates the two tests that characterized the old 10000ms-regardless-of-
config bug to assert the new, decoupled, configurable behavior instead,
and adds a test for the new callTimeout override.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The package-lock.json committed alongside the new uix-guest test
infrastructure resolved its freshly-added typescript devDependency
through an Adobe-internal Artifactory proxy (a local machine default,
not this repo's configured registry -- see .npmrc) at a version
Artifactory had never cached. CI's npm ci then failed trying to fetch
it through Artifactory with UNABLE_TO_GET_ISSUER_CERT_LOCALLY, since
that path isn't what .npmrc/CI actually use.

Regenerated by wiping node_modules and running npm ci explicitly
against registry.npmjs.org (matching .npmrc), so every entry resolves
the way CI expects. Verified: npm ci, build, declarations, and the
full unit suite all pass cleanly on the regenerated lockfile.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comments in guest.test.ts, host.test.ts, and port.test.ts pointed to
docs/extension-loading-timeout-investigation.md, which lives in an
internal mono repo outside of this open-source repo rather than in
this tree. Removed the dangling path references while keeping the
surrounding explanatory context intact.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

1 participant