Skip to content

Improve handling of webworker failures. - #1538

Merged
dpvc merged 3 commits into
developfrom
better-worker-failures
Sep 1, 2026
Merged

dpvc merged 3 commits into
developfrom
better-worker-failures

Conversation

@dpvc

@dpvc dpvc commented Aug 5, 2026

Copy link
Copy Markdown
Member

This PR sets things up to better handle failures in landing the speech worker or its maps, rather than just crash.

In KeyExplorer.ts, we remove initial , (for when the speech is empty, as it will be if speech has failed). The Word/Math message is used when there should be speech, but speech hasn't been attached (i.e., speech has failed to be produced). This way, if the page is being read, you will at least get "math" as the output rather than nothing at all.

In speech.ts, the errors are always strings, not Error objects, so fixed that. In speechError(), we only report non-empty messages (this is to help avoid getting a speech error for every math expression on the page when SRE fails; see WebWorker.ts for more).

In GeneratorPool.ts, if the worker failed to start, we produce a message indicating that rather than no speech or braille. An alternative (in a comment) is to use the text content of the selected element, but that might be pretty ugly. On the other hand, it would at least be specific to the node being walked.

In WebWorker.ts, we only report a termination warning once if the warning is the same. That way, if the worker doesn't load, you don't get a redundant warning for every expression on the page, but just one warning. We also don't throw an error when terminating if the worker hasn't started (since we need to terminate the tasks). Finally, we add some commands to handle failure to start the worker, and failure to load a map file. We check to see if the error already includes the path that failed, and if not add it ourselves.

The speech-worker.ts file adds error handling to the getMap() methods so that errors are reported back to MathJax rather than just logged to the console.

Similarly, the HTMLAdaptor and NodeMixin adaptor are modified to include error trapping when the worker is started up.

Finally, the asyncLoad() function is fixed to handle node: references, which was lost somewhere along the line in the locale branch.

@dpvc
dpvc requested a review from zorkow August 5, 2026 13:05
@dpvc dpvc added this to the v4.2 milestone Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 6.25000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.02%. Comparing base (a8088bb) to head (18ec3a9).
⚠️ Report is 70 commits behind head on develop.

Files with missing lines Patch % Lines
ts/adaptors/NodeMixin.ts 0.00% 8 Missing ⚠️
ts/adaptors/HTMLAdaptor.ts 0.00% 5 Missing ⚠️
ts/util/AsyncLoad.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1538      +/-   ##
===========================================
+ Coverage    86.93%   87.02%   +0.09%     
===========================================
  Files          388      390       +2     
  Lines        87571    88131     +560     
  Branches      3290     5042    +1752     
===========================================
+ Hits         76129    76697     +568     
+ Misses       11442    11414      -28     
- Partials         0       20      +20     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zorkow zorkow 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.

lgtm.

Comment thread ts/a11y/speech/GeneratorPool.ts
Comment thread ts/a11y/speech/GeneratorPool.ts
@dpvc

dpvc commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

I have added a live-region message when the web worker fails to start up. This again brings up the question about how to add new localized messages. This branch only has the English and German localizations, but develop now has AI-generated versions for the other locales. How do we handle that situation? Should I merge develop into this branch, and add the missing key into those other files?

@dpvc
dpvc requested a review from zorkow August 28, 2026 11:03
@dpvc

dpvc commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

In any case, please re-review the changes for the live region.

@zorkow

zorkow commented Sep 1, 2026

Copy link
Copy Markdown
Member

I have added a live-region message when the web worker fails to start up. This again brings up the question about how to add new localized messages. This branch only has the English and German localizations, but develop now has AI-generated versions for the other locales. How do we handle that situation? Should I merge develop into this branch, and add the missing key into those other files?

We let CrowdIn handle this as soon as this PR has landed on develop.

@zorkow zorkow 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.

Changes look good. Just one suggestion for the German message.

Comment thread ts/a11y/speech/__locales__/de.json Outdated
Co-authored-by: Volker Sorge <v.sorge@mathjax.org>
@dpvc

dpvc commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

We let CrowdIn handle this as soon as this PR has landed on develop.

So I assume that means that CrowdIn takes care of propagating the new key to the other files, and marks them as needing translation?

@dpvc
dpvc merged commit 72f812e into develop Sep 1, 2026
2 of 3 checks passed
@dpvc
dpvc deleted the better-worker-failures branch September 1, 2026 11:30
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