Skip to content

TLS certificates page: count reads upfront and keep partial ACME rows - #1261

Merged
stephdl merged 2 commits into
sdl-8099from
sdl-8099-loading-counters
Jul 29, 2026
Merged

TLS certificates page: count reads upfront and keep partial ACME rows#1261
stephdl merged 2 commits into
sdl-8099from
sdl-8099-loading-counters

Conversation

@stephdl

@stephdl stephdl commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Targets sdl-8099, the branch of #1257, not main.

Description

A review pass on #1258 (HTTP routes page) raised two issues that apply here as well, since both pages share the same read pattern. This PR brings the two branches to the same shape, so the shared mixin planned as a follow-up on #1257 has one behaviour to extract instead of two.

Counters are set before the loop. listCertificates() and getAcmeServer() incremented inside the loop and decremented through a helper clamped with Math.max(0, n - 1). Two problems hide behind that clamp. The counter reaches zero between two iterations, so loadingCertificates / loadingServers can go false while the next createModuleTaskForApp is still awaited — the table drops out of its loading state with an empty or partial list. And a surplus decrement is swallowed rather than made impossible, which is what let the counter go out of balance in the first place. The counter is now this.traefikInstances.length before the loop and the decrements are plain --. Nothing is left to clamp: clearTaskListeners() already $offs the handlers of the previous round, so an event delivered late reaches nothing.

listCertificatesNum is released when the instance lookup is aborted. listInstalledModulesAborted reset loading.listInstalledModules but left the certificates counter untouched. On a reconnection the watcher restarts from listInstalledModules(), so if that task aborts the certificates chain never runs and whatever the counter held stays there — skeleton rows for good, with no way out but a manual reload.

One offline node no longer hides the others on the ACME tab. error.getAcmeServer is a single shared string, and NsDataTable renders its error notification instead of the rows (v-if="isErrorShown"v-else-if="isLoading" … in NsDataTable.vue). On a cluster where one node is unreachable, the settings of every node that answered were replaced by the banner. The notification now renders above the table when there are rows to keep, and isErrorShown is left for the case where there is genuinely nothing to display. The Certificates tab already handles this through offlineTraefikInstances and listCertificatesErrors, so it needs no change.

Commits

  1. fix(ui): count ACME and certificate reads upfront
  2. fix(ui): keep the ACME rows readable when one node fails

Testing

core/ui has no test runner, so this was checked with lint and a production build, on each commit. Both pass.

To verify on a cluster:

  • Both tables populate normally on a cold load and after a tab switch
  • Save ACME settings on the connected node: the websocket drops, reconnects, and both tables refresh on their own with no skeleton rows left
  • Reload during the Traefik restart and let it settle: neither table stays on skeleton rows
  • Stop one traefik, open the ACME tab: the rows of the healthy nodes are readable, with the error notification above the table
  • Restart it and save on another node: the missing row comes back and the notification clears
  • Stop the leader so list-installed-modules aborts after a reconnection: the certificates table shows its error instead of loading for ever
  • No duplicate rows in either table after several reconnections

stephdl added 2 commits July 29, 2026 14:34
The counters were incremented inside the loop and decremented through a
helper clamped with Math.max(0, n - 1). The clamp hides two things: the
counter reaches zero between two iterations, so the table can leave its
loading state while the next task is still being created, and a surplus
decrement is silently swallowed instead of being impossible.

Set the counter to the number of instances before the loop and decrement
it plainly. clearTaskListeners() already makes a stale event unable to
reach a handler, so nothing is left to clamp.

Release listCertificatesNum when list-installed-modules is aborted: the
certificates chain never runs, and the skeleton rows stayed for good when
the abort followed a reconnection.
error.getAcmeServer is a single shared string and NsDataTable renders its
error notification instead of the rows, so one unreachable node hid the
settings of every node that did answer.

Show the notification above the table when there are rows to keep, and
leave isErrorShown to the case where there is nothing else to display.
@stephdl
stephdl merged commit 495c42f into sdl-8099 Jul 29, 2026
2 checks passed
@stephdl
stephdl deleted the sdl-8099-loading-counters branch July 29, 2026 12:36
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.

1 participant