Repository navigation
Isolate errors thrown by one element during a shared-timer tick - #398
Closed
mattcosta7 wants to merge 8 commits into
Closed
mattcosta7 wants to merge 8 commits into
mattcosta7 wants to merge 8 commits into
Conversation
A throw while updating one element aborted the whole tick: the elements after it were not updated and the timer was never rescheduled, so every timestamp on the page stopped updating. Each element's `getUnitFactor` and re-render now run in their own try/catch. A failing element is skipped for scheduling when its `getUnitFactor` threw, and its error is rethrown from a zero-delay `setTimeout`, so it still reaches the page as an uncaught error exactly once per throw. `setTimeout` is used rather than `reportError`, which older browsers and non-browser hosts lack and which the shared timer does not otherwise need. Lookups left recording by the failed render are discarded. This covers errors thrown by `getUnitFactor` as well as by the render (a throwing `date` getter, or an element over an hour old whose `time-zone` is invalid and so fails in `msUntilNextDay`). Errors in `connectedCallback` and the attribute-change microtask are unchanged. Supersedes #371, which wrapped only `update()`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…g part way An element whose isConnected getter throws still aborted the tick, because the check ran before the per-element try. It now runs inside it. A new test fails a render after it has started recording formatter lookups and checks that later elements still skip or render as before. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Patching RelativeTimeElement.prototype.update now turns skipping off, so the test counts renders through Element.prototype.hasAttribute instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
# Conflicts: # src/relative-time-element.ts
An element whose update threw after moving itself in the document was added back to the end of the observer's Set and visited again, throwing every time, so the tick never finished. An element that fails is now skipped if the tick reaches it again. Elements added during a tick are still visited, as before. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
# Conflicts: # src/relative-time-element.ts
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #397 (which is stacked on #396). Review only this branch's own commits (
d1745dd,c35c916,19b06cd; the merge commits bring in #397's latest fixes); the base of this PR should beperf-skip-unchanged-ticks.Supersedes #371.
What this fixes
All
<relative-time>elements share one timer. Its tick loops over the elements and, once the loop finishes, schedules the next tick. Onmain, if anything throws while one element is handled, the loop is abandoned: the elements after it are not updated and the next tick is never scheduled. One bad element therefore freezes every timestamp on the page until something else re-observes an element.Why #371 is superseded
#371 wrapped only the
update()call intry/catch. Ship-check found it incomplete:getUnitFactor(timeEl)runs outside thattry. A throwingdategetter, or an element at least an hour old with an invalidtime-zone(msUntilNextDaythrows an IntlRangeError), still aborted the whole tick and stopped rescheduling.main, before the observer changes in Stop retaining disconnected elements in the shared date observer #396 and Skip shared-timer re-renders whose output cannot change #397.The change
In
dateObserver.update(), each element's connected check,getUnitFactorand re-render now run in their owntry/catch, so even a throwingisConnectedgetter can't abort the tick:getUnitFactorthrows, the element is skipped for scheduling (it cannot poisonnearestDistance) and is not rendered, so its error is reported once.setTimeout(() => { throw error }), so it still reaches the page as an uncaught error, asynchronously and exactly once per throw.setTimeoutis used instead ofreportErrorbecausereportErroris missing from older browsers and non-browser hosts, and the rest of the observer already relies only onsetTimeout.Errors thrown from
connectedCallbackand the attribute-change microtask are not changed.Tests
New suite
errors thrown during a tickintest/relative-time.js. ThesetTimeoutstub tells the observer's timer (which has a delay) from the rethrow (which has none), so the rethrow is captured, not dropped:updatethrows does not stop others updating, and the timer reschedulesdategetter throws does not stop others or reschedulingtime-zonedoes not stop others or reschedulingwindowas an uncaughterrorevent (realsetTimeoutfor the rethrow)All of these except the last (a control) fail on
perf-skip-unchanged-tickswithout the source change.Commands run (Node 22):
With
src/relative-time-element.tsrestored toperf-skip-unchanged-ticksand the new tests kept:The 6 failures are the first 6 tests listed above.