Skip to content

Fix segmentation fault on invalid home-id in live command - #17

Merged
KristofferRisa merged 2 commits into
KristofferRisa:mainfrom
pcriv:fix/issue-13
Aug 28, 2026
Merged

Fix segmentation fault on invalid home-id in live command#17
KristofferRisa merged 2 commits into
KristofferRisa:mainfrom
pcriv:fix/issue-13

Conversation

@pcriv

@pcriv pcriv commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Closes #13

pcriv added 2 commits August 25, 2026 23:59
- Use specific error messages for invalid home IDs
- Refactor test file to use table-driven tests
- Remove unsafe fmt.Errorf usage
@pcriv
pcriv requested a review from KristofferRisa as a code owner August 25, 2026 21:59
@pcriv
pcriv marked this pull request as draft August 25, 2026 22:00
@pcriv
pcriv marked this pull request as ready for review August 26, 2026 06:25
@KristofferRisa

Copy link
Copy Markdown
Owner

Merging — thanks for the fix and especially for the tests.

I verified this beyond the green check, since CI ran on 2026-08-25 against base 365c41d and main has moved on (#12, #15, #16, #19) since:

  • Trial-merged into current main — clean, no conflicts.
  • make check on the merged result passes: fmt, vet, go mod verify, go test -race ./....
  • Confirmed the tests are not vacuous: swapping the pre-fix websocket.go back under the new test file fails all three error cases with expected error, got nil, and they pass again once restored. They genuinely pin the regression.

The fix is at the right layer. parsePayload returning (nil, nil) on a Tibber error payload was the root cause — Subscribe then called handler(nil), and pretty.go:193 dereferenced it. Stopping nil at the parse boundary is better than nil-guarding the formatter.

Three notes for the record, none blocking:

  1. This changes streaming behavior beyond the segfault fix. Subscribe goes from continue to return err on parse failure, so a single malformed frame now terminates the stream where it was previously skipped. Right call for the invalid-home case, but it also means a transient bad frame mid-session ends the stream. Calling it out in the merge commit so it is not a surprise later.
  2. The friendly message keys off exact matches on "user not authorized to access home" / "home not found". If Tibber rewords, it falls through to the raw upstream message — degrades gracefully, so fine.
  3. FormatLiveMeasurement still has no nil guard. Unreachable via this path now, but it stays a sharp edge for future callers.

Minor nit for next time: the test struct field is named errContains but compared with != exact equality.

Closes #13.

@KristofferRisa
KristofferRisa merged commit b320ea2 into KristofferRisa:main Aug 28, 2026
5 checks passed
@pcriv
pcriv deleted the fix/issue-13 branch August 29, 2026 07:38
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.

panic: segmentation fault in powerctl live with invalid home ID

2 participants