Skip to content

fix: fall back when service worker registration fulfills with undefined - #764

Merged
gioboa merged 1 commit into
QwikDev:mainfrom
MFA-G:fix/snippet-undefined-sw-registration
Oct 1, 2026
Merged

gioboa merged 1 commit into
QwikDev:mainfrom
MFA-G:fix/snippet-undefined-sw-registration

Conversation

@MFA-G

@MFA-G MFA-G commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

The snippet's register().then() success callback reads swRegistration.active unguarded. Some environments fulfill register() with undefined instead of rejecting it. Playwright's serviceWorkers: 'block' does this: I checked in Chromium, and navigator.serviceWorker.register(...) resolves to undefined. The read then throws TypeError: Cannot read properties of undefined (reading 'active') as an unhandled rejection. Partytown only recovers once fallbackTimeout fires (9999 ms by default).

Reported downstream in withastro/astro#18171, which shows this in production on real Chrome/Android user agents as well as HeadlessChrome.

Change

Tests

  • tests/unit/snippet.spec.ts: new case where register() fulfills with undefined. It asserts the partytown script falls back to the main thread. Before the fix it fails with the TypeError.
  • tests/platform/fallback: this test already uses serviceWorkers: 'block'. It passed only because of the 1 s fallbackTimeout while the error went unnoticed. It now also asserts there are no page errors. That assertion fails on main with "Cannot read properties of undefined (reading 'active')" and passes with this change.
  • pnpm test.unit: 52/52. playwright test tests/platform --browser=chromium: 44 passed. fmt.check passes.
  • Added a patch changeset.

@changeset-bot

changeset-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

馃 Changeset detected

Latest commit: 9b3fb1c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@qwik.dev/partytown Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gioboa gioboa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix 馃憦 Thanks @MFA-G

@gioboa
gioboa merged commit 4e42305 into QwikDev:main Oct 1, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 1, 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.

2 participants