fix: webview - retry service worker registration and reload webview by dnch13 · Pull Request #333833 · microsoft/vscode

D Fnx (dnch13) · 2026-09-01T18:28:35Z

Bugfix is related to SUPER hot issue - 125993 (Webview crash)

Service worker registration in webviews can transiently fail, e.g. with "InvalidStateError: The document is in an invalid state" when the document is not fully active during registration, which previously surfaced a fatal "Error loading webview" notification that could only be resolved by reloading the whole window.

pre/index.html: retry registration with backoff (1s/1s/2s/3s/5s) and clean up possibly corrupted service worker state (caches / registrations on desktop) between attempts. Fail fast on permanent conditions (user denied permission, InvalidStateError) since retrying cannot recover the current document.
webviewElement: on fatal service worker registration errors, reload the webview into a fresh document (up to 5 times with backoff) before surfacing the error to the user, and offer a Reload Webview action with the final error.

…nsient failures Service worker registration in webviews can transiently fail, e.g. with "InvalidStateError: The document is in an invalid state" when the document is not fully active during registration, which previously surfaced a fatal "Error loading webview" notification that could only be resolved by reloading the whole window. - pre/index.html: retry registration with backoff (1s/1s/2s/3s/5s) and clean up possibly corrupted service worker state (caches / registrations on desktop) between attempts. Fail fast on permanent conditions (user denied permission, InvalidStateError) since retrying cannot recover the current document. - webviewElement: on fatal service worker registration errors, reload the webview into a fresh document (up to 5 times with backoff) before surfacing the error to the user, and offer a Reload Webview action with the final error.

Copilot

Pull request overview

Adds layered recovery for transient webview service-worker registration failures.

Changes:

Retries registration with backoff and state cleanup.
Reloads failed webviews before reporting fatal errors.
Adds a final “Reload Webview” notification action.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`webviewElement.ts`	Adds host-side reload retries and recovery notification.
`pre/index.html`	Adds registration retries and cleanup logic.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

D Fnx (dnch13) · 2026-09-02T08:01:45Z

@microsoft-github-policy-service agree

- Clear webview caches only on desktop, where each webview has an isolated origin. In the browser, webviews share an origin, service worker, and cache storage, so clearing them would evict resources for every other open webview. Update the inline script CSP hash. - Ignore duplicate registration failure reports while a reload is already scheduled, so repeated content events from a failed document cannot exhaust the retry budget before any reload happens. - Track mount-time drag listeners in a per-mount disposable store that is cleared on each mount, so repeated remounts no longer accumulate duplicate listeners. - Guard the Reload Webview notification action against disposed or detached webviews.

D Fnx (dnch13) · 2026-09-02T08:08:27Z

paul-cheung0607 ayo, since this fix is solving really a ridiculous problem, bet would be better if getting steamlined ASAP (just stating that process is not as important as people's time).

Thank you.

D Fnx (dnch13) · 2026-09-03T09:10:20Z

Matt Bierner (@mjbvz) Robo (@deepak1556)
Ayo! Since this bug is eating tons of people's time, just wanted to warn a few core vscode guys. So say, to wake up, open eyes, this thing needs at least temporary solution not to waste so much time.

Copilot

🟡 Changes recommended

Retry handling can lose messages and cleanup can disrupt other webviews sharing the same origin.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 2
Review effort level: Balanced

- Only clear webview caches and unregister service workers on the final in-document registration attempt. Cache storage and registrations are scoped to the webview origin, which may be shared by several webview instances at once (e.g. notebook webviews of one view type, chat output webviews of one renderer), so cleaning up for a transient failure of one instance must not evict state still in use by the others. - When a service worker reload is scheduled, transition the webview out of Ready and close the failed document's message port right away, so messages sent during the backoff are queued instead of being written to the dead port. Preserve the pending message queue across the reload and replay it once the new document becomes ready.

Copilot

🟡 Changes recommended

The host-side error matcher misses serialized Error: messages, preventing the intended webview reload recovery.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/vs/workbench/contrib/webview/browser/webviewElement.ts:649

This condition never matches the registration failures emitted by pre/index.html: the rejected Error is sent with e + '', so the message starts with Error: Could not register service worker:. As a result, the host-side reload path—including recovery for InvalidStateError—is skipped and the original fatal notification is shown immediately. Match the serialized Error: prefix (or send error.message from the preloader).

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

After the service worker reload retry budget is exhausted, the failed document keeps reporting the same registration error for every subsequent content update. This would re-log the error, recreate the notification, and refire onFatalError for each update. Track the terminal failure and suppress further registration errors until the webview is reinitialized with a fresh document, which starts a fresh retry cycle.

Copilot

🟡 Changes recommended

Pending retries and message listeners are not fully scoped to repeated webview mounts.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/vs/workbench/contrib/webview/browser/webviewElement.ts:480

The new per-mount store only owns the drag listeners; _registerMessageHandler still registers its window listener on the class store. If mountTo runs again before the prior document sends webview-ready, that old listener survives, and after the next ready event the additional listeners return because _messagePort is already set without disposing themselves. Have _registerMessageHandler return its disposable and add it to this per-mount store as well.

	private readonly _mountListeners = this._register(new DisposableStore());

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

reinitializeAfterDismount can also be invoked by external callers, such as overlay webview remounts, while a service worker retry backoff is still pending. The armed timeout would then fire later and reload the newly initialized document again, interrupting its content and messages. Cancel the pending retry when reinitializing.

Copilot

🔵 Needs a closer look

The fatal-error matcher excludes the serialized Error: prefix, preventing the host-side recovery path from running.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/vs/workbench/contrib/webview/browser/webviewElement.ts:655

The retry branch never sees the emitted registration errors: pre/index.html sends e + '', so an Error arrives as Error: Could not register service worker: .... Because this regex is anchored directly at Could, all such failures take the generic path and the webview is never reloaded or offered the new reload action. Accept the serialized Error: prefix (while retaining the colon so the permission-denied variant still fails fast).

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Balanced

The webview document reports the workerReady rejection using e + '', which serializes through Error.prototype.toString as 'Error: Could not register service worker: ...'. The retry matcher was anchored at 'Could', so it never matched and every registration failure took the generic fatal-error path without the reload recovery. Accept the serialized 'Error: ' prefix while keeping the trailing colon so the non-retryable third-party-cookie variant still fails fast.

Copilot

🟡 Changes recommended

Retry classification and lifecycle handling currently cause unnecessary reloads and retained listeners or notifications.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/vs/workbench/contrib/webview/browser/webviewElement.ts:715

The prompt handle is discarded even though its Reload action captures this. If the editor closes—as the callback explicitly anticipates—the notification can remain in the notification center with a button that silently does nothing, while retaining the disposed webview and its content. Keep the handle in a lifecycle-owned disposable and close it when the webview is disposed or when a replacement prompt is created.

src/vs/workbench/contrib/webview/browser/webviewElement.ts:480

The new mount-scoped store does not include the window message listener created by _registerMessageHandler at line 497, which still registers into the class-wide store. Every retry/remount therefore leaves another disposable retained until the whole webview is disposed (and a mount that never reaches webview-ready leaves its listener active). Move that registration into this mount scope, or manage it with a mount-scoped MutableDisposable.

	/**
	 * Listeners that are scoped to a single mount. `mountTo` can be called
	 * repeatedly over the lifetime of the webview (e.g. when retrying a
	 * failed service worker registration), so these are replaced on each
	 * mount instead of accumulating.
	 */
	private readonly _mountListeners = this._register(new DisposableStore());

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

…nent The document-side fail-fast for 'user denied permission' errors only skips the in-document registration retries. The catch wrapping the error for the host kept the !onElectron guard, so on Electron the permission error was serialized into the retryable colon form and the host burned all five document reloads on a permanent condition. Report permission denials in the period-terminated non-retryable form on every platform, and have the host side also refuse to retry such messages as defense in depth.

Copilot

🔵 Needs a closer look

Retry-state handling, desktop error guidance, and notification lifetime issues remain.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/vs/workbench/contrib/webview/browser/webviewElement.ts:686

Elapsed time between failure reports does not show that the webview was working. If register()/update() takes over 60 seconds before rejecting, every failed fresh document resets the counter here, so the host can reload forever and never surface the terminal error. Reset the budget only after an explicit successful worker/content-ready signal, rather than when the next failure arrives.
src/vs/workbench/contrib/webview/browser/webviewElement.ts:722
The prompt action closes over this, so allowing the notification to outlive a closed editor retains the disposed WebviewElement (and its object graph) until the user manually clears the notification; the action is a no-op by then anyway. Track the returned notification handle in a MutableDisposable and close it when this webview is disposed or reinitialized.

src/vs/workbench/contrib/webview/browser/pre/index.html:364

Removing the !onElectron guard makes desktop permission failures tell users to enable third-party cookies, but that browser setting is not an actionable remedy in desktop VS Code. Keep the period-prefixed non-retryable form for the host classifier, while retaining the third-party-cookie guidance only on web.

					if (error instanceof Error && error.message.includes('user denied permission')) {
						// A permission denial is a permanent condition; neither retrying
						// here nor the host reloading the document can fix it. Report it
						// in the period-terminated form so that the host does not
						// classify it as retryable and burn its reload budget.
						return reject(new Error(`Could not register service worker. Please make sure third party cookies are enabled: ${error}`));

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Balanced

Reset the reload retry budget only when a new document actually becomes ready (webview-ready), not based on elapsed time between failures: a slow registration that rejects more than 60s after the previous failure would otherwise reset the budget on every attempt and reload forever without surfacing the terminal error. Track the terminal-error notification in a registered MutableDisposable so it is closed when the webview is disposed or reinitialized instead of keeping the disposed webview alive through its action closure. On desktop, report permission-denied service worker failures without the browser-oriented third-party cookie guidance, while keeping the period-terminated non-retryable message form on all platforms.

Copilot

🟡 Changes recommended

The notification handle violates the disposable type constraint, and retry remounts retain stale window listeners.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/vs/workbench/contrib/webview/browser/webviewElement.ts:480

The new mount-scoped store does not include the window listener created by _registerMessageHandler, which still uses this._register on every mount. A failed document never sends webview-ready, so retries retain each prior listener; after a successful retry, only the first listener disposes while the others return because _messagePort is already set. Scope that subscription to this store (or a dedicated MutableDisposable) as well.

	private readonly _mountListeners = this._register(new DisposableStore());

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

… mount INotificationHandle exposes close() but does not implement IDisposable, so it cannot be stored in MutableDisposable<T extends IDisposable> directly. Store an IDisposable wrapper that closes the handle instead. The webview-ready message listener was registered on the webview element's lifetime while mountTo can run repeatedly (service worker retries, overlay transfers). A failed document never sends webview-ready, so each remount retained another window listener; after a successful retry only the first listener disposed itself. Register the listener in the mount-scoped listener store instead.

Copilot

🟡 Changes recommended

The readiness handshake resets retries prematurely, causing infinite reloads and potential message loss.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/vs/workbench/contrib/webview/browser/webviewElement.ts:718

This only preserves messages queued after the fatal error changes _state back to Initializing. Messages queued before the initial webview-ready are already flushed to the outer document, where pre/index.html holds them in its own pendingMessages; when registration fails and this document is replaced, those messages (and transferred buffers) are lost even though their postMessage promises resolved true. Keep application messages host-side until the pre document confirms that service-worker/content initialization succeeded, then flush them.

				const pendingMessages = this._state.type === WebviewState.Type.Initializing ? this._state.pendingMessages : [];
				this._state = new WebviewState.Initializing(pendingMessages);

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

…webview-ready is sent by the pre-document as soon as the message port is\nestablished, before service worker registration settles. Resetting the\nretry budget (and flushing queued messages) on it meant every failed\nreplacement document reset the budget before reporting its own failure,\nso a persistent InvalidStateError reloaded the webview every second\nforever, and messages flushed into a doomed document were lost when it\nwas replaced.\n\n- pre/index.html reports the outcome of workerReady directly: success\n posts a new worker-ready channel, failure posts fatal-error. The\n content handler catch remains as a backstop only.\n- webviewElement keeps the state Initializing after webview-ready and\n only flushes pending messages, transitions to Ready, and resets the\n retry budget on worker-ready, so messages queued before or after a\n document failure survive document replacement host-side and are\n replayed once a fresh document succeeds.\n- webviewMessages.d.ts adds the worker-ready channel.\n\nA persistent registration failure now exhausts the five reload retries\n(~12s) and surfaces the terminal notification instead of retrying\nforever. CSP hash updated for the index.html change (double-run\nverified HASH-UNCHANGED).

Copilot

🟡 Changes recommended

Queue replay can lose extension messages, and a permanent unsupported-service-worker condition is unnecessarily retried.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/vs/workbench/contrib/webview/browser/webviewElement.ts:807

Preserving the whole queue here conflicts with the unconditional reload() below, which appends another content message after queued extension messages. A normal overlay can therefore flush [content, message, styles, content]; the second content update clears pre/index.html's pending message queue at line 1118, so the extension message is never delivered. Rebuild the queue with one current content update before replayable non-content messages, while settling superseded sends.

		const pendingMessages = this._state.type === WebviewState.Type.Initializing ? this._state.pendingMessages : [];
		this._state = new WebviewState.Initializing(pendingMessages);

Files reviewed: 2/3 changed files
Comments generated: 1
Review effort level: Balanced

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Terminal failures leave messages unresolved, and manual reload cannot recover consumers that already processed onFatalError.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/3 changed files
Comments generated: 2
Review effort level: Balanced

…nTerminal failure paths left the webview in the Initializing state, so\nevery queued _send promise and every later postMessage stayed pending\nforever (only worker-ready drains the queue and dispose() settles it).\nThis affected both the exhausted-retry branch and the non-retryable\nfatal-error branches now that webview-ready no longer transitions to\nReady. Add an explicit WebviewState.Failed: entering it settles queued\nmessages as not delivered, further sends resolve as false, and the dead\ndocument's port is closed. reinitializeAfterDismount returns the webview\nto Initializing (with an empty queue; reload() requeues the content).\n\nThe terminal Reload Webview action could revive the webview element, but\nconsumers that gave up on onFatalError stayed broken: BackLayerWebView\nrejects its one-shot initialization promise and NotebookEditorWidget\ncaches that rejection, so a recovered notebook webview was never used\nagain. Add a recovery contract: WebviewElement fires a new\nonFatalErrorResolved when a document reports worker-ready after a\nprevious fatal error (tracked by a flag that survives reinitialization\nand is only cleared on actual recovery, so the event fires exactly\nonce). The event is declared on IWebview, forwarded by OverlayWebview,\nre-exposed by BackLayerWebView, and NotebookEditorWidget replaces the\ncached rejected _webviewResolvePromise with a resolved one — no\nre-running of createWebview, since the recovered document re-posts\n'initialized' which resynchronizes the back layer's state.

D Fnx (dnch13) · 2026-09-05T07:48:40Z

Robo (@deepak1556) ayo, bro, i can't stand copilot, this thing drives me out.... i just wanted to show better path: bug resolved -> people do not spend excessive time on reopening IDE (solution may be dirty, save time = priority) -> software delivered -> everyone is happy.
Hopefully would be better to speed it up, not sure about the method.

D Fnx (dnch13) · 2026-09-05T07:50:02Z

Robo (@deepak1556) you may also try 200 line version before copilot ***p, thx

Copilot AI balanced review requested due to automatic review settings September 1, 2026 18:28

Copilot started reviewing on behalf of D Fnx (dnch13) September 1, 2026 18:29 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

vs-code-engineering Bot added the triage-needed label Sep 1, 2026

vs-code-engineering Bot assigned Paul (pwang347) Sep 1, 2026

Robo (deepak1556) assigned Robo (deepak1556) and unassigned Paul (pwang347) Sep 3, 2026

Robo (deepak1556) removed the triage-needed label Sep 3, 2026

Robo (deepak1556) added this to the 1.137.0 milestone Sep 3, 2026

D Fnx (dnch13) requested a balanced review from Copilot September 5, 2026 05:05

Copilot started reviewing on behalf of D Fnx (dnch13) September 5, 2026 05:06 View session