Implement retry logic in HTTP Send method

[myselfghost]

Added retry logic with exponential backoff to the Send method for HTTP requests, improving error handling and resilience against transient failures.

[qwerty287]

Thanks, but could you explain why this is necessary?

[myselfghost]

Thanks, but could you explain why this is necessary?

In my actual usage, I found that every week some pipelines randomly fail to start due to HTTP request failures, possibly because the HTTP service is unstable. Therefore, retrying HTTP requests is necessary.

[qwerty287]

You're using a config extension?

[myselfghost]

You're using a config extension?

Yes, my entire CI pipeline relies on External Configuration API

[qwerty287]

Ok, so for backoffs we use github.com/cenkalti/backoff/v5. Can you use it?

[myselfghost]

Ok, so for backoffs we use github.com/cenkalti/backoff/v5. Can you use it?

ok , done

[qwerty287]

Would it be possible to let the lib handle the waiting etc, so we don't need a for loop etc.?

Similar to how this is done in https://github.com/woodpecker-ci/woodpecker/blob/main/cmd/server/server.go#L96-L104.

[myselfghost]

Would it be possible to let the lib handle the waiting etc, so we don't need a for loop etc.?

Similar to how this is done in https://github.com/woodpecker-ci/woodpecker/blob/main/cmd/server/server.go#L96-L104.

done

[lafriks]

Does it need result type at all, can't it just return int?

[myselfghost]

Does it need result type at all, can't it just return int?

Dear maintainers, if there are any details that still need to be adjusted, please feel free to make the changes directly. @qwerty287 @lafriks

[qwerty287]

@lafriks changed that, do you want to check it out again?

[qwerty287]

@woodpecker-ci/maintainers as I modified quite some parts of this, does somebody else wants to review?

[xoxys]

Code LGTM, not tested. As we talk about it, can we add tests?

[qwerty287]

Not sure how - we can of course send requests to non-existing servers, but then we somehow have to catch that and count the tries. If the server itself is reachable it won't retry… Do you have an idea?

[anbraten]

You could start an http server in the test with a handler that returns 500 the first x times and then a config. Similar to

woodpecker/server/services/config/combined_test.go

Line 187 in 98929ba

ts := httptest.NewServer(http.HandlerFunc(fixtureHandler))

[qwerty287]

Ah, sorry, I thought any status code is not retried. Yes, this works.

[qwerty287]

Test added in a9e49a1