Skip to content

Handle dashboard promises and stop shadowing min/max builtins - #169

Merged
LarsLaskowski merged 2 commits into
mainfrom
claude/clever-goodall-y0wroe
Oct 17, 2026
Merged

LarsLaskowski merged 2 commits into
mainfrom
claude/clever-goodall-y0wroe

Conversation

@LarsLaskowski

@LarsLaskowski LarsLaskowski commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

📖 Description

This PR clears the six open SonarCloud findings without changing behavior, and bumps the Go toolchain to fix the failing vulnerability scan.

  • Floating promises in the dashboard (javascript:S9383, 4×).
    • Problem: internal/web/assets/app.js called its async poll functions and main() as bare statements, so their promises were dropped. A failure outside each function's own try/catch (for example a render error in reloadAll → renderVersion()/startPolling()) only showed up as an anonymous unhandled rejection.
    • Fix: The three refreshes in the visibilitychange handler are independent and each handles its own fetch errors, so they are now marked fire-and-forget with void. main() now has a .catch that logs the failure with console.error.
  • Shadowed Go builtins (go:S978, 2×). handleHealthz used a local named max, and eventReaches took a parameter named min. Both names shadow the builtins added in Go 1.21. They are renamed to maxStale and minLevel.
  • Go toolchain 1.26.7 → 1.26.9.
    • Problem: govulncheck reports ten reachable standard-library vulnerabilities for go1.26.7 in crypto/tls, net/http and net/textproto (GO-2026-6603 … GO-2026-6617). This also fails the Vulnerability scan job on main.
    • Fix: All ten are fixed in go1.26.9, so go.mod now pins that version. go mod tidy leaves go.sum unchanged. The README and CONTRIBUTING say "Go 1.26+" and stay accurate.

🎫 Issues

Closes #168

👩‍💻 Reviewer Notes

  • Components touched:
    • the dashboard bootstrap and visibility handler (internal/web/assets/app.js)
    • GET /healthz (internal/httpapi/handlers.go)
    • webhook level filtering (internal/alert/notify.go)
    • the toolchain pin in go.mod
  • Unchanged: the Go code changes are pure renames. No route, config key, collector field or /api/v1 response shape changes.
  • Smoke test on a Pi:
    1. Open the dashboard and confirm it loads.
    2. Switch to another tab, wait at least one poll interval, then switch back. The cards should refresh immediately.
    3. Check the browser console: there should be no Uncaught (in promise) errors.

📑 Test Plan

  • New test TestAppJS_AsyncCallsAreNotFloating (internal/web/polling_test.go). It finds every async function declared in app.js and fails if any of them is called as a bare statement. Run against the old app.js, it reports exactly the four Sonar lines; against the fixed file it passes.
  • Renames: covered by the existing handleHealthz staleness tests and the eventReaches table test in internal/alert/notify_test.go. They change no behavior, so they get no new Go tests.
  • Toolchain bump: verified by the CI Vulnerability scan (govulncheck). The local environment cannot reach vuln.go.dev.
  • Local runs with go1.26.9: go build ./..., go vet ./... and go test ./... -race -cover pass, and golangci-lint run at v2.13.1 (the CI pin) reports 0 issues.
  • Not verified in a real browser: step 2 of the smoke test.

✅ Checklist

General

  • I have added/updated tests for my changes (go test ./... -race -cover passes locally).
  • go vet ./... and golangci-lint run are clean.
  • I have tested my changes.
  • I have read the CONTRIBUTING documentation and followed the project's code style guidelines.
  • I have updated ARCHITECTURE.md if this changes a documented design decision. (not applicable: no design decision changes)

REST API / configuration / packaging

  • I have updated docs/API.md to reflect a REST API change. (not applicable: no API change)
  • No breaking change to /api/v1/... response shapes, or a new API version
    (/api/v2/...) was introduced instead.
  • I have updated README.md / packaging/pimonitor.example.yaml to reflect a new or
    changed configuration option. (not applicable)
  • I have updated packaging/install.sh or the systemd units if this changes
    installation/packaging, and kept the unprivileged/privileged service split intact
    (see SECURITY.md). (not applicable)

⏭ Next Steps

After merge, check that the SonarCloud analysis on main shows 0 open issues.

The dashboard called its async poll functions and main() as bare
statements, dropping their promises: a rejection outside the functions'
own try/catch (e.g. a render bug) surfaced only as an anonymous
unhandled rejection. The visibilitychange refreshes are now explicitly
fire-and-forget via void, and main() gets a .catch that logs the
failure. A guard test fails if any of app.js's async functions is again
called as a bare statement.

handleHealthz and eventReaches named locals max and min, shadowing the
Go 1.21 builtins; rename them to maxStale and minLevel. No behavior
change.

Resolves the six open SonarCloud findings (javascript:S9383, go:S978).

Closes #168

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvX6eSR4d9QW6L7vbDSVXK

LarsLaskowski commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner Author

Vulnerability scan is red, but this PR didn't cause it.

go tool govulncheck ./... reports 10 reachable Go standard-library vulnerabilities (GO-2026-6603 … GO-2026-6617) in crypto/tls, net/http and net/textproto. They are found in go1.26.7, the toolchain go.mod pins (go 1.26.7), and fixed in go1.26.9.

main fails the same way on its base commit 49fa88a: see the scheduled CI run from 2026-10-12. The failure doesn't depend on this PR's diff, so I haven't re-run it. No existing PR fixes it yet.

Proposed fix, a one-line toolchain bump:

-go 1.26.7
+go 1.26.9

go.mod is the only place in the repo that pins 1.26.7. I'll leave it to the maintainer whether that goes into this PR or a separate one.

govulncheck reports ten reachable standard-library vulnerabilities in
crypto/tls, net/http and net/textproto (GO-2026-6603 through
GO-2026-6617) for go1.26.7, the version go.mod pinned. All are fixed in
go1.26.9. This was failing the Vulnerability scan job on main as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BvX6eSR4d9QW6L7vbDSVXK
@sonarqubecloud

Copy link
Copy Markdown

@LarsLaskowski
LarsLaskowski merged commit 13c4be0 into main Oct 17, 2026
12 checks passed
@LarsLaskowski
LarsLaskowski deleted the claude/clever-goodall-y0wroe branch October 17, 2026 16:32
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.

Resolve the 6 open SonarCloud findings (floating promises in app.js, shadowed min/max in Go)

2 participants