[PR #7656] [MERGED] Fix shared worker resumption after tab suspend #56619

Closed
opened 2026-05-01 04:36:18 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/7656
Author: @jfdoming
Created: 4/29/2026
Status: Merged
Merged: 4/30/2026
Merged by: @jfdoming

Base: masterHead: jfdoming/sharedworker-resume-split


📝 Commits (4)

  • 5dca064 [AI] Fix SharedWorker tab resume recovery
  • b441135 [AI] Fix SharedWorker reload readiness
  • d486f40 Add release notes
  • 964ba5a Update packages/desktop-client/src/shared-browser-server-core.ts

📊 Changes

4 files changed (+418 additions, -27 deletions)

View changed files

📝 packages/desktop-client/src/browser-preload.js (+63 -9)
📝 packages/desktop-client/src/shared-browser-server-core.test.ts (+203 -5)
📝 packages/desktop-client/src/shared-browser-server-core.ts (+146 -13)
upcoming-release-notes/7656.md (+6 -0)

📄 Description

Description

Note: I used AI pretty aggressively for this PR since I wasn't familiar with this part of the codebase. I understand the architecture a bit better now as well as the solution, but very open to input on the PR since the code is not mine.

This PR fixes two bugs with multi-tab coordination that could leave tabs loading forever in certain cases.

First, in Safari, tabs are naturally suspended after a certain amount of time. When a tab is suspended, it is no longer able to keep up with heartbeats to the coordinator, which means it will be removed from the coordinator's internal state. When such a tab resumes, it uses the same MessagePort it was already assigned, but with no other tabs open, the coordinator wouldn't know which budget the port refers to, leaving the tab waiting forever. (This was the bug found by Claude in the other thread.) Fixed by adding explicit logic for resuming the shared worker connection, and attempting to reattach to the same budget group only if it still exists on the same port.

Second, when the leader tab is disconnected, the coordinator will fail the budget over to a new tab. In the specific scenario of a tab reload (encountered a lot during my testing), the failover process and the reloaded-tab reconnection logic occur in close succession, meaning that the reloaded tab would likely be told the backend is ready before the newly-promoted leader actually had a chance to load the budget. This meant that the reloaded tab would likely hang forever due to the connection error. Fixed by delaying the worker connect message until the new leader has actually finished loading.

Fixes https://github.com/actualbudget/actual/issues/7598

Testing

Would love some help testing this further! But I tested this in two ways:
(a) I found a process locally with Safari that reproduced the initial issue semi-reliably: I enabled the Safari Debug menu, then toggled "Always Suspend WebProcesses Aggressively" under "Miscellaneous Flags". (Not 100% sure this was necessary.) Then opened a bunch of tabs, unfocused the Actual window, and waited ~5 minutes. Without this PR, that reproduced the linked issue semi-reliably; with this PR, I was no longer able to reproduce.
(b) I opened two tabs next to each other, and tried various combinations of reloading the tabs and making changes. This is how I discovered the second bug, and with this PR I was no longer able to reproduce that bug.

Checklist

  • Release notes added (see link above)
  • No obvious regressions in affected areas
  • Self-review has been performed - I understand what each change in the code does and why it is needed

Bundle Stats

Bundle Files count Total bundle size % Changed
desktop-client 35 14.01 MB → 14.01 MB (+1.18 kB) +0.01%
loot-core 1 5.26 MB 0%
api 2 3.89 MB 0%
cli 1 7.97 MB 0%
crdt 1 41.83 kB 0%
View detailed bundle stats

desktop-client

Total

Files count Total bundle size % Changed
35 14.01 MB → 14.01 MB (+1.18 kB) +0.01%
Changeset
File Δ Size
src/browser-preload.js 📈 +1.18 kB (+13.79%) 8.53 kB → 9.7 kB
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger

Asset File Size % Changed
static/js/index.js 1.87 MB → 1.87 MB (+1.18 kB) +0.06%

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
static/js/BackgroundImage.js 121.09 kB 0%
static/js/FormulaEditor.js 962.55 kB 0%
static/js/PayeeRuleCountLabel.js 52.52 kB 0%
static/js/ReportRouter.js 1.22 MB 0%
static/js/ScheduleEditForm.js 145.68 kB 0%
static/js/TransactionEdit.js 189.54 kB 0%
static/js/TransactionList.js 85.81 kB 0%
static/js/Value.js 4.94 MB 0%
static/js/ca.js 191.46 kB 0%
static/js/chart-theme.js 796.5 kB 0%
static/js/client.js 451.37 kB 0%
static/js/da.js 104.22 kB 0%
static/js/de.js 173.88 kB 0%
static/js/en-GB.js 8.2 kB 0%
static/js/en.js 176.89 kB 0%
static/js/es.js 182.3 kB 0%
static/js/extends.js 518.76 kB 0%
static/js/fr.js 182.5 kB 0%
static/js/indexeddb-main-thread-worker-e59fee74.js 13.46 kB 0%
static/js/it.js 168.33 kB 0%
static/js/narrow.js 364.31 kB 0%
static/js/nb-NO.js 151.39 kB 0%
static/js/nl.js 108.46 kB 0%
static/js/pl.js 88.14 kB 0%
static/js/pt-BR.js 193.24 kB 0%
static/js/resize-observer.js 18.06 kB 0%
static/js/ru.js 121.6 kB 0%
static/js/th.js 178.63 kB 0%
static/js/theme.js 31.67 kB 0%
static/js/uk.js 212.03 kB 0%
static/js/useFormatList.js 8.63 kB 0%
static/js/wide.js 453 B 0%
static/js/workbox-window.prod.es5.js 7.33 kB 0%
static/js/zh-Hans.js 119.88 kB 0%

loot-core

Total

Files count Total bundle size % Changed
1 5.26 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.B5tkfYkU.js 5.26 MB 0%

api

Total

Files count Total bundle size % Changed
2 3.89 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
index.js 3.89 MB 0%
models.js 0 B 0%

cli

Total

Files count Total bundle size % Changed
1 7.97 MB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
cli.js 7.97 MB 0%

crdt

Total

Files count Total bundle size % Changed
1 41.83 kB 0%
View detailed bundle breakdown

Added
No assets were added

Removed
No assets were removed

Bigger
No assets were bigger

Smaller
No assets were smaller

Unchanged

Asset File Size % Changed
index.js 41.83 kB 0%

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/7656 **Author:** [@jfdoming](https://github.com/jfdoming) **Created:** 4/29/2026 **Status:** ✅ Merged **Merged:** 4/30/2026 **Merged by:** [@jfdoming](https://github.com/jfdoming) **Base:** `master` ← **Head:** `jfdoming/sharedworker-resume-split` --- ### 📝 Commits (4) - [`5dca064`](https://github.com/actualbudget/actual/commit/5dca064e1288a86ef81ca922fafe937774026bae) [AI] Fix SharedWorker tab resume recovery - [`b441135`](https://github.com/actualbudget/actual/commit/b4411358cf27887e71434d4569021df33b1e7fdf) [AI] Fix SharedWorker reload readiness - [`d486f40`](https://github.com/actualbudget/actual/commit/d486f40e17ca8c19b6ccd94b55d755b6bad28b58) Add release notes - [`964ba5a`](https://github.com/actualbudget/actual/commit/964ba5a5d807ab26778b6a66f77a3684395ff3b5) Update packages/desktop-client/src/shared-browser-server-core.ts ### 📊 Changes **4 files changed** (+418 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `packages/desktop-client/src/browser-preload.js` (+63 -9) 📝 `packages/desktop-client/src/shared-browser-server-core.test.ts` (+203 -5) 📝 `packages/desktop-client/src/shared-browser-server-core.ts` (+146 -13) ➕ `upcoming-release-notes/7656.md` (+6 -0) </details> ### 📄 Description <!-- Thank you for submitting a pull request! Make sure to follow the instructions to write release notes for your PR — it should only take a minute or two: https://github.com/actualbudget/docs#writing-good-release-notes. Try running yarn generate:release-notes *before* pushing your PR for an interactive experience. --> ## Description _Note: I used AI pretty aggressively for this PR since I wasn't familiar with this part of the codebase. I understand the architecture a bit better now as well as the solution, but very open to input on the PR since the code is not mine._ This PR fixes two bugs with multi-tab coordination that could leave tabs loading forever in certain cases. First, in Safari, tabs are naturally suspended after a certain amount of time. When a tab is suspended, it is no longer able to keep up with heartbeats to the coordinator, which means it will be removed from the coordinator's internal state. When such a tab resumes, it uses the same `MessagePort` it was already assigned, but with no other tabs open, the coordinator wouldn't know which budget the port refers to, leaving the tab waiting forever. (This was the bug found by Claude in the other thread.) Fixed by adding explicit logic for resuming the shared worker connection, and attempting to reattach to the same budget group only if it still exists on the same port. Second, when the leader tab is disconnected, the coordinator will fail the budget over to a new tab. In the specific scenario of a tab reload (encountered a lot during my testing), the failover process and the reloaded-tab reconnection logic occur in close succession, meaning that the reloaded tab would likely be told the backend is ready before the newly-promoted leader actually had a chance to load the budget. This meant that the reloaded tab would likely hang forever due to the connection error. Fixed by delaying the worker connect message until the new leader has actually finished loading. ## Related issue(s) Fixes https://github.com/actualbudget/actual/issues/7598 ## Testing Would love some help testing this further! But I tested this in two ways: (a) I found a process locally with Safari that reproduced the initial issue semi-reliably: I enabled the Safari Debug menu, then toggled "Always Suspend WebProcesses Aggressively" under "Miscellaneous Flags". (Not 100% sure this was necessary.) Then opened a bunch of tabs, unfocused the Actual window, and waited ~5 minutes. Without this PR, that reproduced the linked issue semi-reliably; with this PR, I was no longer able to reproduce. (b) I opened two tabs next to each other, and tried various combinations of reloading the tabs and making changes. This is how I discovered the second bug, and with this PR I was no longer able to reproduce that bug. ## Checklist - [x] Release notes added (see link above) - [x] No obvious regressions in affected areas - [x] Self-review has been performed - I understand what each change in the code does and why it is needed <!--- actual-bot-sections ---> <!--- bundlestats-action-comment key:combined start ---> ### Bundle Stats Bundle | Files count | Total bundle size | % Changed ------ | ----------- | ----------------- | --------- desktop-client | 35 | 14.01 MB → 14.01 MB (+1.18 kB) | +0.01% loot-core | 1 | 5.26 MB | 0% api | 2 | 3.89 MB | 0% cli | 1 | 7.97 MB | 0% crdt | 1 | 41.83 kB | 0% <details> <summary>View detailed bundle stats</summary> #### desktop-client **Total** Files count | Total bundle size | % Changed ----------- | ----------------- | --------- 35 | 14.01 MB → 14.01 MB (+1.18 kB) | +0.01% <details> <summary>Changeset</summary> File | Δ | Size ---- | - | ---- `src/browser-preload.js` | 📈 +1.18 kB (+13.79%) | 8.53 kB → 9.7 kB </details> <details> <summary>View detailed bundle breakdown</summary> <div> **Added** No assets were added **Removed** No assets were removed **Bigger** Asset | File Size | % Changed ----- | --------- | --------- static/js/index.js | 1.87 MB → 1.87 MB (+1.18 kB) | +0.06% **Smaller** No assets were smaller **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- static/js/BackgroundImage.js | 121.09 kB | 0% static/js/FormulaEditor.js | 962.55 kB | 0% static/js/PayeeRuleCountLabel.js | 52.52 kB | 0% static/js/ReportRouter.js | 1.22 MB | 0% static/js/ScheduleEditForm.js | 145.68 kB | 0% static/js/TransactionEdit.js | 189.54 kB | 0% static/js/TransactionList.js | 85.81 kB | 0% static/js/Value.js | 4.94 MB | 0% static/js/ca.js | 191.46 kB | 0% static/js/chart-theme.js | 796.5 kB | 0% static/js/client.js | 451.37 kB | 0% static/js/da.js | 104.22 kB | 0% static/js/de.js | 173.88 kB | 0% static/js/en-GB.js | 8.2 kB | 0% static/js/en.js | 176.89 kB | 0% static/js/es.js | 182.3 kB | 0% static/js/extends.js | 518.76 kB | 0% static/js/fr.js | 182.5 kB | 0% static/js/indexeddb-main-thread-worker-e59fee74.js | 13.46 kB | 0% static/js/it.js | 168.33 kB | 0% static/js/narrow.js | 364.31 kB | 0% static/js/nb-NO.js | 151.39 kB | 0% static/js/nl.js | 108.46 kB | 0% static/js/pl.js | 88.14 kB | 0% static/js/pt-BR.js | 193.24 kB | 0% static/js/resize-observer.js | 18.06 kB | 0% static/js/ru.js | 121.6 kB | 0% static/js/th.js | 178.63 kB | 0% static/js/theme.js | 31.67 kB | 0% static/js/uk.js | 212.03 kB | 0% static/js/useFormatList.js | 8.63 kB | 0% static/js/wide.js | 453 B | 0% static/js/workbox-window.prod.es5.js | 7.33 kB | 0% static/js/zh-Hans.js | 119.88 kB | 0% </div> </details> --- #### loot-core **Total** Files count | Total bundle size | % Changed ----------- | ----------------- | --------- 1 | 5.26 MB | 0% <details> <summary>View detailed bundle breakdown</summary> <div> **Added** No assets were added **Removed** No assets were removed **Bigger** No assets were bigger **Smaller** No assets were smaller **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- kcab.worker.B5tkfYkU.js | 5.26 MB | 0% </div> </details> --- #### api **Total** Files count | Total bundle size | % Changed ----------- | ----------------- | --------- 2 | 3.89 MB | 0% <details> <summary>View detailed bundle breakdown</summary> <div> **Added** No assets were added **Removed** No assets were removed **Bigger** No assets were bigger **Smaller** No assets were smaller **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- index.js | 3.89 MB | 0% models.js | 0 B | 0% </div> </details> --- #### cli **Total** Files count | Total bundle size | % Changed ----------- | ----------------- | --------- 1 | 7.97 MB | 0% <details> <summary>View detailed bundle breakdown</summary> <div> **Added** No assets were added **Removed** No assets were removed **Bigger** No assets were bigger **Smaller** No assets were smaller **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- cli.js | 7.97 MB | 0% </div> </details> --- #### crdt **Total** Files count | Total bundle size | % Changed ----------- | ----------------- | --------- 1 | 41.83 kB | 0% <details> <summary>View detailed bundle breakdown</summary> <div> **Added** No assets were added **Removed** No assets were removed **Bigger** No assets were bigger **Smaller** No assets were smaller **Unchanged** Asset | File Size | % Changed ----- | --------- | --------- index.js | 41.83 kB | 0% </div> </details> </details> <!--- bundlestats-action-comment key:combined end ---> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-01 04:36:18 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#56619