In season_trends.js, renderTrends() sets loaded = true synchronously before the async buildTrends() resolves. If the on-demand Chart.js CDN fetch (or /api/trends/series) fails on the first open, the .catch calls showError, which does wrapper.innerHTML = ... and destroys the <canvas>. On a later open, loaded === true with an empty charts array, so renderTrends no-ops and the Trends subpage stays in its error state until a full page reload.
This is a deliberate, documented tradeoff today (see the catch comment at ~lines 254-259: showError destroys the canvases, so a naive rebuild can't reuse them). It roughly matches the pre-PR behavior (a static <head> Chart.js load that failed was also unrecoverable without reload), but the failure surface is wider now that the fetch is click-triggered rather than once-at-page-load.
Robust fix (deferred -- needs the error path reworked + failure-injection testing):
- Latch
loaded = true only on successful build (in the .then), not eagerly.
- Make
showError overlay the message instead of replacing wrapper.innerHTML, so the <canvas> survives and a later renderTrends can re-enter buildTrends.
- Handle partial-success retry (one chart built, one failed) so a rebuild doesn't double-instantiate.
Surfaced by the loop-review altitude pass on PR #170. Low severity (transient CDN blip on a subpage), so deferred rather than blocking the merge.
In
season_trends.js,renderTrends()setsloaded = truesynchronously before the asyncbuildTrends()resolves. If the on-demand Chart.js CDN fetch (or/api/trends/series) fails on the first open, the.catchcallsshowError, which doeswrapper.innerHTML = ...and destroys the<canvas>. On a later open,loaded === truewith an emptychartsarray, sorenderTrendsno-ops and the Trends subpage stays in its error state until a full page reload.This is a deliberate, documented tradeoff today (see the catch comment at ~lines 254-259:
showErrordestroys the canvases, so a naive rebuild can't reuse them). It roughly matches the pre-PR behavior (a static<head>Chart.js load that failed was also unrecoverable without reload), but the failure surface is wider now that the fetch is click-triggered rather than once-at-page-load.Robust fix (deferred -- needs the error path reworked + failure-injection testing):
loaded = trueonly on successful build (in the.then), not eagerly.showErroroverlay the message instead of replacingwrapper.innerHTML, so the<canvas>survives and a laterrenderTrendscan re-enterbuildTrends.Surfaced by the loop-review altitude pass on PR #170. Low severity (transient CDN blip on a subpage), so deferred rather than blocking the merge.