[PR #323] [MERGED] fix(installer): propagate swallowed GatherUpdatesInfo errors #318

New Issue

2026-06-06T22:10:12-04:00

yindo commented

2026-06-06 22:10:12 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/323
Author: @mrigankad
Created: 5/31/2026
Status: ✅ Merged
Merged: 5/31/2026
Merged by: @asdek

Base: main ← Head: fix/installer-swallowed-gather-error

📝 Commits (1)

6bea158 fix(installer): propagate swallowed GatherUpdatesInfo errors

📊 Changes

1 file changed (+10 additions, -10 deletions)

View changed files

📝 backend/cmd/installer/processor/logic.go (+10 -10)

📄 Description

Problem

In the installer processor (cmd/installer/processor/logic.go), failures from the post-operation state refresh checker.GatherUpdatesInfo(ctx) were silently discarded. As a result apply / install / update / remove / purge could report success to the UI even when the post-step refresh actually failed.

Surfaced by golangci-lint (ineffassign), which the project's CI runs with --issues-exit-code=0, so these never blocked anything.

Root Cause

Two distinct patterns wrapped the error into a value that was then thrown away:

Deferred closures in applyChanges, install, update, purge:

defer func() {
    if err != nil { return }
    if err := p.checker.GatherUpdatesInfo(ctx); err != nil { // shadows named return
        err = fmt.Errorf("failed to gather info after update: %w", err) // assigns the shadow
    }
}()

The inner if err := ... shadows the function's named return err, so the wrap never reaches the outer defer sendCompletion(stack, err).

ProductStackWorker switch cases in remove and purge:
```
if err := p.checker.GatherUpdatesInfo(ctx); err != nil {
    err = fmt.Errorf("failed to gather info after update: %w", err) // block-local, falls through
}
```
The block-local err is wrapped but the case falls through without returning — even though every sibling check in the same switch returns on error.

Solution

Deferred closures: wrap into a gatherErr local and assign the named return err, so the failure propagates to sendCompletion.
Switch cases: return the wrapped error, matching the sibling checks in the same switch.

The two contexts need different fixes: a deferred closure cannot return the operation's error, so it must assign the named return; the switch cases follow the established early-return-on-error contract.

Testing

go build ./cmd/installer/...           # OK
go vet ./cmd/installer/processor/...    # OK
go test ./cmd/installer/processor/...   # ok
golangci-lint run -E ineffassign ./cmd/installer/processor/...   # logic.go clean

Risk Assessment

Low. Behavior changes only on the previously-swallowed error path:

Deferred-closure cases now report a GatherUpdatesInfo failure that was previously hidden (no early return added — semantics of the success path unchanged).
The two switch cases now early-return on GatherUpdatesInfo error, consistent with every adjacent check in the same switch. The success path (err == nil) is unchanged.
No public API, schema, or migration changes.

_{🔄 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/vxcontrol/pentagi/pull/323 **Author:** [@mrigankad](https://github.com/mrigankad) **Created:** 5/31/2026 **Status:** ✅ Merged **Merged:** 5/31/2026 **Merged by:** [@asdek](https://github.com/asdek) **Base:** `main` ← **Head:** `fix/installer-swallowed-gather-error` --- ### 📝 Commits (1) - [`6bea158`](https://github.com/vxcontrol/pentagi/commit/6bea1581f6f1029f3a44579355bd0d7a19bad920) fix(installer): propagate swallowed GatherUpdatesInfo errors ### 📊 Changes **1 file changed** (+10 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `backend/cmd/installer/processor/logic.go` (+10 -10) </details> ### 📄 Description ## Problem In the installer processor (`cmd/installer/processor/logic.go`), failures from the post-operation state refresh `checker.GatherUpdatesInfo(ctx)` were silently discarded. As a result `apply` / `install` / `update` / `remove` / `purge` could report **success** to the UI even when the post-step refresh actually failed. Surfaced by `golangci-lint` (`ineffassign`), which the project's CI runs with `--issues-exit-code=0`, so these never blocked anything. ## Root Cause Two distinct patterns wrapped the error into a value that was then thrown away: 1. **Deferred closures** in `applyChanges`, `install`, `update`, `purge`: ```go defer func() { if err != nil { return } if err := p.checker.GatherUpdatesInfo(ctx); err != nil { // shadows named return err = fmt.Errorf("failed to gather info after update: %w", err) // assigns the shadow } }() ``` The inner `if err := ...` shadows the function's named return `err`, so the wrap never reaches the outer `defer sendCompletion(stack, err)`. 2. **`ProductStackWorker` switch cases** in `remove` and `purge`: ```go if err := p.checker.GatherUpdatesInfo(ctx); err != nil { err = fmt.Errorf("failed to gather info after update: %w", err) // block-local, falls through } ``` The block-local `err` is wrapped but the case falls through without returning — even though every sibling check in the same switch `return`s on error. ## Solution - **Deferred closures:** wrap into a `gatherErr` local and assign the named return `err`, so the failure propagates to `sendCompletion`. - **Switch cases:** `return` the wrapped error, matching the sibling checks in the same `switch`. The two contexts need different fixes: a deferred closure cannot `return` the operation's error, so it must assign the named return; the switch cases follow the established early-return-on-error contract. ## Testing ``` go build ./cmd/installer/... # OK go vet ./cmd/installer/processor/... # OK go test ./cmd/installer/processor/... # ok golangci-lint run -E ineffassign ./cmd/installer/processor/... # logic.go clean ``` ## Risk Assessment Low. Behavior changes only on the previously-swallowed error path: - Deferred-closure cases now report a `GatherUpdatesInfo` failure that was previously hidden (no early return added — semantics of the success path unchanged). - The two switch cases now early-return on `GatherUpdatesInfo` error, consistent with every adjacent check in the same switch. The success path (err == nil) is unchanged. No public API, schema, or migration changes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:10:12 -04:00

yindo closed this issue

2026-06-06 22:10:12 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#318