[PR #413] fix(connect): decode registration into a fresh WgData to prevent stale hcStatus on reconnect #8993

New Issue

GiteaMirror · 2026-07-14T21:07:50-05:00

GiteaMirror commented

2026-07-14 21:07:50 -05:00

📋 Pull Request Information

Original PR: https://github.com/fosrl/newt/pull/413
Author: @lafoush
Created: 7/14/2026
Status: 🔄 Open

Base: main ← Head: fix/reconnect-stale-hcstatus

📝 Commits (1)

4e35596 fix(connect): decode registration into a fresh WgData to prevent stale hcStatus

📊 Changes

2 files changed (+84 additions, -1 deletions)

View changed files

📝 newt/connect.go (+8 -1)
➕ newt/wgdata_reconnect_test.go (+76 -0)

📄 Description

Problem

A resource with a health check can be reported permanently Unhealthy after a reconnect, even though it responds correctly, until newt is restarted. It's intermittent and only happens on reconnect (never on a clean boot).

Root cause

handleConnect decodes the newt/wg/connect registration payload into the persistent n.wgData:

json.Unmarshal(jsonData, &n.wgData)

Go's encoding/json merges a JSON array into an existing non-empty slice element-by-element, by position, and a JSON null is a no-op for a non-pointer field such as healthcheck.Config.Status (hcStatus, an int). n.wgData is reused across reconnects, so:

First connect populates wgData.HealthCheckTargets, e.g. slot 0 = {id:47, hcStatus:401}, slot 1 = {id:5, hcStatus:null}.
On reconnect the server returns the same targets but in a different order (the health-check config query has no ORDER BY), e.g. [{id:5, hcStatus:null}, {id:47, hcStatus:401}].
Decoding into the reused slice merges new-slot-0 (id:5, hcStatus:null) into old-slot-0 (id:47, hcStatus:401). id is overwritten to 5, but the null hcStatus does not overwrite the retained 401.
Result: target 5 now expects 401. Its check (GET /manifest.json → 200) fails forever with unexpected status code: 200 (expected: 401).

handleSync is unaffected because it decodes into a fresh local SyncData. A clean boot is unaffected because wgData starts empty. This matches the observed profile exactly: reconnect-only, intermittent (needs a row reorder), self-heals on restart.

Fix

Decode the registration payload into a fresh WgData and assign it on success, so no state is ever carried over from a previous connection:

var wgData WgData
if err := json.Unmarshal(jsonData, &wgData); err != nil {
    ...
    return
}
n.wgData = wgData

n.wgData is written nowhere else, so a wholesale replace is safe.

Test

Adds newt/wgdata_reconnect_test.go, which pins both halves:

decoding two reordered payloads into a reused WgData reproduces the stale 401 inheritance;
decoding each into a fresh WgData (what handleConnect now does) keeps target 5 free of target 47's expected code.

go build ./..., go vet ./newt/, and the full ./newt/ + ./healthcheck/ suites pass.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fosrl/newt/pull/413 **Author:** [@lafoush](https://github.com/lafoush) **Created:** 7/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/reconnect-stale-hcstatus` --- ### 📝 Commits (1) - [`4e35596`](https://github.com/fosrl/newt/commit/4e355968d1de36358adbd30891b8ba6ba63dfa06) fix(connect): decode registration into a fresh WgData to prevent stale hcStatus ### 📊 Changes **2 files changed** (+84 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `newt/connect.go` (+8 -1) ➕ `newt/wgdata_reconnect_test.go` (+76 -0) </details> ### 📄 Description ## Problem A resource with a health check can be reported **permanently Unhealthy after a reconnect**, even though it responds correctly, until newt is restarted. It's intermittent and only happens on reconnect (never on a clean boot). ### Root cause `handleConnect` decodes the `newt/wg/connect` registration payload into the **persistent** `n.wgData`: ```go json.Unmarshal(jsonData, &n.wgData) ``` Go's `encoding/json` merges a JSON array into an existing non-empty slice **element-by-element, by position**, and a JSON `null` is a **no-op** for a non-pointer field such as `healthcheck.Config.Status` (`hcStatus`, an `int`). `n.wgData` is reused across reconnects, so: 1. First connect populates `wgData.HealthCheckTargets`, e.g. slot 0 = `{id:47, hcStatus:401}`, slot 1 = `{id:5, hcStatus:null}`. 2. On reconnect the server returns the same targets but in a **different order** (the health-check config query has no `ORDER BY`), e.g. `[{id:5, hcStatus:null}, {id:47, hcStatus:401}]`. 3. Decoding into the reused slice merges new-slot-0 (`id:5`, `hcStatus:null`) **into** old-slot-0 (`id:47`, `hcStatus:401`). `id` is overwritten to 5, but the `null` `hcStatus` does **not** overwrite the retained `401`. 4. Result: target 5 now expects `401`. Its check (`GET /manifest.json` → `200`) fails forever with `unexpected status code: 200 (expected: 401)`. `handleSync` is unaffected because it decodes into a fresh local `SyncData`. A clean boot is unaffected because `wgData` starts empty. This matches the observed profile exactly: reconnect-only, intermittent (needs a row reorder), self-heals on restart. ## Fix Decode the registration payload into a **fresh** `WgData` and assign it on success, so no state is ever carried over from a previous connection: ```go var wgData WgData if err := json.Unmarshal(jsonData, &wgData); err != nil { ... return } n.wgData = wgData ``` `n.wgData` is written nowhere else, so a wholesale replace is safe. ## Test Adds `newt/wgdata_reconnect_test.go`, which pins both halves: - decoding two reordered payloads into a **reused** `WgData` reproduces the stale `401` inheritance; - decoding each into a **fresh** `WgData` (what `handleConnect` now does) keeps target 5 free of target 47's expected code. `go build ./...`, `go vet ./newt/`, and the full `./newt/` + `./healthcheck/` suites pass. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-07-14 21:07:50 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/newt#8993