Skip to content

Fix Tunnel.toIframe breaking on a duplicate same-key handshake offer (SITES-48958) - #151

Merged
fe-lix- merged 3 commits into
mainfrom
fix/SITES-48958-duplicate-offer-race
Jul 31, 2026
Merged

Fix Tunnel.toIframe breaking on a duplicate same-key handshake offer (SITES-48958)#151
fe-lix- merged 3 commits into
mainfrom
fix/SITES-48958-duplicate-offer-race

Conversation

@fe-lix-

@fe-lix- fe-lix- commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Description

A regression introduced by the SITES-42495 fix (#150): Tunnel.toIframe
could silently break an already-established, healthy connection if the
guest's own handshake retry loop sent one more redundant offer before it
noticed its own tunnel had connected. This PR adds an e2e test that
reproduces the failure deterministically and a small fix that restores
protection against it without reintroducing SITES-42495.

Related Issue

Reported internally as SITES-48958 (Adobe internal Jira - Content Fragment
Editor customer escalation, Casio): https://jira.corp.adobe.com/browse/SITES-48958

No public GitHub issue exists for this - it came in through an internal
customer support ticket, same as SITES-42495.

Motivation and Context

uix-sdk 1.1.10 (containing both #148 and #150) shipped to CF Editor
PROD v1.58.0 on July 30. That same day, a customer (Casio) started
reporting Content Fragment custom fields intermittently stuck on "Loading
custom field..." forever - some fields on a page would render, others
wouldn't, with no error surfaced anywhere.

Root cause: Tunnel.toIframe's offerListener, fixed in #150 to stop
permanently ignoring every offer once connected (so a genuinely reloaded
guest could reconnect), had no way to distinguish that legitimate case from
a much more mundane one - a guest that hasn't reloaded at all, just hasn't
finished processing the host's "accepted" reply yet:

  1. Tunnel.toParent's sendOffer retries every 100ms, using the same
    key, until the guest's own tunnel reports connected - which only
    happens after it receives and processes the host's "accepted" message,
    an async round trip.
  2. If that round trip takes longer than 100ms - plausible when several
    GuestUIFrame instances of the same extension are initializing
    concurrently on one CF Editor page, all competing for main-thread time -
    a second, redundant offer with the same key arrives after the host has
    already connected.
  3. Post-Fix GuestUIFrame losing connection when moved within a stable-keyed list (SITES-42495) #150, the host reprocesses it: opens a second MessageChannel,
    posts a new port to the guest, and calls tunnel.connect() again -
    closing the port pair the guest is actually using and replacing it with
    one paired to a port the guest already stopped listening for (its
    acceptListener unsubscribes right after the first accept).
  4. Both sides now report isConnected: true, but neither can reach the
    other. Nothing throws and nothing times out (the one-shot connection
    timeout was already cleared on the first successful connect) - a silent,
    permanent hang. That matches the "stuck loading" screenshots in the
    ticket exactly, and being a timing race, it only bites some fields on
    some page loads, matching "intermittent."

The fix

Track the offers key of the handshake the tunnel is currently connected
with (acceptedOfferId). A repeat of that same key while already
connected is now ignored as a retry echo. A different key - a genuinely
new Tunnel.toParent instance, i.e. an actual reload - is still treated as
a fresh connection attempt, preserving the SITES-42495 behavior.

How Has This Been Tested?

Added e2e/tests/tests/duplicate-offer-race.js plus supporting fixtures
(e2e/host-app/src/HostAppDuplicateOffer.jsx,
e2e/guest-app/src/DuplicateOfferProbe.jsx, routing wire-up in both apps'
App.js). The scenario can't rely on incidental timing jitter to be
reliable in CI, so it forces the race deterministically: the guest
monkey-patches its own window.parent.postMessage to capture its real
first handshake offer and replay the identical message, from inside its
own realm (so event.source genuinely reflects the guest, not the host),
~800ms after the connection has already succeeded. The guest then makes a
real post-connect RPC call (host.probe.ping()) exposed via GuestUIFrame's
privateMethods.

  • Before the fix: the test fails - "Calling host.probe.ping() timed out after 10000ms" (the SDK's own RPC-call timeout), since the tunnel was
    silently swapped for a dead port pair.
  • After the fix: same test, unmodified, passes.
  • A second test with the duplicate injection disabled (?inject=0) is a
    positive control, confirming the harness itself is sound (ping succeeds
    immediately absent any interference).
  • multifield-reorder.js (SITES-42495) still passes unmodified, confirming
    this doesn't regress the original reconnect-after-reload fix.
  • Full suite run locally via npm run test:e2e: 17/17 passing, no
    regressions to any of the 15 pre-existing e2e tests.
  • npm run test:unit: 15 suites, 83 passed, 2 pre-existing skips (the
    documented JSDOM postMessage/MessageEvent limitation on
    Tunnel.toIframe's own describe block) - no change from before this PR.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation. (No - this
    restores behavior consistent with Tunnel.toIframe's existing doc
    comment; no public API changed.)
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Investigated and authored with Claude Code.

fe-lix- and others added 3 commits July 31, 2026 10:35
…d tunnel

Adds a #/duplicate-offer host scenario where a guest's real first
handshake offer is captured and replayed verbatim, from inside the
guest's own realm, ~800ms after the connection has already succeeded -
simulating Tunnel.toParent's 100ms retry loop firing once more before
noticing its own tunnel had connected. Tunnel.toIframe's offerListener
(after the SITES-42495 fix removed the !tunnel.isConnected guard)
reprocesses that duplicate: it opens a second MessageChannel and calls
tunnel.connect() again, closing the port the guest is still using and
replacing it with one paired to a port the guest already stopped
listening for. Both sides report isConnected, but neither can reach
the other - a silent, permanent hang with no error or timeout, since
the initial connection timeout was already cleared on first connect.

The test asserts the correct behavior (a post-connect RPC call still
succeeds) and currently fails with "Calling host.probe.ping() timed
out after 10000ms". A second test with duplicate injection disabled
(?inject=0) is a positive control, proving the harness itself is
sound.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Iframe

The SITES-42495 fix removed the !tunnel.isConnected guard in
offerListener so a genuinely reloaded guest (a new Tunnel.toParent
instance, with a new key) could reconnect. But it also removed
protection against a *still-connecting* guest re-sending its offer:
Tunnel.toParent's sendOffer retries every 100ms until the guest's own
tunnel reports connected, which only happens after it processes this
host's "accepted" reply - an async round trip. If that takes longer
than 100ms (plausible with several GuestUIFrame instances of the same
extension initializing concurrently and contending for main-thread
time), a second, redundant offer carrying the *same* key arrives after
the host already connected, gets reprocessed, and tears down the
working MessagePort in favor of one the guest already stopped
listening for - both sides report connected while unable to reach
each other, hanging forever with no error (SITES-48958).

Track the key of the offer last accepted and only treat a *different*
key as a fresh connection attempt (an actual reload); a repeat of the
same key while already connected is now ignored as a retry echo.

Verified against e2e/tests/tests/duplicate-offer-race.js, which now
passes without any changes to the test itself, and against
multifield-reorder.js (SITES-42495), which still passes - confirming
this doesn't regress the original reconnect-after-reload fix. Full
suite: 17/17 e2e, 83/85 unit (2 pre-existing skips), no regressions.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Rename the offer's destructured `offers` to `offerKey` in
  offerListener - reads more clearly at the equality check site than
  the wire-protocol field name does as a local variable.
- Set `acceptedOfferId` only after tunnel.connect() succeeds, so it
  unambiguously represents the key of the active connection rather
  than merely the last offer seen, and clear it in cleanup() for
  explicit lifecycle intent (not currently load-bearing - cleanup
  already removes offerListener from window - but this function has
  now caused two regressions from two different people modifying it,
  so the extra clarity is worth it).
- Add a focused unit test for the early-return guard itself, dispatching
  a manually-constructed MessageEvent with an explicit `source` instead
  of a real window.postMessage() call, which sidesteps the jsdom
  limitation that keeps the neighboring describe.skip block skipped
  (jsdom's postMessage still doesn't populate event.source correctly,
  confirmed against jsdom's current source - but MessageEvent's own
  constructor handles source/origin/ports correctly, so dispatching one
  directly bypasses the broken code path entirely). This also surfaced
  and fixes a latent bug in the FakeIframe test helper: contentWindow's
  paired port was discarded, so nothing could ever observe messages the
  SDK sent via contentWindow.postMessage() - exposed as guestSidePort so
  tests can listen on it.

Verified: npm run test:unit (15/15 suites, 84 passed [+1], 2 pre-existing
skips) and npm run test:e2e (17/17 passing, including both the
SITES-42495 and SITES-48958 scenarios).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@fe-lix-
fe-lix- merged commit 4906347 into main Jul 31, 2026
4 checks passed
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