Skip to content

GitHub release check: split for testability, add visible error on failure #200

Description

@jibrilsharafi

Context

CustomServer::_fetchGitHubReleaseInfo (source/src/customserver.cpp:1314) fetches the latest release from the GitHub API and decides whether the running firmware is up to date. It's the only path used to populate /api/v1/firmware/update-info in community mode.

Problem 1: not isolatable / not unit-tested

The function is static inside namespace CustomServer and fuses four concerns in one place:

  • connectivity gate (CustomWifi::isFullyConnected)
  • network I/O (HTTPClient GET)
  • JSON parsing (ArduinoJson w/ SpiRamAllocator)
  • business logic: asset .bin matching, version compare (_compareVersions), deciding which fields to populate

None of this can be linked or compiled under the native test env (pio test -e native), so it has zero test coverage. The project already has an established pattern for this split — see lib/shadow_logic, lib/meter_logic: pure logic with no Arduino/ArduinoJson/FreeRTOS deps lives in lib/* and gets a test/test_* on the host.

Ask: extract the pure parts into lib/github_release/ (or similar):

  • _compareVersions — already dependency-free, trivial lift-and-shift.
  • the release-JSON -> decision logic (given tagName, releaseDate, changelog, asset names) -> {isLatest, availableVersion, releaseDate, updateUrl, changelogUrl} — currently inline, needs pulling out as a pure function taking already-parsed primitives.

_fetchGitHubReleaseInfo keeps only the network I/O and calls into the new pure function.

Problem 2: failures are invisible

Every failure path (no internet, HTTP != 200, JSON parse error, missing tag_name) returns false, and the caller (customserver.cpp:1433-1436) swallows it into isLatest = true + a single LOG_WARNING. There's no user-visible signal that the check itself is broken — a persistently failing check looks identical to genuinely being up to date.

Ask: surface a distinct state to the API/UI (e.g. "updateCheckStatus": "ok" | "error" alongside isLatest, with a short reason) instead of silently defaulting to "latest" on any internal failure, so a broken check is visibly a broken check and not indistinguishable from "you're current."

Possible root cause worth checking alongside this

http.begin(GITHUB_API_LATEST_RELEASE_URL) uses the single-arg overload for an https:// URL with no explicit WiFiClientSecure/CA cert — unconfirmed whether this negotiates TLS correctly on this HTTPClient version. Worth checking device-side logs (GitHub API request failed with code: %d) to see if this is the actual failure mode before/while doing the split.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrobustnessCrash handling, stability, and system resilience

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions