Skip to content

feat: add missing htt StatusCode and make it settable#6051

Open
mhenselin wants to merge 5 commits intostackitcloud:mainfrom
mhenselin:fix/add_missing_temp_error_code
Open

feat: add missing htt StatusCode and make it settable#6051
mhenselin wants to merge 5 commits intostackitcloud:mainfrom
mhenselin:fix/add_missing_temp_error_code

Conversation

@mhenselin
Copy link
Copy Markdown

Description

relates to #1234

Checklist

  • Issue was linked above
  • [ x ] No generated code was adjusted manually (check comments in file header)
  • Changelogs
    • Changelog in the root directory was adjusted (see here)
    • [ x ] Changelog(s) of the service(s) were adjusted (see e.g. here)
  • [ x ] VERSION file(s) of the service(s) were adjusted
  • Code format was applied: make fmt
  • Examples were added / adjusted (see examples/ directory)
  • Unit tests got implemented or updated
  • Unit tests are passing: make test (will be checked by CI)
  • No linter issues: make lint (will be checked by CI)

@mhenselin mhenselin requested a review from a team as a code owner March 30, 2026 10:28
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

@github-actions github-actions Bot added the Stale label Apr 8, 2026
@github-actions
Copy link
Copy Markdown

This PR was closed automatically because it has been stalled for 7 days with no activity. Feel free to re-open it at any time.

@github-actions github-actions Bot closed this Apr 15, 2026
@marceljk marceljk reopened this Apr 15, 2026
@marceljk marceljk removed the Stale label Apr 15, 2026
cgoetz-inovex
cgoetz-inovex previously approved these changes Apr 15, 2026
Comment thread core/wait/wait.go
)

var RetryHttpErrorStatusCodes = []int{http.StatusBadGateway, http.StatusGatewayTimeout}
var RetryHttpErrorStatusCodes = []int{http.StatusBadGateway, http.StatusGatewayTimeout, http.StatusServiceUnavailable}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always assumed this is public to allow setting it. With this change I'd be in favour of deprecating this and making it private after the deprecation period.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand this allows to express: "whatever the default is + sth. else" like:

myRetries := append(RetryHttpErrorStatusCodes, http.StatusRequestTimeout)

Comment thread core/wait/wait.go
}
// Some APIs may return temporary errors and the request should be retried
if !utils.Contains(RetryHttpErrorStatusCodes, oapiErr.StatusCode) {
if !utils.Contains(h.retryHttpErrorStatusCodes, oapiErr.StatusCode) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !utils.Contains(h.retryHttpErrorStatusCodes, oapiErr.StatusCode) {
if !slices.Contains(h.retryHttpErrorStatusCodes, oapiErr.StatusCode) {

I'd prefer the builtin func instead of the utils one, if we are already touching this line

Comment thread core/wait/wait_test.go
got.SetTempErrRetryLimit(tt.tempErrRetryLimit)

diff := cmp.Diff(got, want, cmpOpts...)
diff := cmp.Diff(want, got, cmpOpts...)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thx!

Comment thread core/wait/wait.go
}
// Some APIs may return temporary errors and the request should be retried
if !utils.Contains(RetryHttpErrorStatusCodes, oapiErr.StatusCode) {
if !utils.Contains(h.retryHttpErrorStatusCodes, oapiErr.StatusCode) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd also need a fallback here when h.retryHttpErrrorStatusCodes == nil for people that did not use New but a AsyncActionHandler{} literal to create an instance.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants