Skip to content

Fix History API loop#2354

Merged
lslezak merged 3 commits intomasterfrom
fix_history_api_loop
May 13, 2025
Merged

Fix History API loop#2354
lslezak merged 3 commits intomasterfrom
fix_history_api_loop

Conversation

@lslezak
Copy link
Contributor

@lslezak lslezak commented May 12, 2025

Problem

Debugging

To debug the issue I have reverted the #2281 fix and added a small snippet which tracks using the History API into the web/src/index.tsx file:

let counter = 1;
const original = window.history.pushState;
window.history.pushState = function (data, unused, url) {
  console.log("pushState: ", counter, data, url);
  counter += 1;
  original.apply(this, [data, unused, url]);
};

That revealed that the History API was switching quickly between the product selection page and the probing progress page:

agama-too-many-push-states-orig

The Firefox browser has a limit of 200 History API calls within 10 seconds. After that an error is reported and an exception is raised. That exception causes a crash in the React router resulting in empty screen.

The problematic code was in the main application router:

if (selectedProduct === undefined && location.pathname !== PRODUCT.root) {
  return <Navigate to={PRODUCT.root} />;
}

if (phase === InstallationPhase.Config && isBusy && location.pathname !== PRODUCT.progress) {
  return <Navigate to={PRODUCT.progress} />;
}

The problem happened when the selected product was not updated yet (selectedProduct === undefined) but probing was already in progress (isBusy === true). In that case these conditions caused switching between the two paths as fast as possible. On my machine it was switching about 1000 times per second as displayed in the log above.

A similar problem was present in the product selection page.

Solution

  • Handle this case explicitly, if that happens display the loading page until the selected product is updated.

Testing

  • Tested manually, registration from CLI works fine even without that delay workaround
  • There are just few calls to the History API
    agama-too-many-push-states-fix1

lslezak added 2 commits May 12, 2025 14:55
- Avoid quickly switching between two paths in a loop causing "Too many
calls to Location or History APIs within a short timeframe" error in Firefox
and crashing the UI
- This is the proper fix for gh##2274
- The problem has been fixed in the web UI
- Removed workaround for gh##2274
Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

That's indeed a better fix than previous. Thanks for taking the time to investigate it and for the information added into the PR description. Much appreciated. Nice work 👍

@dgdavid
Copy link
Contributor

dgdavid commented May 12, 2025

BTW, this reminds me that we would like to improve both, the entire thing about the backend status (installation phase and isBusy stuff) and the App.tsx internal router... when time permits.

const [isLoading, setIsLoading] = useState(false);

if (!isEmpty(registration?.key)) return <Navigate to={PATHS.root} />;
if (!isEmpty(registration?.key) && selectedProduct) return <Navigate to={PATHS.root} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird to have the registration key set but the product not selected yet.

Copy link
Contributor Author

@lslezak lslezak May 12, 2025

Choose a reason for hiding this comment

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

That's because the product has been asynchronously selected from CLI in background. When using UI only this cannot happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. Make sense.

@dgdavid
Copy link
Contributor

dgdavid commented May 12, 2025

BTW, this reminds me that we would like to improve both, the entire thing about the backend status (installation phase and isBusy stuff) and the App.tsx internal router... when time permits.

Using @imobachgs words

yes; the whole progress/status machinery needs some rework

@lslezak lslezak merged commit 0e7367b into master May 13, 2025
7 of 9 checks passed
@lslezak lslezak deleted the fix_history_api_loop branch May 13, 2025 06:40
@imobachgs imobachgs mentioned this pull request May 26, 2025
imobachgs added a commit that referenced this pull request May 26, 2025
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