Description
A wait handler built with wait.WaiterHelper reports success with a nil response when a single poll hits a
retryable HTTP status (502 or 504), and the retry it is supposed to perform never happens.
Two pieces of core/wait disagree about what done means:
WaiterHelper.Wait() returns true for waitFinished on any fetch error, so the error is the only thing that
distinguishes "finished" from "broken"
(waiterhelper.go):
instance, err := w.FetchInstance()
if err != nil {
...
return true, nil, err
}
WaitWithContext then hands that error to handleError, which swallows a retryable status and returns a nil
error — but the if done check that follows does not know the error was swallowed
(wait.go):
done, res, err := h.checkFn()
if err != nil {
retryTempErrorCounter, err = h.handleError(retryTempErrorCounter, err)
if err != nil {
return res, err
}
}
if done {
return res, nil // res is nil, err was swallowed, no retry
}
So the caller gets (nil, nil). Callers reasonably treat a nil error as success and dereference the response, which
panics. tempErrRetryLimit is never reached because control never returns to the loop.
Steps to reproduce
package main
import (
"context"
"fmt"
"net/http"
"time"
"github.com/stackitcloud/stackit-sdk-go/core/oapierror"
"github.com/stackitcloud/stackit-sdk-go/core/wait"
)
type instance struct{ Name string }
func main() {
calls := 0
helper := wait.WaiterHelper[instance, string]{
// One single 502 from the API, then the resource would be ACTIVE.
FetchInstance: func() (*instance, error) {
calls++
if calls == 1 {
return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusBadGateway}
}
return &instance{Name: "my-resource"}, nil
},
GetState: func(i *instance) (string, error) { return "ACTIVE", nil },
ActiveState: []string{"ACTIVE"},
ErrorState: []string{"ERROR"},
}
handler := wait.New(helper.Wait())
handler.SetThrottle(10 * time.Millisecond).SetTimeout(5 * time.Second)
res, err := handler.WaitWithContext(context.Background())
fmt.Printf("FetchInstance calls: %d\n", calls)
fmt.Printf("returned response: %v\n", res)
fmt.Printf("returned error: %v\n", err)
}
go mod init repro && go get github.com/stackitcloud/stackit-sdk-go/core@v0.27.0
go run .
Actual behavior
FetchInstance calls: 1
returned response: <nil>
returned error: <nil>
The waiter stops after the very first poll, reports no error, and returns no instance. FetchInstance is called
once, so the retry the code sets out to perform did not occur.
Expected behavior
Either of the following would be correct, the first seems closer to the intent:
- After
handleError swallows a retryable status, continue the loop and poll again, up to tempErrRetryLimit.
In the example above the second poll returns ACTIVE and the waiter succeeds.
- Failing that, never return
(nil, nil) — a nil error must imply a usable response.
A minimal fix for (1) is to not let a swallowed error fall through to the done branch, e.g.
done, res, err := h.checkFn()
if err != nil {
retryTempErrorCounter, err = h.handleError(retryTempErrorCounter, err)
if err != nil {
return res, err
}
// the error was retryable and got swallowed, so `done` reflects the failed
// fetch rather than a finished action - poll again instead of returning
done = false
}
if done {
return res, nil
}
Impact
Every service waiter built on WaiterHelper is affected — the create and update handlers just as much as delete,
since RetryHttpErrorStatusCodes is checked before any of the delete-specific handling. In the Terraform provider
this surfaces as a panic: the SFS resources dereference the wait handler result after checking only the error
(stackitcloud/terraform-provider-stackit#1741 guards the call sites, but the nil-with-nil-error return comes from
here).
Environment
- OS: macOS 15 (darwin/arm64)
- Go version (see
go version): 1.26.8
- Version of the STACKIT Go SDK:
core v0.27.0 (also reproduces on v0.26.0)
Description
A wait handler built with
wait.WaiterHelperreports success with a nil response when a single poll hits aretryable HTTP status (502 or 504), and the retry it is supposed to perform never happens.
Two pieces of
core/waitdisagree about whatdonemeans:WaiterHelper.Wait()returnstrueforwaitFinishedon any fetch error, so the error is the only thing thatdistinguishes "finished" from "broken"
(waiterhelper.go):
WaitWithContextthen hands that error tohandleError, which swallows a retryable status and returns a nilerror — but the
if donecheck that follows does not know the error was swallowed(wait.go):
So the caller gets
(nil, nil). Callers reasonably treat a nil error as success and dereference the response, whichpanics.
tempErrRetryLimitis never reached because control never returns to the loop.Steps to reproduce
go mod init repro && go get github.com/stackitcloud/stackit-sdk-go/core@v0.27.0go run .Actual behavior
The waiter stops after the very first poll, reports no error, and returns no instance.
FetchInstanceis calledonce, so the retry the code sets out to perform did not occur.
Expected behavior
Either of the following would be correct, the first seems closer to the intent:
handleErrorswallows a retryable status, continue the loop and poll again, up totempErrRetryLimit.In the example above the second poll returns
ACTIVEand the waiter succeeds.(nil, nil)— a nil error must imply a usable response.A minimal fix for (1) is to not let a swallowed error fall through to the
donebranch, e.g.Impact
Every service waiter built on
WaiterHelperis affected — the create and update handlers just as much as delete,since
RetryHttpErrorStatusCodesis checked before any of the delete-specific handling. In the Terraform providerthis surfaces as a panic: the SFS resources dereference the wait handler result after checking only the error
(stackitcloud/terraform-provider-stackit#1741 guards the call sites, but the nil-with-nil-error return comes from
here).
Environment
go version):1.26.8core v0.27.0(also reproduces onv0.26.0)