Repository navigation
Isolate errors thrown by one element during a shared-timer tick - #399
Open
mattcosta7 wants to merge 2 commits into
Open
mattcosta7 wants to merge 2 commits into
mattcosta7 wants to merge 2 commits into
Conversation
All relative-time elements share one timer. If anything threw while the tick handled one element, including its isConnected check, getUnitFactor (for example an invalid time-zone on an element an hour or more old) or update(), the loop was abandoned: later elements weren't updated and the next tick was never scheduled, so every timestamp on the page froze. Each element is now handled in its own try/catch, and the error is rethrown from a task of its own so it is still reported, uncaught, exactly once. An element that fails is skipped if the tick reaches it again, as when it moved itself and rejoined the observer's set, so the tick always finishes. Supersedes #371. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Nov 8, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the changes isolating per-element failures in the shared timer.
No open issues 🎉
What changed in this PR
This PR isolates per-element failures in the shared <relative-time> timer, building on #396 so one broken element no longer prevents other timestamps from updating or stops future ticks.
Changes:
- Guards connection checks, scheduling calculations, and rendering, while reporting caught errors asynchronously.
- Prevents failed elements that rejoin the observer from being processed repeatedly in the same tick.
- Adds regression tests for failure isolation, retries, timer scheduling, and uncaught error reporting.
| File | Description |
|---|---|
test/relative-time.js |
Adds shared-timer error-isolation and error-reporting tests. |
src/relative-time-element.ts |
Catches per-element failures and skips failed elements on repeat visits within a tick. |
Review effort: Balanced (auto)
This branch has not been deployed
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 #396. Supersedes #371 and #398.
What this fixes
All
<relative-time>elements share one timer. Each tick loops over the elements, then schedules the next tick. Onmain, if anything throws while one element is handled, the loop is abandoned. The elements after it aren't updated and the next tick is never scheduled. One broken element, for example one an hour or more old with an invalidtime-zone, freezes every timestamp on the page.The change
In
dateObserver.update(), each element is handled in its owntry/catch: its connected check,getUnitFactorandupdate().getUnitFactorthrows, the element doesn't count toward the next tick's delay and isn't rendered.update()throws, the element's delay still counts, so it's retried on the next tick.setTimeout(() => { throw error }). So it still reaches the page as an uncaught error, asynchronously and exactly once per throw. I usedsetTimeoutinstead ofreportError, which older browsers and non-browser hosts lack; the observer already depends only onsetTimeout.Errors thrown from
connectedCallbackand from the attribute-change microtask behave as before.One behavior changes: a permanently broken element now reports its error on every tick. Before, the first error stopped the timer for the whole page.
Why #371 and #398 are superseded
update().getUnitFactorran outside itstry, so a throwingdategetter or an invalidtime-zonestill stopped the timer. Nothing tested that the error is still reported, and its base branch is 61 commits behindmain.Tests
The new suite
errors thrown during a tickintest/relative-time.jscovers these cases:update(),isConnectedgetter ordategetter throwstime-zonewindowas an uncaughterrorevent through the realsetTimeoutThe
setTimeoutstub tells the observer's timer, which has a delay, apart from the rethrow, which has none, so rethrows are captured rather than dropped.With the source change reverted and the new tests kept, the 8 isolation tests fail and the control passes.
Measured in Chromium with 500 elements a few hours old and one fresh element ticking every second, the per-element
trydoesn't change tick cost: about 20.5 ms of busy time per 8 s both with and without it.npm test(build, lint, web-test-runner)npx prettier --check src testnpx web-test-runner --files test/relative-time.jswithsrcfrom #396