Skip to content

core/wait: waiter returns (nil, nil) on a retryable 502/504 and never retries #11084

Description

@devpie

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)
}
  1. go mod init repro && go get github.com/stackitcloud/stackit-sdk-go/core@v0.27.0
  2. 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:

  1. 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.
  2. 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)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions